-
Notifications
You must be signed in to change notification settings - Fork 1
fix: DateTime maps back to Schema.DateFromSelf (Date ↔ Date) #29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| --- | ||
| "prisma-effect-kysely": minor | ||
| --- | ||
|
|
||
| fix: DateTime maps back to `Schema.DateFromSelf` (Date ↔ Date) — Prisma+Kysely canonical | ||
|
|
||
| Reverts the 5.6.0 change that mapped `DateTime` to `DateFromInput` (a | ||
| `Schema.Union(DateFromSelf, Date)` with `Encoded = Date | string`). | ||
|
|
||
| **Why revert**: the dual-input Union pushed the boundary problem onto | ||
| DA-layer consumers. Kysely's pg driver returns native `Date` instances, | ||
| but `Selectable<X>.created_at` typed as `Date | string` forced every DA | ||
| mapper that copies `result.created_at` into a contract type to either | ||
| narrow manually (no cast-free path) or wrap the read in | ||
| `Schema.decode(Selectable(X))` (heavy refactor across hundreds of sites). | ||
|
|
||
| **Why DateFromSelf is correct**: | ||
|
|
||
| - **Prisma docs**: *"Prisma Client returns all DateTime values as native | ||
| JavaScript Date objects. ... DateTime values must be passed as Date | ||
| objects, not strings, to avoid runtime errors."* | ||
| - **Kysely docs**: idiomatic DateTime column is | ||
| `created_at: ColumnType<Date, string | undefined, never>` — SELECT | ||
| yields `Date`. *"TypeScript is a compile-time concept and cannot | ||
| alter runtime JavaScript types. If your TypeScript definition for a | ||
| column differs from the database's actual return type, the runtime | ||
| type will not change automatically."* | ||
| - **Effect Schema docs (Doc 10944)**: *"schemas should be defined such | ||
| that encode + decode return the original value"* — one Type, one | ||
| Encoded per schema. The dual-boundary problem (DA Date ↔ Date vs | ||
| RPC string ↔ Date) is solved by **two schemas** (one per boundary), | ||
| not one Union. Doc 4312 (`@effect/sql/Model.Class`) shows this | ||
| canonical variant pattern (`select`/`insert`/`update` vs | ||
| `json`/`jsonCreate`/`jsonUpdate`). | ||
|
|
||
| **For RPC/HTTP wire boundaries**: define a contract-layer schema that | ||
| overrides date columns with `Schema.Date` (Encoded = string) before the | ||
| RPC framework calls `Schema.decode`. This is the same pattern as | ||
| `@effect/sql`'s `json` variants — one schema per boundary. | ||
|
|
||
| **`DateFromInput` is still exported** from the package for consumers that | ||
| specifically want the dual-input behavior at a single call site. The | ||
| codegen just no longer auto-emits it for every DateTime column. | ||
|
|
||
| **The Schema.Schema.Encoded fix in 5.7.0 stays** — that's still correct | ||
| for join-table column exposure (`_product_tags.A`/`B`). | ||
|
|
||
| **Migration**: most consumers benefit immediately (DA mappers stop | ||
| seeing `Date | string`). For RPC contracts that previously didn't have | ||
| a Date override (because they relied on `DateFromInput`), re-add a | ||
| `Schema.extend` with `Schema.Date` overrides for date columns to keep | ||
| wire decode working. |
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
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DateTime mapping violates generator contract rule
At Line 34, mapping
DateTimetoSchema.DateFromSelfbreaks the repository’s required generation contract for DateTime fields. Please map it back toDateFromInputin the centralized mapping (and keep header/import generation aligned accordingly).Suggested fix
As per coding guidelines, "DateTime fields must map to
DateFromInputschema acceptingDate | stringinput and encoding as Date".📝 Committable suggestion
🤖 Prompt for AI Agents