Skip to content

Type EmptyFilter as a string literal instead of a unique symbol - #6205

Draft
candrewlee14 wants to merge 7 commits into
drizzle-team:rc5from
candrewlee14:empty-filter-string-literal
Draft

Type EmptyFilter as a string literal instead of a unique symbol#6205
candrewlee14 wants to merge 7 commits into
drizzle-team:rc5from
candrewlee14:empty-filter-string-literal

Conversation

@candrewlee14

@candrewlee14 candrewlee14 commented Aug 29, 2026

Copy link
Copy Markdown

Stacked on #6200 — it contains that PR's commits until it lands. Review only the last commit, Type EmptyFilter as a string literal.

Separate from #6200 because it is the one change that touches observable API. That call is yours; #6200 stands on its own without it.

What

-export const EmptyFilter = Symbol.for('drizzle:EmptyFilter');
+export const EmptyFilter = 'drizzle:EmptyFilter';
 export type EmptyFilter = typeof EmptyFilter;

Why

#6200 strips every nominal member from the dialects — including AggregatedField#table, which is what the recursive selection type was actually tripping over — and all three then come to rest here. I verified this is still load-bearing after that: with #6200 applied and this change reverted, all three dialects fail again on Type 'unique symbol' is not assignable to type 'AnyTableFilter | unique symbol'.

Symbol.for returns the same symbol in every copy of the package on disk, so the value was already portable at runtime — but its inferred type is unique symbol, which is nominal per declaration site. The type contradicted the runtime, and every public type mentioning EmptyFilter stopped being assignable across installs.

Why a string literal specifically

The filter builder relies on unit-ness:

if (processed === EmptyFilter) continue;
parts.push(processed);   // <- requires EmptyFilter to be narrowed out of the union

Only unit types narrow under ===. I tried the obvious structural alternative first — a branded symbol & { readonly [Brand]: true } — and it fails to compile at exactly that line: structural, but not a unit type, so the guard stops narrowing. A string literal is the only type that is both, so it is the only option that keeps the narrowing and gains the portability.

Trade-off, explicitly

This changes an exported runtime value from a symbol to a string. Identity comparison (where === EmptyFilter, where: EmptyFilter) is unaffected, and EmptyFilter is confined to relations.ts internally — but it is observable: typeof EmptyFilter === 'symbol', or reliance on symbol non-enumerability when serialising a filter, would notice.

If that is unacceptable, drop this PR. It costs the three dialect rows and nothing else.

Result

PgDialect, MySqlDialect and SQLiteDialect flip to portable — 22 of 30 to 25 of 30 on the matrix.

The cockroach, mssql and singlestore dialects need this change too, but do not flip: they reach their session class and are blocked behind a further chain of nominal members. They stay recorded rather than half-fixed.

Verification

Full drizzle-orm suite green (27 files, 1344 tests); test:types clean, including the narrowing site above.

@candrewlee14
candrewlee14 force-pushed the empty-filter-string-literal branch from 056fec0 to 025ae9f Compare August 29, 2026 18:46
@candrewlee14
candrewlee14 force-pushed the empty-filter-string-literal branch from 025ae9f to 0d0e354 Compare August 29, 2026 18:53
@candrewlee14 candrewlee14 changed the title Type EmptyFilter as a string literal instead of a unique symbol Make the dialects portable across package instances Aug 29, 2026
@candrewlee14
candrewlee14 force-pushed the empty-filter-string-literal branch from 0d0e354 to d783ee4 Compare August 29, 2026 19:01
@candrewlee14 candrewlee14 changed the title Make the dialects portable across package instances Type EmptyFilter as a string literal instead of a unique symbol Aug 29, 2026
@candrewlee14
candrewlee14 force-pushed the empty-filter-string-literal branch 2 times, most recently from 2b3e7be to 1063059 Compare August 30, 2026 01:56
cursoragent and others added 5 commits August 29, 2026 20:03
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
candrewlee14 force-pushed the empty-filter-string-literal branch from 1063059 to ac30f25 Compare August 30, 2026 02:06
candrewlee14 and others added 2 commits August 29, 2026 20:44
…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
Symbol.for returns the same symbol in every copy of the package on disk, so the
value was already portable at runtime -- but the inferred unique symbol type is
nominal per declaration site, so the type disagreed with the runtime, and every
public type mentioning EmptyFilter stopped being assignable across installs.

A string literal is the only structural unit type. Unit-ness is what the filter
builder relies on: `if (processed === EmptyFilter) continue` has to narrow the
union afterwards. A branded `symbol & {...}` intersection is structural but not
a unit type, so it strips the narrowing and does not compile. A literal keeps
both properties.

Flips PgDialect, MySqlDialect and SQLiteDialect. The cockroach, mssql and
singlestore dialects need this too, but are blocked behind a further chain of
nominal members through their session classes and stay recorded as such.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Qd99Mmmi1fZkKQiV3u1jXy
@candrewlee14
candrewlee14 force-pushed the empty-filter-string-literal branch from ac30f25 to 28f7f92 Compare August 30, 2026 02:46
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.

2 participants