Make the published types portable across package instances - #6200
Draft
candrewlee14 wants to merge 6 commits into
Draft
Make the published types portable across package instances#6200candrewlee14 wants to merge 6 commits into
candrewlee14 wants to merge 6 commits into
Conversation
This was referenced Aug 29, 2026
candrewlee14
marked this pull request as draft
August 29, 2026 18:19
This was referenced Aug 29, 2026
candrewlee14
force-pushed
the
sql-types-portable
branch
2 times, most recently
from
August 30, 2026 01:56
06a6f67 to
ac5e657
Compare
Strip implementation-only query-builder details from published declarations, infer decoder results structurally, and compile two physical package copies to prevent regressions.
The single SQL assignment only guarded one type. Replace it with a table of public types probed in one tsc program, asserting both directions: a portable type regressing fails, and a known leak becoming portable fails too, prompting the flag to be flipped. Records the measured state of the surface, annotating each known leak with the private/protected member responsible. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qd99Mmmi1fZkKQiV3u1jXy
Name#brand, Param#brand and CodecsCollection#resolveTypes are implementation details that reached the emitted .d.ts as protected members, making all three types nominal and so non-portable between two installs of the package. resolveTypes is declared as a field rather than left as a constructor parameter property: on a parameter the @internal marker survives into the emitted constructor signature as a stray comment. Flips Param, Name and CodecsCollection to portable in the matrix. The three dialects do not follow -- with the nominal blocker gone they fail structurally instead, on the anonymous recursive BuildRelationalQueryResult['selection'], which is recorded as their blocker. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qd99Mmmi1fZkKQiV3u1jXy
PgColumn#toBuilder returns a PgColumnBuilder, so the builder's private and protected members are part of PgColumn's published shape, and PgTable's _.columns index signature carries them up to the table. One private field was therefore enough to make every pg table and column type non-portable between two installs. config follows its base: ColumnBuilder#config is already @internal and the pg override had not inherited the marker. The other dialects declare the same foreignKeyConfigs field but are unaffected, since only pg-core exposes toBuilder() in a public return type. Flips PgTable and PgColumn to portable. NodePgDatabase stays recorded as a known leak: its chain runs through the session, transaction and relational query builder classes and does not converge to a contained change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qd99Mmmi1fZkKQiV3u1jXy
The view classes leaked two kinds of nominal member into the emitted .d.ts: Symbol.for-keyed config properties ([PgViewConfig], [MySqlViewConfig] and the materialized/dialect equivalents). A Symbol.for key is a distinct computed key per declaration site, so copy A's PgView carries a property copy B's does not, even though Symbol.for guarantees the runtime symbol is shared. Table already marks every Symbol.for key it declares @internal, which is why the table and column types were portable while the views were not; this applies the same treatment. Protected brand members ($MySqlViewBrand and friends), which exist only to keep the view classes distinct at type level. Flips PgView, MySqlView and SQLiteView to portable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qd99Mmmi1fZkKQiV3u1jXy
candrewlee14
force-pushed
the
sql-types-portable
branch
from
August 30, 2026 02:06
ac5e657 to
204d654
Compare
…tions The dialect classes emitted their private query-building methods into the published .d.ts as `private buildSelection;` and friends, which makes the class nominal and so non-portable between two installs. AggregatedField#table is the same bug one level down. The union inside BuildRelationalQueryResult['selection'] names AggregatedField directly, so a protected member there makes that class nominal, which makes the recursive selection type non-portable, which propagates out through every dialect's mapperGenerators. The diagnostic surfaces at the recursive type rather than at the member, which makes it easy to mistake for a compiler limitation. This does not yet flip the dialect rows: behind these members they come to rest on EmptyFilter, whose unique symbol type is nominal per declaration site. That is an API decision rather than a marker, and is made separately. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qd99Mmmi1fZkKQiV3u1jXy
candrewlee14
force-pushed
the
sql-types-portable
branch
from
August 30, 2026 02:46
204d654 to
ce9f229
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Two copies of
drizzle-ormon disk produce two declaration sites for every class, and TypeScript compares classes carryingprivate/protectedmembers nominally — the members must originate from the same declaration. So any such member surviving into the emitted.d.tsmakes its type non-portable: a value from copy A is not assignable to the same type from copy B.That situation is routine — a transitive dep pinning a different version, a pnpm peer split, a linked tarball — and it produces the maximally unhelpful error, where both sides print identically:
This strips those members from the published declarations. The build already sets
stripInternal: true, so/** @internal */is all that is needed; nothing observable is lost, because none of it was callable.Commits
Each is self-contained and moves the measured surface:
SQL—collectSQL,mapInlineParam, andBuildQueryConfig.codecs(which reachedCodecsCollection'sprotected resolveTypes), plusGetDecoderResultmatching on shape rather than naming theColumnclass.Name#brand,Param#brand,CodecsCollection#resolveTypes.PgColumnBuilderinternals — reachable from every pg table and column viaPgColumn#toBuilder().Symbol.for-keyed configs and brand members.AggregatedField#table—buildSelection,buildJoins,buildWithCTEand friends, plus one member a level down.Every change here is load-bearing
I checked rather than assumed: each was reverted individually against the packed artifact, rebuilt, and re-measured. Two results worth stating, because both contradicted what I expected:
GetDecoderResultlooked like an optional tidy-up. It is not. Naming theColumnclass in that conditional makes it depend on relating one install'sColumnto another's, and reverting it regressesColumn,CodecsCollection,Name, and the mysql table/column/dialect types — six in total.BuildQueryConfig.codecsis the one marker that becomes redundant later, onceCodecsCollection#resolveTypesis stripped in commit 3. It is still required at commit 1 forSQLto be portable on its own, so it stays, with a comment saying as much.AggregatedField#tableis worth a second lookThe union inside
BuildRelationalQueryResult['selection']namesAggregatedFielddirectly. Aprotectedmember on that class makes it nominal, which makes the whole recursiveselectiontype non-portable, which propagates out through every dialect'smapperGenerators.The diagnostic surfaces at the recursive type rather than at the member, and reads like a compiler recursion limit. It is not — it is one leaked member, and stripping it fixes the whole chain. I flag it because I initially misdiagnosed it that way myself.
The
Symbol.forkeys are worth a second lookPgViewdeclared[PgViewConfig]unmarked. ASymbol.forkey is a distinct computed key per declaration site, so copy A'sPgViewcarries a property copy B's does not — even thoughSymbol.forguarantees the runtime symbol is shared. The type contradicted the runtime.This is already handled correctly elsewhere: every
Symbol.forkeyTabledeclares is@internaland stripped, which is exactly why the table and column types were portable while the views were not.Result
The matrix now probes 30 types and covers every file this change touches — including the cockroach, mssql and singlestore dialects and views, which were previously edited but unguarded by any probe.
Of those 30, 22 are portable after this PR. The pg/mysql/sqlite dialects stop one step short on
EmptyFilter'sunique symbol, which is an API decision and is proposed separately in #6205 — taking it to 25.NodePgDatabaseandPgSelectremain non-portableNodePgDatabase,PgSelect, and the cockroach / mssql / singlestore dialects are recorded in the matrix with the member that currently blocks each. The last three reach their session class and inherit a chain of nominal members through the prepared-query and transaction classes; I tried one round of stripping it, it did not converge, and I stopped rather than repeat a sweep that flips nothing.I chased both further than this PR goes. Past their nominal members each ends at a generic type deferred on an unresolved parameter —
BuildQueryResult<..., TConfig, ...>for the database object,CheckTableLikeSelection<TJoinedTable>for the select builder — where the compiler cannot relatekeyof XorX extends Yacross two declaration sites. I stripped every nominal member on the select path to check, and it did not flip, so those markers are not in this PR.I would not state flatly that those are unfixable, because I made exactly that call about the recursive
selectiontype above and was wrong: it turned out to be one leaked member wearing a confusing diagnostic. Treat them as unresolved, not as proven walls.Verification
Every commit was verified against a real
bun --bun run scripts/build.ts && npm run pack, since the bug lives in what is emitted, not what is written.drizzle-ormsuite green (27 files, 1344 tests) andtest:typesclean at each commit.drizzle-kitanddrizzle-seedtypecheck clean against the modified ORM.ormshard, which already provisions it.