fix: DateTime maps back to Schema.DateFromSelf (Date ↔ Date)#29
Conversation
Reverts 5.6.0 DateFromInput Union mapping. DA-layer consumers got hit with Date|string typed columns, no cast-free narrowing path. Prisma+Kysely canonical contract: DateTime columns return native Date instances on SELECT. Effect docs (Rule of Schemas, Doc 10944) prescribe one Type, one Encoded per schema; dual boundaries (DA Date↔Date vs wire string↔Date) handled with two schemas, not one Union — see @effect/sql Model.Class variants (Doc 4312). For RPC wire boundaries: contract-layer Schema.extend with Schema.Date overrides for date columns. Same pattern as @effect/sql json variants. DateFromInput is still exported from the package for explicit use at specific call sites; codegen just no longer auto-emits it. Schema.Schema.Encoded DB interface fix from 5.7.0 stays — still correct for join-table A/B column exposure. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR reverts the ChangesDateTime Schema Mapping Reversion
🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/utils/type-mappings.ts`:
- Line 34: The mapping for DateTime currently points to Schema.DateFromSelf
which violates the generator contract; change the centralized mapping so the key
DateTime maps to DateFromInput instead of Schema.DateFromSelf, and update any
related header/import generation to reference DateFromInput (ensuring
DateFromInput accepts Date | string and encodes as Date) so functions/classes
referencing DateTime use DateFromInput consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ec101a67-46b7-40ca-ae0c-a93b85409cc8
📒 Files selected for processing (4)
.changeset/datetime-back-to-datefromself.mdsrc/__tests__/kysely-native-integration.test.tssrc/effect/generator.tssrc/utils/type-mappings.ts
| Decimal: 'Schema.String', // For precision | ||
| Boolean: 'Schema.Boolean', | ||
| DateTime: 'DateFromInput', // Date | string ↔ Date — dual-boundary safe | ||
| DateTime: 'Schema.DateFromSelf', // Date ↔ Date — Prisma+Kysely canonical |
There was a problem hiding this comment.
DateTime mapping violates generator contract rule
At Line 34, mapping DateTime to Schema.DateFromSelf breaks the repository’s required generation contract for DateTime fields. Please map it back to DateFromInput in the centralized mapping (and keep header/import generation aligned accordingly).
Suggested fix
- DateTime: 'Schema.DateFromSelf', // Date ↔ Date — Prisma+Kysely canonical
+ DateTime: 'DateFromInput', // Accepts Date | string input, encodes as DateAs per coding guidelines, "DateTime fields must map to DateFromInput schema accepting Date | string input and encoding as Date".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| DateTime: 'Schema.DateFromSelf', // Date ↔ Date — Prisma+Kysely canonical | |
| DateTime: 'DateFromInput', // Accepts Date | string input, encodes as Date |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/utils/type-mappings.ts` at line 34, The mapping for DateTime currently
points to Schema.DateFromSelf which violates the generator contract; change the
centralized mapping so the key DateTime maps to DateFromInput instead of
Schema.DateFromSelf, and update any related header/import generation to
reference DateFromInput (ensuring DateFromInput accepts Date | string and
encodes as Date) so functions/classes referencing DateTime use DateFromInput
consistently.
Summary
Reverts the 5.6.0 change that mapped
DateTimetoDateFromInput(aSchema.Union(DateFromSelf, Date)withEncoded = Date | string). DA-layer consumers now seeSelectable<X>.created_at: Dateagain — matching what Kysely's pg driver actually returns.Why
created_at: ColumnType<Date, string | undefined, never>— SELECT yieldsDate. "TypeScript is a compile-time concept and cannot alter runtime JavaScript types."@effect/sqlModel.Classvariants pattern, Doc 4312), not a single Union.The
DateFromInputUnion approach pushed Date|string typed columns onto every DA mapper. No cast-free narrowing path. Wrapping reads inSchema.decodewas a hundreds-of-sites refactor.What stays from earlier sprint
Schema.Schema.Encodedfor DB interface) stays. Still correct — fixes the join-table_product_tags.A/Bcolumn exposure bug.DateFromInputis still exported from the package. Consumers that explicitly want the dual-input behavior at a specific call site can still import it. Codegen just no longer auto-emits it for every DateTime column.For RPC/HTTP wire boundaries
Define a contract-layer schema that overrides date columns with
Schema.Date(Encoded = string) before the RPC framework callsSchema.decode. Same pattern as@effect/sql'sjsonvariants — one schema per boundary.Test plan
bun run prepublishOnly— lint + typecheck + 347 tests + build, all greenkysely-native-integration.test.tsto assertSchema.DateFromSelf(wasDateFromInput)RpcDateOverridesto product RPC contract for wire decode🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Documentation