fix(codegen-ts): projection read type mirrors view-column nullability#81
Merged
Merged
Conversation
… + cross-port compile guards A non-required (nullable) projection field generated a nullable Drizzle view column but a NON-null Zod read type, so the generated projection query returned `T | null` into a non-null `<Name>` field and failed to compile under strict TS (the likely source of the "null is not assignable to number" friction seen building real apps). Fix: the projection Zod read schema now appends `.nullable()` whenever the view column is not `.notNull()`, so the read type matches what `db.select().from(view)` actually yields (and is more correct — a nullable derived field is typed nullable). Closes the cross-port test gap that let the original PR #80 projection bug ship: only C# compiled generated projection code. Adds a real compile/structure guard to every port that lacked one — TS (tsc), Python (exec), Java (in-process javac of the DTO), Kotlin (kotlin-compile-testing of the data class) — each exercising a projection with a required id + a non-required derived field. Test-only for Java/Kotlin/Python (those ports were already correct); the TS change is the fix. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LuZWKnWzYGVnESijL7uuky
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.
Intent
Close the cross-port projection-codegen test gap and fix a projection nullability bug it surfaced.
THE FIX (codegen-ts, the only releasable/behavioral change): a non-required (nullable) projection field generated a nullable Drizzle view column but a non-null Zod read type, so the generated projection query returned T|null into a non-null field and failed to compile under strict TS (likely the source of 'null is not assignable to number' friction seen building real apps). projection-decl.ts now appends .nullable() to the Zod read field whenever the view column is not .notNull(), so the read type matches the view's SELECT type and is more correct. Verified: codegen-ts 826 pass, typecheck clean, no golden broke, cli/sdk unaffected.
THE TEST GAP (test-only, no behavior change): PR #80's original projection bug shipped because only the C# port compiled generated projection code. This adds a real compile/structure guard to every other port — TS (tsc via ts.createProgram), Python (exec of the generated model), Java (in-process javac of the generated DTO, reusing the existing compiler-harness pattern), Kotlin (kotlin-compile-testing of the generated data class; the Exposed table is asserted structurally since Exposed isn't on that module's test classpath — full table compile already lives in integration-tests-kotlin). Each fixture is a projection with a REQUIRED id + a NON-required derived aggregate field (the exact shape that exposed the nullability mismatch). Java/Kotlin/Python were confirmed CORRECT by a prior audit — their tests are preventive guards, not fixes. Production generators in those ports are untouched (test files only).
A prior cross-port audit confirmed TS was the only port with the original PR #80 bug; the others gate projections correctly (Java via KIND_TABLE, Kotlin/Python via subtype/source-kind, C# via IsReadOnlyProjection).
What Changed
projection-decl.tsnow appends.nullable()to a projection field's Zod read type whenever its generated Drizzle view column is not.notNull(), so a non-required derived field's read type matches the view'sSELECTtype instead of emittingT | nullinto a non-null field (previously failedtscstrict compilation, e.g. "null is not assignable to number").tscviats.createProgram), Python (exec of the generated model), Java (in-processjavacof the generated DTO), and Kotlin (kotlin-compile-testing of the generated data class) — each fixture being a projection with a requiredidplus a non-required derived aggregate field. These ports' generators are unchanged (test files only); the cross-port audit confirmed only TS carried the bug.CHANGELOG.mdanddocs/features/source-kinds.mdto document that a projection's read-type nullability mirrors its view column.Risk Assessment
✅ Low: The single behavioral change is a small, well-bounded correctness fix whose nullability invariant is guaranteed consistent with the emitted view column by construction (both derive from the same mapColumnType modifiers), and the remaining changes are preventive test-only guards.
Testing
Exercised the fix at the product level by compiling actual generated projection code with each port's real compiler. The TS compile guard passes with the fix and, when I reverted the one-line change, failed with the exact
null is not assignable to numbererror the fix targets — confirming both the bug and the fix. Captured the generated projection code showing the nullable view column now matched by a.nullable()Zod read type. The Python/Java/Kotlin preventive compile guards all pass, and the codegen-ts suite is green (826). No visual/UI surface is involved — this is codegen output verified via compilation and rendered generated source.Evidence: TS compile guard FAILS without the fix (proves it catches the bug)
Type '{ id: number; weekCount: number | null; }' is not assignable to type '{ id: number; weekCount: number; }'. Types of property 'weekCount' are incompatible. Type 'number | null' is not assignable to type 'number'. Type 'null' is not assignable to type 'number'. (fail) projection codegen compiles (TS2724 regression guard)Evidence: TS compile guard PASSES with the fix
1 pass 0 fail Ran 1 test across 1 file.Evidence: Generated projection code: nullable view column now matched by .nullable() Zod read type
id: integer("id").notNull(), weekCount: integer("week_count"), }).existing(); export const ProgramSummarySchema = z.object({ id: z.number().int(), weekCount: z.number().int().nullable(), });Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
server/typescript/packages/codegen-ts/src/templates/projection-decl.ts:145- mapColumnType(f, dialect, columnNamingStrategy, timestampMode) is now invoked twice per field: once in the zodLines map (line 145) purely to read .modifiers for nullability, and again in the viewColumnLines map (line 166) for the full spec. The two passes iterate the same allFields. Computing the spec once per field (e.g. a single pass building both, or a precomputed specs array) would remove the duplicate work and make the nullability/notNull coupling explicit rather than two independent calls that happen to agree. Non-functional; current behavior is correct because both calls are deterministic on the same inputs.server/java/codegen-spring/src/test/java/com/metaobjects/generator/spring/SpringProjectionDtoCompileRunTest.java:174- The StandardFileManager obtained via javac.getStandardFileManager(...) is never closed in compile(), a minor resource leak in the test harness. Wrapping it in try-with-resources (it is Closeable) would mirror the Files.walk stream that is already closed properly. Test-only, no production impact.✅ **Test** - passed
✅ No issues found.
cd server/typescript && bun test packages/codegen-ts/test/projection/compile.test.ts(with fix → 1 pass)Revertedprojection-decl.tsto base commit and re-ran the compile test → fails withType 'null' is not assignable to type 'number'onweekCount, then restored the fix (worktree clean)Generated the ProgramSummary projection entity+queries to inspect output: nullable view columnweekCount: integer("week_count")now paired withweekCount: z.number().int().nullable()read typebun test packages/codegen-ts/test/projection/(54 pass) and fullbun test packages/codegen-ts/(826 pass)cd server/python && uv run python -m pytest tests/codegen/test_projection_compile.py -v(2 pass)cd server/java && mvn -pl codegen-spring -am test -Dtest=SpringProjectionDtoCompileRunTest(1 pass, in-process javac of DTO)cd server/java && mvn -pl codegen-kotlin -am test -Dtest=KotlinProjectionCompileTest(2 pass, kotlin-compile-testing of generated data class)✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.