Skip to content

fix(types): export ParamEntry and ParamsIn - #31

Merged
Makisuo merged 2 commits into
mainfrom
feat/export-param-types
Oct 14, 2026
Merged

Makisuo merged 2 commits into
mainfrom
feat/export-param-types

Conversation

@Makisuo

@Makisuo Makisuo commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

What

ParamEntry, OptionalParamEntry, OptionalParams and ParamsIn are now exported from every dialect entry point (/clickhouse, /postgres, through core), and they're documented in docs/reference.md.

Why

When a package exports a query that uses a param.*, or a defineFn helper, tsc --declaration has to write those types out, and it couldn't name them:

TS4023: Exported variable 'byApp' has or is using name 'ParamEntry' from external module ".../dist/expr-DO5VktjE" but cannot be named.
TS2742: The inferred type of 'toStartOfDay' cannot be named without a reference to '.../dist/expr-DO5VktjE.mjs'.

These types live in an internal chunk with a hashed filename, so the emitted declarations had nothing public to refer to. This shows up whenever a project defines its queries in one package and runs them from another. superwall/paywall-next#4365 had to work around it by wrapping a query in a non-exported compile function and hand-writing the signatures of three defineFn helpers.

Testing

  • Reproduced first: I installed the packed tarball in a fresh project and got both errors. With this change, the emitted .d.mts refers to CH.ParamEntry and CH.ParamsIn instead.

  • New regression check: test:package now emits declarations, on both TypeScript 7 and 5, for:

    • exported ClickHouse and Postgres queries;
    • queries with optional params (when / whenTrue) and union-named params, which carry OptionalParamEntry;
    • a join, a CTE, a subquery and a union over parameterized queries;
    • a defineFn helper.

    Without the exports it fails with TS4023. The extra shapes found no other unexported type.

  • Full suite: bun run typecheck and bun run test pass: 684 tests, doc citations, the export catalog (now 287 /clickhouse and 205 /postgres exports) and doc examples.

🤖 Generated with Claude Code

A package that exports a query with a `param.*`, or a `defineFn`
helper, failed `tsc --declaration`: TS4023 for the query (its type
carries `ParamEntry`) and TS2742 for the helper (`ParamsIn`). tsc could
reach either only through a hashed internal chunk, so a project that
defines its queries in one package and runs them from another had to
wrap them in non-exported functions or hand-write signatures.

Both are now exported from every dialect entry point. The
package-consumer check emits declarations for an exported ClickHouse
query, a Postgres query and a defineFn helper on TypeScript 7 and 5;
without the exports it fails with TS4023.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@maple-review-bot

maple-review-bot Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Maple review

🟢 Confidence 7/10 · likely safe to merge
The export is right for plain params, but the when/union-name params shape that also names a chunk-only type is untested and I could not run tsc to confirm emit.
quality 90/100 · 1 warning · tests partial · risk low

Exports ParamEntry and ParamsIn from the shared core barrel so a consumer's exported query or defineFn helper can name them, and adds a declaration-emit check on the packed tarball. The list looks one type short for queries whose params come from when.

  • core.ts re-exports ParamEntry, ParamsIn and ParamsSatisfied from ./ch/expr
  • test:package now emits declarations for a ClickHouse query, a Postgres query and a defineFn helper

Before merge

  • Manual · If OptionalParamEntry / OptionalParams are exported too, docs/reference.md must name them or bun run test fails

Findings

🟠 Warning · F1 · when queries still fail declaration emit: OptionalParamEntry unexported

correctness · src/core.ts:101

A consumer that exports a query whose where uses when / whenTrue (or a param whose name is a union, e.g. param.int(cond ? "a" : "b")) gets OptionalParams<ParamEntry<…>> as the query's params phantom (src/ch/expr.ts:756, :764), which expands to OptionalParamEntry<…> — declared at src/ch/expr.ts:88 and re-exported from no public entry. tsc --declaration has to name that chunk-only symbol exactly as it had to name ParamEntry, so the consumer's emitted .d.mts still fails with TS4023. tests/package-consumer-declarations.mts only uses a plain param.int / param.string, so the new check does not exercise this shape.

Add `type OptionalParamEntry` and `type OptionalParams` to this export statement, add a `when`-based exported query to `tests/package-consumer-declarations.mts`, and name both types in `docs/reference.md` (`scripts/check-exports-documented.mjs` fails on an undocumented export).
🤖 Prompt to fix this finding with an AI agent
Findings from an automated review of commit 6bfed007d295f80a57ad94e6a828d0618012c1d4. Verify each one against the current code before changing anything, fix only those that still apply, and keep each fix to the lines it names.

---

