chore: upgrade to TypeScript 6#2629
Conversation
TypeScript 6 changed excess property checking for complex generic union/ intersection types, causing `XOR`-based mutual-exclusion to stop flagging invalid combinations (e.g. discriminator fields in create input, mixing field-based and path-based JSON filters). Fix by explicitly adding `?: never` for the opposing side's fields in each XOR branch so the error is enforced as a direct type constraint rather than relying on excess property checking. Also upgrades TypeScript to 6 across the workspace and regenerates affected samples and type-check tests accordingly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughRefines TypeScript typings (narrowing several Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
|
Claude finished @ymc9's task —— View job Code Review for TypeScript 6 XOR Type Errors FixTodo List:
SummaryThis PR addresses a critical TypeScript 6 compatibility issue where XOR-based mutual-exclusion type constraints stopped working due to changes in excess property checking for complex generic union/intersection types. The fix replaces the generic Key Changes AnalysisCore Type System Changes (packages/orm/src/client/crud-types.ts:805-827)Before: export type TypedJsonFilter<...> = XOR<JsonFilter<AllowedKinds>, TypedJsonTypedFilter<...>>;After: export type TypedJsonFilter<...> =
| (JsonFilter<AllowedKinds> & { [Key in GetTypeDefFields<Schema, TypeDefName>]?: never })
| (TypedJsonTypedFilter<Schema, TypeDefName, Array, AllowedKinds> & {
[Key in keyof JsonFilter<AllowedKinds>]?: never;
})
| (Optional extends true ? null | JsonNullValues : never);Analysis: Test Coverage UpdatesDelegate Type Checking (tests/e2e/orm/schemas/delegate/typecheck.ts):
JSON Filter Tests (tests/e2e/orm/client-api/json-filter.test.ts):
Code Quality Assessment✅ Strengths
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/cli/src/actions/proxy.ts (1)
222-223: Changeschema: anytoschema: SchemaDefinstartServersignature.The
createProxyAppfunction at line 201 expectsschema: SchemaDef, butstartServeracceptsschema: anyand passes it directly, creating a type safety gap. Update the parameter type to match the expected contract.♻️ Suggested change
-function startServer(client: ClientContract<SchemaDef>, schema: any, options: Options) { +function startServer(client: ClientContract<SchemaDef>, schema: SchemaDef, options: Options) { const app = createProxyApp(client, schema);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/actions/proxy.ts` around lines 222 - 223, The startServer function currently types its second parameter as schema: any which breaks type safety when passing it to createProxyApp; change the startServer signature to accept schema: SchemaDef (e.g., function startServer(client: ClientContract<SchemaDef>, schema: SchemaDef, options: Options)) so it matches createProxyApp's expected type and preserves the SchemaDef contract throughout the call chain; update any related references to startServer to satisfy the new type if necessary.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/orm/src/client/crud-types.ts`:
- Around line 811-816: The current union for JSON filtering makes operator-named
fields unfilterable by blanket-applying "?: never" across all JsonFilter keys;
update the mutual-exclusion constraint so it only forbids actual typedef field
names on the raw-JSON branch (or only forbids JSON operator keys on the
typed-JSON branch) instead of banning every key. Concretely, replace the mapped
never constraint "[Key in keyof JsonFilter<AllowedKinds>]?: never" with a
conditional exclusion using Exclude/Extract so that the raw JSON branch uses
Exclude<keyof JsonFilter<AllowedKinds>, GetTypeDefFields<Schema, TypeDefName>>
(or the typed branch uses Exclude<GetTypeDefFields<Schema, TypeDefName>, keyof
JsonFilter<AllowedKinds>> as appropriate), keeping the Optional/null arm
unchanged; adjust types JsonFilter, TypedJsonTypedFilter, GetTypeDefFields,
Schema, TypeDefName, AllowedKinds, Optional, and JsonNullValues references
accordingly.
In `@tests/e2e/orm/schemas/delegate/typecheck.ts`:
- Around line 104-114: Uncomment the TS6 discriminator regression test blocks so
the typecheck asserts remain active: restore the commented create/update/upsert
cases that call client.ratedVideo.create, client.ratedVideo.update, and
client.ratedVideo.upsert (and any sibling test blocks at the other commented
ranges) and keep the inline // `@ts-expect-error` markers on assignments to the
discriminator field (e.g., assetType: 'Video') so the TypeScript typecheck suite
fails if delegate discriminator fields are allowed; ensure the tests compile as
part of the typecheck run.
---
Nitpick comments:
In `@packages/cli/src/actions/proxy.ts`:
- Around line 222-223: The startServer function currently types its second
parameter as schema: any which breaks type safety when passing it to
createProxyApp; change the startServer signature to accept schema: SchemaDef
(e.g., function startServer(client: ClientContract<SchemaDef>, schema:
SchemaDef, options: Options)) so it matches createProxyApp's expected type and
preserves the SchemaDef contract throughout the call chain; update any related
references to startServer to satisfy the new type if necessary.
🪄 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: 72bad4a5-783d-4469-92b6-59ccf1b4a783
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (15)
packages/cli/src/actions/proxy.tspackages/cli/tsdown.config.tspackages/orm/src/client/crud-types.tspackages/orm/src/client/plugin.tspackages/orm/src/client/zod/factory.tspnpm-workspace.yamlsamples/next.js/app/global.d.tssamples/next.js/zenstack/input.tssamples/nuxt/zenstack/input.tssamples/orm/zenstack/input.tssamples/sveltekit/src/zenstack/input.tstests/e2e/orm/client-api/json-filter.test.tstests/e2e/orm/client-api/slicing.test.tstests/e2e/orm/client-api/unsupported.test.tstests/e2e/orm/schemas/delegate/typecheck.ts
|
Claude finished @ymc9's task —— View job Code Review for TypeScript 6 XOR Type Errors FixTodo List:
SummaryThis PR addresses a critical TypeScript 6 compatibility issue where XOR-based mutual-exclusion type constraints stopped working due to changes in excess property checking for complex generic union/intersection types. The fix replaces the generic Key Changes AnalysisCore Type System Changes (packages/orm/src/client/crud-types.ts:805-827)Before: export type TypedJsonFilter<...> = XOR<JsonFilter<AllowedKinds>, TypedJsonTypedFilter<...>>;After: export type TypedJsonFilter<...> =
| (JsonFilter<AllowedKinds> & { [Key in GetTypeDefFields<Schema, TypeDefName>]?: never })
| (TypedJsonTypedFilter<Schema, TypeDefName, Array, AllowedKinds> & {
[Key in keyof JsonFilter<AllowedKinds>]?: never;
})
| (Optional extends true ? null | JsonNullValues : never);Analysis: Test Coverage UpdatesDelegate Type Checking (tests/e2e/orm/schemas/delegate/typecheck.ts):
JSON Filter Tests (tests/e2e/orm/client-api/json-filter.test.ts):
Code Quality Assessment✅ Strengths
|
|
Claude finished @ymc9's task —— View job Code Review for TypeScript 6 XOR Type Errors FixTodo List:
SummaryThis PR addresses a critical TypeScript 6 compatibility issue where XOR-based mutual-exclusion type constraints stopped working due to changes in excess property checking for complex generic union/intersection types. The fix replaces the generic Key Changes AnalysisCore Type System Changes (packages/orm/src/client/crud-types.ts:805-827)Before: export type TypedJsonFilter<...> = XOR<JsonFilter<AllowedKinds>, TypedJsonTypedFilter<...>>;After: export type TypedJsonFilter<...> =
| (JsonFilter<AllowedKinds> & { [Key in GetTypeDefFields<Schema, TypeDefName>]?: never })
| (TypedJsonTypedFilter<Schema, TypeDefName, Array, AllowedKinds> & {
[Key in keyof JsonFilter<AllowedKinds>]?: never;
})
| (Optional extends true ? null | JsonNullValues : never);Analysis: Test Coverage UpdatesDelegate Type Checking (tests/e2e/orm/schemas/delegate/typecheck.ts):
JSON Filter Tests (tests/e2e/orm/client-api/json-filter.test.ts):
Code Quality Assessment✅ Strengths
|
Summary
XOR-based mutual-exclusion to silently stop enforcing invalid combinationsvideoTypein delegate models) and mixing field-based + path-based JSON filters?: neverfor the opposing side's fields in each XOR branch, making the constraint a direct type error rather than relying on excess property checkingTypedJsonTypedFilterto remove theOptionalparameter and hoistnull | JsonNullValuesinto the parentTypedJsonFilter, making the type structure cleanerTest plan
@ts-expect-erroronvideoTypeindelegate.test.tsis valid again (TypeScript 6 correctly flags it)json-filter.test.tsis correctly flaggedaddress: null(null filter for optional nested typedef field) still works —null | JsonNullValuescorrectly excluded from the?: neverintersection🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Refactor
Chores
Tests