fix(codegen-ts): finish timestampMode:"date" — wire coercion, sqlite guard, view schemas - #283
Merged
Merged
Conversation
…ublish review #281 fixed four codegen sites that ignored timestampMode: "date", closing a compile failure an adopter hit, but a pre-publish review (verified by EXECUTING generated Zod, not reading emitted text) found the mode still half-fixed on three paths, one a regression #281 itself introduced. CRITICAL 1 — z.date() rejected every JSON wire value. The three @autoset sites and zodFieldExpr's timestamp case emitted z.date() in date mode, but these schemas validate raw JSON request bodies (ISO strings on the wire), not live Date instances — z.date().safeParse(isoString) is false. Fixed to z.coerce.date(), which also passes a driver-returned Date through unchanged (the TPH read-schema case) and still short-circuits null via .nullable() (FR-035 present-null clearing). CRITICAL 2 — sqlite/D1 + date mode emitted non-compiling code, a regression from #281: its new ternaries key on timestampMode alone, never dialect, but column-mapper's sqlite branch ignores the mode entirely (Drizzle's sqlite-core text() has no Date-typed mode). Fixed at the one choke point: normalizeConfig and makeRenderContext now normalize timestampMode to "string" whenever dialect === "sqlite", so the combination is a safe no-op instead of silently broken output. Documented as Postgres-only on the config option. CRITICAL 3 — a fifth timestamp-Zod emitter (zodTypeFor, via view-decl.ts's renderViewReadZodObject) was missed by #281's sweep and still hardcoded z.string() for a "date"-mode view/projection read schema, disagreeing with the Date-typed Drizzle column on any write-through entity or projection carrying a timestamp. zodTypeFor now takes an optional timestampMode parameter (default "string", byte-identical when omitted). IMPORTANT 5 (assessed while fixing Critical 1) — a value-object-hosted timestamp must stay z.string() even in date mode: VO storage is inherently ISO-string jsonb, and the VO structural interface (inferred-types.ts's SCALAR_TS_BY_SUBTYPE) is deliberately not mode-aware. zodFieldExpr now excludes object.value-hosted fields from the mode. IMPORTANT 4 (assessed, documented rather than fixed) — runtime-ts's filter parser keeps a timestamp filter value a string unconditionally, which throws against a Date-mode Drizzle column at request time. A correct fix needs timestampMode threaded from codegen into the generated allowlist or mount options across two runtime packages (drizzle-fastify + hono) and two codegen sites — more invasive than the rest of this fix. Documented as a known limitation on the config option and at the crash site; not enforced at codegen time. Testing: every #281 test asserted on emitted source text; this repo has a named precedent for that exact failure mode (0.20.6, z.string().ip()) hiding a breakage until a test executed the schema. Added timestamp-mode-execution.test.ts, which writes real rendered output to a temp .ts file and dynamically imports it, then calls the real safeParse/parse on the real Zod object — covering all three criticals plus the VO exclusion. Default ("string") mode output is unchanged — the golden suite (codegen-ts/test/golden) shows zero diff. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KTGT5ksntpcJDZVJ5VyXHS
…estamps (Important 4 follow-up) Reviewer-requested addition on top of the timestampMode "date" fix: the Important-4 gap (filtering a date-mode timestamp throws at request time in runtime-ts's filter parser) was documented-not-fixed because the real fix needs runtime threading across two packages. That's still true, but the crash was learned from a production 500, not from the build — and this repo's own precedent (#226/#258) is detect-and-refuse/warn at generation time instead. runGen now warns (never refuses — date mode is otherwise usable) once per run, after config normalization, whenever timestampMode === "date" and any non-abstract entity has a @filterable field.timestamp — naming every offending entity+field in one line, not one per field/entity. Reading the mode post-normalizeConfig means this is naturally silent on sqlite/D1 (normalized to "string" there, per Critical 2) and in the default "string" mode, with no extra dialect/mode branching needed. Uses the existing `warnings: string[]` mechanism every other runGen diagnostic already uses — no new channel. Added 4 tests to timestamp-mode-execution.test.ts: fires once naming both offenders (and never a non-filterable timestamp) under postgres + date mode; silent in string mode; silent with no filterable timestamp; silent under sqlite even with timestampMode: "date" requested (ties to Critical 2's normalization). Cross-referenced the warning from both the timestampMode config-option doc comment and the filter-parser.ts limitation note. Default-mode output and diagnostics remain byte-identical — golden suite shows zero diff. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KTGT5ksntpcJDZVJ5VyXHS
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.
Completes
timestampMode: "date", which #281 left half-fixed. Found by a pre-publish review that verified by executing Zod rather than reading emitted text — the same gap that let #281 ship incomplete.All three findings were Critical: each would have shipped date mode in a knowably broken state to four immutable registries.
Critical 1 —
z.date()rejected every JSON wire valueThe Insert/Update schemas validate raw request bodies (
mountCrudRoutes→safeParse(req.body)), where timestamps arrive as ISO strings.z.date().safeParse("2026-08-08T10:00:00.000Z")isfalse, so date mode would have gone from "doesn't compile" to "compiles, then 400s every write" — and React form saves would have failed silently, the #227 pattern.Now
z.coerce.date()at all four sites: accepts wire ISO strings anddatetime-localvalues, passes DB-driverDates through unchanged, and.nullable()still short-circuitsnullso FR-035 present-null clearing is safe.Critical 2 — sqlite/D1 + date mode emitted non-compiling code (regression from #281)
column-mapper's sqlite branch ignorestimestampMode(baretext), but #281's ternaries keyed on the mode alone. Result:text("created_at").$defaultFn(() => new Date())— TS2322 in the generated schema itself. That combination compiled before #281; the fix created the bug on the other dialect.timestampModenow normalizes to"string"whendialect === "sqlite"at both choke points (normalizeConfigandmakeRenderContext), so the option is a documented safe no-op there rather than a trap. D1 is covered — a D1 project's codegen dialect issqlite.Critical 3 — a fifth timestamp-Zod emitter was missed
zodTypeFor(viaview-decl.ts) hardcodedz.string(), so a write-through entity or projection carrying a timestamp kept the original TS2322 cascade — the exact failure #281 set out to fix. Now mode-aware for TIMESTAMP only, and it threads the same opts object that types the view column, so the two cannot diverge again by construction.field.date/field.timedeliberately stayz.string()— verified against Drizzle:date()is string-typed andtime()has no mode, so they genuinely aren't governed bytimestampMode.Also
@filterabletimestamp meets date mode. Filtering such a column throws at request time inruntime-ts's filter parser; threading the mode through the runtime is genuinely invasive, so this surfaces it at build time instead of a production 500 — matching the migrate --dialect d1: the sqlite table-rebuild's PRAGMA foreign_keys OFF is a no-op on remote D1, so any parent-table rebuild fails with FOREIGN KEY constraint failed #226/migrate: no primary-key change kind, so moving a table's PK leaves it with none and every referencing FK fails #258 detect-at-generation precedent.Testing — the gap that caused this
Every #281 test asserted on emitted source text; none executed a schema. This repo has a named precedent for that exact failure mode (0.20.6,
z.string().ip()).timestamp-mode-execution.test.tswrites real rendered output to a temp file, dynamicallyimport()s it, and calls realsafeParseon the real Zod object — including negative cases and thenull-clearing path.codegen-ts1048 pass / 0 fail · golden diff zero (default string mode byte-identical).