F1 · Warning · correctness · src/core.ts:101
`when` queries still fail declaration emit: `OptionalParamEntry` unexported
A consumer that exports a query whose `where` uses `when` / `whenTrue` (or a param whose name is a union, e.g. `param.int(cond ? "a" : "b")`) gets `OptionalParams<ParamEntry<…>>` as the query's params phantom (`src/ch/expr.ts:756`, `:764`), which expands to `OptionalParamEntry<…>` — declared at `src/ch/expr.ts:88` and re-exported from no public entry. `tsc --declaration` has to name that chunk-only symbol exactly as it had to name `ParamEntry`, so the consumer's emitted `.d.mts` still fails with TS4023. `tests/package-consumer-declarations.mts` only uses a plain `param.int` / `param.string`, so the new check does not exercise this shape.
Suggested fix: Add `type OptionalParamEntry` and `type OptionalParams` to this export statement, add a `when`-based exported query to `tests/package-consumer-declarations.mts`, and name both types in `docs/reference.md` (`scripts/check-exports-documented.mjs` fails on an undocumented export).
What was checked
  • ParamEntry and ParamsIn are declared and exported in src/ch/expr.ts (lines 77, 110), so the new statement resolves
  • Root tsconfig.json includes only src/**/*.ts, and tsconfig.tests.json:3 matches *.ts, so the new .mts consumer file is not typechecked in-repo
  • scripts/check-exports-documented.mjs only requires reference.md to name the built exports; the added doc lines name both types

6bfed00 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

@coderabbitai

coderabbitai Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 35259acf-7032-480c-9fc3-957cff8e1956
📥 Commits

Reviewing files that changed from the base of the PR and between ec0a12f and 6bfed00.

📒 Files selected for processing: 5
  • .changeset/export-param-types.md
  • docs/reference.md
  • scripts/check-package.ts
  • src/core.ts
  • tests/package-consumer-declarations.mts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

The package now exports and documents ParamEntry and ParamsIn. A package-consumer fixture and declaration-emission check cover the exported types with the installed TypeScript compiler and TypeScript 5.

Changes: Parameter Type Exports

Layer / File(s) Summary
Expose parameter types
src/core.ts, docs/reference.md, .changeset/export-param-types.md
src/core.ts re-exports ParamEntry and ParamsIn. The reference documents their roles in parameter typing and generated declarations. The changeset describes the exports and declaration checks.
Check consumer declarations
tests/package-consumer-declarations.mts, scripts/check-package.ts
The fixture exports ClickHouse and Postgres tables and queries, plus a ClickHouse custom function. The package check emits declarations from the fixture with the installed TypeScript compiler and TypeScript 5.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 6bfed

No actionable issue remains from this review; the change is mergeable after normal checks.

Pre-merge checks: Passed 5
Passed Passed checks: 5 passed
Check name Status Explanation
Docstring Coverage Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 3…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely identifies the main change: exporting the ParamEntry and ParamsIn types.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@maple-review-bot maple-review-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 inline note from Maple's review. The score and summary are in the review comment above.

Comment thread src/core.ts Outdated
A query whose filter uses `when` / `whenTrue`, or a param named by a
union, carries `OptionalParams<…>` (expanding to `OptionalParamEntry`),
which an exported query's declarations failed to name with TS4023 just
like `ParamEntry`. Both are public and documented now.

The declaration-emit consumer gains optional and union-named params,
plus a join, a CTE, a subquery and a union over parameterized queries;
probing those found no other unexported type.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@maple-review-bot

maple-review-bot Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Maple review

🟢 Confidence 10/10 · safe to merge
Type-only re-exports; OptionalParamEntry/OptionalParams are public at src/core.ts:102 and reach both dialect entries through export * from "./core".
quality 100/100 · no findings · tests covered · risk low

Exports ParamEntry, OptionalParamEntry, OptionalParams, ParamsIn and ParamsSatisfied from core, so declaration emit for exported param queries and defineFn helpers can name them. Safe to merge.

  • core.ts re-exports the five param types; dialect entries pick them up via export * from "./core"
  • test:package now compiles a declaration-emitting consumer covering when, whenTrue, union-named params, joins, CTEs, subqueries, unions

Fixed since the last review

  • ✅ F1 · when queries still fail declaration emit: OptionalParamEntry unexported
What was checked
  • OptionalParamEntry and OptionalParams are exported (src/core.ts:102), so the earlier when/whenTrue TS4023 is gone
  • No duplicate export of these names in core.ts or the dialect barrels (src/clickhouse.ts:7, src/postgres.ts:16)

56a2893 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

@Makisuo
Makisuo merged commit 5328b71 into main Oct 14, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant