Skip to content

fix: make codegen output deterministic - #1

Open
typedrat wants to merge 10 commits into
betafrom
typedrat/deterministic-codegen
Open

fix: make codegen output deterministic#1
typedrat wants to merge 10 commits into
betafrom
typedrat/deterministic-codegen

Conversation

@typedrat

Copy link
Copy Markdown
Member

Regenerating from an unchanged Drizzle schema doesn't reliably produce an unchanged file. The worst case I found:

Move the WY entry to the top of the usStates object literal in db/drizzle/country.ts. It's a key reorder, in a file that declares no tables. Regenerate:

1717a1718        5955a5956        6935a6936
>         | "WY"  >    | "WY"     >    | "WY"
1742d1742        5980d5980        6960d6960
<         | "WY"  <    | "WY"     <    | "WY"

Three columns, three unrelated tables.

Custom types reach the output as text printed by the TypeScript checker, and TypeScript orders union members by internal type id — assigned when each literal type is first created anywhere in the program. So the printed order encodes the order the whole program got checked, and editing any file that mentions those literals can shuffle unions in tables that have nothing to do with it.

TS 6 has a stableTypeOrdering flag that would fix this at the source, but the checker doing this work is the one ts-morph bundles (5.9.2), not the project's typescript (6.0.3). I wired the option through and confirmed it reaches program.getCompilerOptions() and changes nothing. So ef0c08d sorts the members after printing instead. Same for intersections and object members; tuples are positional and left alone, and anything the parser chokes on passes through untouched.

Ordering

Nothing else was sorted either, so table, column and relationship key order was just the order things happened to be written in. 68a9815 adds a canonicalization pass over tables, columns, relationship owners and relation names, applied both when drizzleZeroConfig builds a schema and again on the way into codegen, so a hand-written schema canonicalizes too. It's idempotent.

Deliberately not sorted: primaryKey, the hops of a relationship, and the sourceField/destField arrays inside a hop, which pair up by position. normalizeClientSchema already sorts tables and columns before hashing the client schema, so those orders are provably invariant. primaryKey isn't — it feeds row-key strings, and primaryKey({columns: [b, a]}) is a real distinction.

Identifier allocation is the same problem. Names used to be handed out with a positional counter, so reordering the schema swapped userProfileTable and userProfileTable2 between two tables. 9bee7bd allocates from the whole key set at once: unique preferred name wins it outright, and when several keys want the same name they all take a suffix derived from their own key.

Bugs

Each has a test that fails on beta and passes here.

  • 9bee7bd — table row types skipped the uniquifier entirely. user and users both emitted export type User; a table named schema emitted a second export type Schema. Both are invalid TypeScript. A table named row shadowed the imported Row.
  • b5ff4f6writeValue matched customType, enableLegacyMutators and enableLegacyQueries by key name at any depth, and read the owning table and column from hard-coded keys[1]/keys[3]. A column named enableLegacyMutators had its entire definition replaced by a boolean.
  • c3fd62dgetZeroSchemaDefsFromConfig looked the config up by base name. ts-morph turns that into a "path ends with" search, so a project with two drizzle-zero.config.ts files picked the wrong one and generated its import and typeof zeroSchema against an unrelated module.
  • 853040a — three .find() lookups took whichever candidate came first, and the column lookup pushed a miss through a ! so an unresolvable column landed in the output as null. The two column lookups also disagreed with each other about duplicate column names, one taking the first declaration and one the last.
  • dde5dd9 — the already-warned-about set was module scoped and never cleared, so only the first schema built in a process warned at all.
  • d511d13camelcase case-maps with the host locale unless told not to, so an I in a key lowercases to a dotless ı under tr. V8 currently treats the undefined locale as root, which is why nobody's hit it, but ECMA-402 says host default.
  • 57b772bformatSchema had module loading, config resolution and formatting under one catch and reported anything that threw as "prettier not found". An unreadable .prettierrc quietly produced an unformatted file, and since the signature covers the formatted text, one whose signature didn't match any correct run.

The fixture diff

9e0d5ea regenerates both checked-in schemas and it's about 13,700 lines. It's all reordering — 6852 insertions against 6852 deletions, and sorting the lines of both versions leaves a delta of 36, every one of them a sorted union or object member. Probably not worth reading line by line.

Testing

735 unit tests, tsc --noEmit, oxlint, prettier --check and pnpm build are clean. Both integration fixtures typecheck against their consumers, and running the generator twice is byte-identical.

The Docker integration suite fails, 9 failed and 1 skipped. I ran it on unmodified beta and got the same result, so it's not from these changes. Everything times out in getNewZero(); db/test-utils.ts:1711 pins the server image to rocicorp/zero:1.7.0-canary.3 against a 1.8.0 client, which would explain it, but I haven't chased it down.

On ts-morph

Leaving the ^27.0.2 range alone. It's a runtime dependency and the bundled compiler decides how custom types resolve, so a transitive bump can move generated output with no input change — pinning it seems right in principle.

I tried 28 (bundles TS 6.0.2, has stableTypeOrdering). No API breakage and all 734 runtime tests pass, but it silently disables custom type resolution: every column falls back to a ZeroCustomType<…> alias, ~2000 extra lines in the integration fixture, and ReadonlyJSONValue stops resolving. It also puts 14 type errors in the existing suite, all of them CustomType<…> collapsing to unknown. Not TS 6 semantics — the project's own 6.0.3 handles those types fine on ts-morph 27 — and not the ts re-export, which I swapped for import * as ts from 'typescript' with no change. Looks like ts-morph 28's bundled declarations landing in the program break the conditional resolution, and any import type {Project} drags them in. Wanted a separate look, not this PR.

`writeValue` matched `customType`, `enableLegacyMutators` and
`enableLegacyQueries` by key name at any depth, and read the owning table
and column from hard-coded `keys[1]`/`keys[3]` offsets. A column named
`enableLegacyMutators` had its whole definition replaced by a boolean, and
any `customType` key nested four levels deep elsewhere in the schema was
rewritten using whatever happened to sit at those offsets.

Match on the full path instead: `customType` only under
tables/<table>/columns/<column>, and the legacy flags only at the schema
root.
`getZeroSchemaDefsFromConfig` reduced the config path to its base name
before calling `tsProject.getSourceFile`. ts-morph treats a bare file name
as a "path ends with" search and returns the first match ordered by
directory depth, so a project holding more than one `drizzle-zero.config.ts`
resolved to the wrong file and generated its import and `typeof zeroSchema`
expression against an unrelated module.

The absolute path is already in hand, so pass it through. This also drops a
`lastIndexOf('/')` that never matched on Windows-style paths.
Generated names were handed out incrementally with a positional counter, and
two namespaces were not policed at all.

Table row types bypassed the uniquifier entirely, so `user` and `users` both
emitted `export type User` and a table named `schema` emitted a second
`export type Schema` - both invalid TypeScript. A table named `row` shadowed
the imported `Row`. Where the counter did apply, the suffix fell to whichever
key came second, so reordering the schema swapped `userProfileTable` and
`userProfileTable2` between two tables.

Allocate every identifier up front from the full set of keys. A key whose
preferred name is unique and unreserved keeps it; when several keys want the
same name they all take a suffix derived from their own key, so no name
depends on a key's position relative to its neighbours. Row types and custom
type aliases now share one type-namespace allocation, table and relationship
consts another, and both treat the names the file declares or imports as
reserved.
Drizzle discards the schema key when it builds relations, so drizzle-zero
recovers it by matching on the database column name. Three lookups did that
with `.find`, taking whichever candidate the schema happened to export or
declare first, and the column lookup laundered a miss through a non-null
assertion so an unresolvable column silently reached the output as `null`.

The two column lookups also disagreed with each other: `createZeroTableBuilder`
built its map with `new Map(...)`, taking the last declaration of a duplicated
column name, while `getDrizzleColumnKeyFromColumnName` took the first.

Share one map between them, resolve every ambiguous match to the smallest
matching key so export and declaration order cannot change the answer, and
throw on a column that no key declares.
The set tracking which columns had already been warned about lived at module
scope and was never cleared, so only the first schema built in a process
reported anything. Anything reusing the API - a watch mode, a test run, a
programmatic caller - silently lost the warnings from every build after the
first.

Scope the set to one `drizzleZeroConfig` call, which is the granularity the
dedupe was reaching for.
`camelcase` case-maps with the host locale unless told otherwise, so under a
Turkish or Azeri locale an `I` in a table or column key lowercases to a
dotless `ı` and the generated identifier changes with the machine the
generator runs on.

V8 currently treats the `undefined` locale as the root locale, which is why
this has not bitten in practice, but ECMA-402 specifies the host default and
`toLocaleLowerCase([])` already follows it. Pass `locale: false` rather than
rely on that.
`formatSchema` wrapped module loading, config resolution and formatting in
one `catch`, and reported anything that threw as "prettier not found" before
returning the unformatted schema. An unreadable `.prettierrc` therefore
produced a silently unformatted file - and, because the signature is computed
over the formatted text, one whose signature differed from every correctly
formatted run.

Only fall back when prettier is genuinely absent, and let a failing config or
formatter surface. `formatSchema` and `loadPrettier` move to `cli/format.ts`
so they can be tested without `cli/index.ts` parsing argv on import.
Nothing about the generated schema was sorted, so table, column and
relationship key order was whatever order the Drizzle schema happened to be
written in. Moving a table between two exports, or a column between two
lines, rewrote the generated file even though neither changes what the schema
means - `normalizeClientSchema` sorts tables and columns before hashing the
client schema, and every other consumer looks entries up by name.

Sort tables, columns, relationship owners and relation names in one pass,
applied both when `drizzleZeroConfig` builds a schema and again on the way
into codegen, so an externally produced schema generates a canonical file
too. The pass is idempotent.

Ordering that does carry meaning is left alone: a table's `primaryKey`, the
hops of a relationship, and the `sourceField`/`destField` arrays inside a hop,
which pair up by position.
Custom types reach the generated schema as text printed by the TypeScript
checker, and the checker orders union and intersection members by the id each
type was assigned when it was first created anywhere in the program. The
printed order therefore encodes the order the whole program was checked in
rather than anything about the type.

Moving one entry to the front of a lookup table in `db/drizzle/country.ts` -
a plain key reorder, in a file that declares no Drizzle table - shifted a
member in three columns across three unrelated tables. The ids also differ
between TypeScript versions, and the checker doing this work is the one
ts-morph bundles rather than the project's own.

Sort the members after printing, so the emitted text depends only on the set
of members. Object members are sorted too; tuple elements are positional and
left in place, and anything the printer emits that this cannot parse is
passed through untouched.
Columns are now emitted in sorted order and resolved union members are
sorted by their printed form, so both checked-in schemas move. The change is
entirely reordering - the line multiset differs only where sorting a union
changed which member ends up last, and both fixtures still typecheck against
their consumers.
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