fix(orm): reimplement typescript type display simplification #405
Conversation
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis PR removes the trailing boolean generic (the Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas that may need extra attention:
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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 |
|
Claude finished @ymc9's task —— View job PR Review: TypeScript Type Display SimplificationI've completed my review of this pull request focusing on code quality, potential issues, performance, and security considerations. Todo List:
Analysis SummaryThis PR implements a well-architected TypeScript type simplification that should significantly improve developer experience by making generated types more readable and reducing TypeScript compiler overhead. ✅ StrengthsType System Improvements
Code Quality
|
There was a problem hiding this comment.
Pull Request Overview
This PR reimplements TypeScript type display simplification by introducing a branded type pattern for schema definitions and removing the Simplify boolean generic parameter from various type definitions. The goal is to prevent TypeScript from expanding complex types while maintaining type safety.
Key changes:
- Introduces a branded type pattern for schema objects using
type Schema = typeof _schema & { __brand?: 'schema' } - Removes the
Simplifyboolean parameter from all Args types (FindManyArgs, CreateArgs, etc.) - Removes the
Schemageneric parameter from primitive filter types (StringFilter, NumberFilter, etc.)
Reviewed Changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/sdk/src/ts-schema-generator.ts | Implements the schema generation logic with branded types |
| packages/orm/src/client/crud-types.ts | Removes Simplify parameters and Schema generic from filter types; removes SimplifyIf import and ProviderSupportsCaseSensitivity helper |
| packages/orm/src/client/crud/dialects/base-dialect.ts | Updates filter method signatures to match new type definitions |
| packages/orm/src/client/contract.ts | Updates all model operation signatures to remove Simplify parameter |
| packages/clients/tanstack-query/src/*.ts | Updates hook signatures for react, vue, and svelte to remove Simplify parameter |
| tests/e2e/orm/schemas/*/schema.ts | Regenerated schema files following the new branded type pattern |
| tests/e2e/apps/rally/zenstack/schema.ts | Regenerated schema file |
| samples//zenstack/schema.ts | Regenerated schema files |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/orm/src/client/crud-types.ts (2)
207-242: ThreadWithAggregationsthrough recursiveAND/OR/NOTbranches to preserve nested aggregate filters inGroupByHavingVerification confirms the bug:
GroupByHavingpassesWhereInput<Schema, Model, true, true>withWithAggregations=true, but the recursiveAND/OR/NOTat lines 239–241 drop this parameter entirely. This breaks access to aggregate-aware filters (like_count,_avg,_sum) in nested logical conditions, since all filter types conditionally include these fields only whenWithAggregationsistrue.Fix by threading
WithAggregationsthrough:} & { - AND?: OrArray<WhereInput<Schema, Model, ScalarOnly>>; - OR?: WhereInput<Schema, Model, ScalarOnly>[]; - NOT?: OrArray<WhereInput<Schema, Model, ScalarOnly>>; + AND?: OrArray<WhereInput<Schema, Model, ScalarOnly, WithAggregations>>; + OR?: WhereInput<Schema, Model, ScalarOnly, WithAggregations>[]; + NOT?: OrArray<WhereInput<Schema, Model, ScalarOnly, WithAggregations>>; };
244-263: String filtermodefield lacks provider-specific TypeScript gating; runtime validation is insufficientThe
StringFiltertype now unconditionally permitsmode?: 'default' | 'insensitive'regardless of provider, but runtime validation only allows this field for PostgreSQL (viaproviderSupportsCaseSensitivitycheck in validator/index.ts:1534-1535). When clients providemodeon unsupported providers, Zod rejects it as an unrecognized key with a generic validation error rather than signaling provider incompatibility.This is a legitimate behavior change: compile-time protection has been removed. While runtime validation catches the issue, the developer experience is poor—valid TypeScript code fails at runtime with an unclear error message.
Recommendation: Either restore provider-specific TypeScript-level gating (e.g.,
ProviderSupportsCaseSensitivity<Schema>helper similar toProviderSupportsDistinct<Schema>) or enhance error messaging to explicitly state "mode 'insensitive' is not supported for this provider."
🧹 Nitpick comments (2)
packages/orm/src/client/crud/dialects/base-dialect.ts (2)
607-619: Consider looseningStringFiltergenerics for consistencyThe change to
payload: StringFilter<true, boolean>and the recursive cast keeps behavior the same, but it’s more specific than the other helpers (BooleanFilter<boolean, boolean>, etc.). For symmetry with thecrud-typesdefinitions and to avoid over-constraining future call sites, consider changing this toStringFilter<boolean, boolean>(and the recursive cast likewise), unlessNullable=trueis intentionally required here.
711-721: Bytes filter bugfix is correct; minor naming cleanup possibleSwitching to
BytesFilter<boolean, boolean>and returningthis.and(...conditions.conditions)correctly uses the object returned bybuildStandardFilterand fixes the prior misuse of the entire result object as a condition list. For readability and consistency with nearby helpers, you could destructure as:- private buildBytesFilter(fieldRef: Expression<any>, payload: BytesFilter<boolean, boolean>) { - const conditions = this.buildStandardFilter( + private buildBytesFilter(fieldRef: Expression<any>, payload: BytesFilter<boolean, boolean>) { + const { conditions } = this.buildStandardFilter( 'Bytes', payload, fieldRef, (value) => this.transformPrimitive(value, 'Bytes', false), (value) => this.buildBytesFilter(fieldRef, value as BytesFilter<boolean, boolean>), true, ['equals', 'in', 'notIn', 'not'], ); - return this.and(...conditions.conditions); + return this.and(...conditions); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
packages/clients/tanstack-query/src/react.ts(1 hunks)packages/clients/tanstack-query/src/svelte.ts(1 hunks)packages/clients/tanstack-query/src/vue.ts(1 hunks)packages/clients/tanstack-query/test/schemas/basic/schema-lite.ts(2 hunks)packages/orm/src/client/contract.ts(17 hunks)packages/orm/src/client/crud-types.ts(8 hunks)packages/orm/src/client/crud/dialects/base-dialect.ts(3 hunks)packages/sdk/src/ts-schema-generator.ts(2 hunks)samples/next.js/zenstack/schema-lite.ts(2 hunks)samples/next.js/zenstack/schema.ts(2 hunks)samples/orm/zenstack/schema.ts(2 hunks)tests/e2e/apps/rally/zenstack/schema.ts(2 hunks)tests/e2e/orm/schemas/basic/schema.ts(2 hunks)tests/e2e/orm/schemas/default-auth/schema.ts(2 hunks)tests/e2e/orm/schemas/delegate/schema.ts(2 hunks)tests/e2e/orm/schemas/name-mapping/schema.ts(2 hunks)tests/e2e/orm/schemas/petstore/schema.ts(2 hunks)tests/e2e/orm/schemas/todo/schema.ts(2 hunks)tests/e2e/orm/schemas/typing/schema.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-21T16:09:31.218Z
Learnt from: ymc9
Repo: zenstackhq/zenstack-v3 PR: 319
File: packages/runtime/src/client/executor/zenstack-query-executor.ts:63-72
Timestamp: 2025-10-21T16:09:31.218Z
Learning: In ZenStack, TypeDefs can be inherited by models. When a TypeDef contains fields with `map` attributes, those mapped field names need to be processed by the QueryNameMapper since they become part of the inheriting model's schema. Therefore, when checking if a schema has mapped names (e.g., in `schemaHasMappedNames`), both `schema.models` and `schema.typeDefs` must be inspected for `@map` and `map` attributes.
Applied to files:
tests/e2e/apps/rally/zenstack/schema.tssamples/orm/zenstack/schema.tssamples/next.js/zenstack/schema-lite.tssamples/next.js/zenstack/schema.ts
📚 Learning: 2025-10-21T16:04:56.292Z
Learnt from: ymc9
Repo: zenstackhq/zenstack-v3 PR: 319
File: packages/runtime/src/client/crud/dialects/base-dialect.ts:745-747
Timestamp: 2025-10-21T16:04:56.292Z
Learning: In packages/runtime/src/client/crud/dialects/base-dialect.ts, it's intentional that buildCursorFilter applies default ordering (via makeDefaultOrderBy fallback) while buildOrderBy does not. This ensures cursor-based pagination always has stable ordering for correctness, while regular queries remain unordered unless explicitly specified. This design is to be consistent with Prisma's pagination requirements.
Applied to files:
packages/orm/src/client/crud/dialects/base-dialect.tspackages/orm/src/client/crud-types.ts
🧬 Code graph analysis (18)
tests/e2e/orm/schemas/typing/schema.ts (4)
packages/clients/tanstack-query/test/schemas/basic/schema-lite.ts (2)
schema(175-175)SchemaType(176-176)samples/next.js/zenstack/schema.ts (2)
schema(118-118)SchemaType(119-119)packages/orm/src/client/crud/validator/index.ts (1)
schema(60-62)packages/plugins/policy/src/expression-transformer.ts (1)
schema(81-83)
tests/e2e/orm/schemas/basic/schema.ts (4)
packages/clients/tanstack-query/test/schemas/basic/schema-lite.ts (2)
schema(175-175)SchemaType(176-176)samples/next.js/zenstack/schema.ts (2)
schema(118-118)SchemaType(119-119)packages/orm/src/client/crud/validator/index.ts (1)
schema(60-62)packages/plugins/policy/src/expression-transformer.ts (1)
schema(81-83)
tests/e2e/orm/schemas/name-mapping/schema.ts (3)
packages/clients/tanstack-query/test/schemas/basic/schema-lite.ts (2)
schema(175-175)SchemaType(176-176)packages/orm/src/client/crud/validator/index.ts (1)
schema(60-62)packages/plugins/policy/src/expression-transformer.ts (1)
schema(81-83)
tests/e2e/orm/schemas/todo/schema.ts (3)
packages/clients/tanstack-query/test/schemas/basic/schema-lite.ts (2)
schema(175-175)SchemaType(176-176)packages/orm/src/client/crud/validator/index.ts (1)
schema(60-62)packages/plugins/policy/src/expression-transformer.ts (1)
schema(81-83)
tests/e2e/apps/rally/zenstack/schema.ts (3)
packages/clients/tanstack-query/test/schemas/basic/schema-lite.ts (2)
schema(175-175)SchemaType(176-176)samples/next.js/zenstack/schema.ts (2)
schema(118-118)SchemaType(119-119)tests/e2e/orm/schemas/basic/schema.ts (2)
schema(290-290)SchemaType(291-291)
packages/clients/tanstack-query/test/schemas/basic/schema-lite.ts (11)
samples/next.js/zenstack/schema-lite.ts (2)
schema(109-109)SchemaType(110-110)samples/next.js/zenstack/schema.ts (2)
schema(118-118)SchemaType(119-119)samples/orm/zenstack/schema.ts (2)
schema(247-247)SchemaType(248-248)tests/e2e/apps/rally/zenstack/schema.ts (2)
schema(2526-2526)SchemaType(2527-2527)tests/e2e/orm/schemas/basic/schema.ts (2)
schema(290-290)SchemaType(291-291)tests/e2e/orm/schemas/default-auth/schema.ts (2)
schema(125-125)SchemaType(126-126)tests/e2e/orm/schemas/delegate/schema.ts (2)
schema(468-468)SchemaType(469-469)tests/e2e/orm/schemas/name-mapping/schema.ts (2)
schema(126-126)SchemaType(127-127)tests/e2e/orm/schemas/petstore/schema.ts (2)
schema(160-160)SchemaType(161-161)tests/e2e/orm/schemas/todo/schema.ts (2)
schema(399-399)SchemaType(400-400)tests/e2e/orm/schemas/typing/schema.ts (2)
schema(350-350)SchemaType(351-351)
tests/e2e/orm/schemas/default-auth/schema.ts (5)
packages/clients/tanstack-query/test/schemas/basic/schema-lite.ts (2)
schema(175-175)SchemaType(176-176)samples/next.js/zenstack/schema-lite.ts (2)
schema(109-109)SchemaType(110-110)samples/next.js/zenstack/schema.ts (2)
schema(118-118)SchemaType(119-119)samples/orm/zenstack/schema.ts (2)
schema(247-247)SchemaType(248-248)tests/e2e/orm/schemas/basic/schema.ts (2)
schema(290-290)SchemaType(291-291)
tests/e2e/orm/schemas/petstore/schema.ts (5)
packages/clients/tanstack-query/test/schemas/basic/schema-lite.ts (2)
schema(175-175)SchemaType(176-176)samples/next.js/zenstack/schema-lite.ts (2)
schema(109-109)SchemaType(110-110)samples/next.js/zenstack/schema.ts (2)
schema(118-118)SchemaType(119-119)packages/orm/src/client/crud/validator/index.ts (1)
schema(60-62)packages/plugins/policy/src/expression-transformer.ts (1)
schema(81-83)
samples/orm/zenstack/schema.ts (4)
packages/clients/tanstack-query/test/schemas/basic/schema-lite.ts (2)
schema(175-175)SchemaType(176-176)samples/next.js/zenstack/schema.ts (2)
schema(118-118)SchemaType(119-119)tests/e2e/orm/schemas/todo/schema.ts (2)
schema(399-399)SchemaType(400-400)packages/orm/src/client/crud/validator/index.ts (1)
schema(60-62)
samples/next.js/zenstack/schema-lite.ts (3)
packages/clients/tanstack-query/test/schemas/basic/schema-lite.ts (2)
schema(175-175)SchemaType(176-176)samples/next.js/zenstack/schema.ts (2)
schema(118-118)SchemaType(119-119)samples/orm/zenstack/schema.ts (2)
schema(247-247)SchemaType(248-248)
packages/clients/tanstack-query/src/svelte.ts (1)
packages/orm/src/client/crud-types.ts (18)
FindUniqueArgs(663-665)SelectSubset(502-508)SimplifiedModelResult(181-187)FindFirstArgs(661-661)FindManyArgs(659-659)CreateArgs(671-673)CreateManyArgs(675-675)CreateManyAndReturnArgs(677-681)UpdateArgs(807-810)UpdateManyArgs(812-815)UpdateManyAndReturnArgs(817-821)UpsertArgs(829-833)DeleteArgs(945-947)DeleteManyArgs(949-952)CountArgs(958-963)Subset(498-500)AggregateArgs(983-997)GroupByArgs(1068-1085)
packages/clients/tanstack-query/src/vue.ts (1)
packages/orm/src/client/crud-types.ts (13)
FindUniqueArgs(663-665)SelectSubset(502-508)SimplifiedModelResult(181-187)FindFirstArgs(661-661)FindManyArgs(659-659)CreateArgs(671-673)UpdateArgs(807-810)UpsertArgs(829-833)DeleteArgs(945-947)CountArgs(958-963)Subset(498-500)AggregateArgs(983-997)GroupByArgs(1068-1085)
packages/orm/src/client/contract.ts (1)
packages/orm/src/client/crud-types.ts (15)
FindManyArgs(659-659)FindUniqueArgs(663-665)FindFirstArgs(661-661)CreateArgs(671-673)CreateManyArgs(675-675)CreateManyAndReturnArgs(677-681)UpdateArgs(807-810)UpdateManyArgs(812-815)UpdateManyAndReturnArgs(817-821)UpsertArgs(829-833)DeleteArgs(945-947)DeleteManyArgs(949-952)CountArgs(958-963)AggregateArgs(983-997)GroupByArgs(1068-1085)
packages/clients/tanstack-query/src/react.ts (1)
packages/orm/src/client/crud-types.ts (18)
FindUniqueArgs(663-665)SelectSubset(502-508)SimplifiedModelResult(181-187)FindFirstArgs(661-661)FindManyArgs(659-659)CreateArgs(671-673)CreateManyArgs(675-675)CreateManyAndReturnArgs(677-681)UpdateArgs(807-810)UpdateManyArgs(812-815)UpdateManyAndReturnArgs(817-821)UpsertArgs(829-833)DeleteArgs(945-947)DeleteManyArgs(949-952)CountArgs(958-963)Subset(498-500)AggregateArgs(983-997)GroupByArgs(1068-1085)
tests/e2e/orm/schemas/delegate/schema.ts (5)
packages/clients/tanstack-query/test/schemas/basic/schema-lite.ts (2)
schema(175-175)SchemaType(176-176)samples/next.js/zenstack/schema-lite.ts (2)
schema(109-109)SchemaType(110-110)samples/next.js/zenstack/schema.ts (2)
schema(118-118)SchemaType(119-119)packages/orm/src/client/crud/validator/index.ts (1)
schema(60-62)packages/plugins/policy/src/expression-transformer.ts (1)
schema(81-83)
packages/orm/src/client/crud/dialects/base-dialect.ts (1)
packages/orm/src/client/crud-types.ts (4)
StringFilter(306-319)BooleanFilter(364-375)DateTimeFilter(338-347)BytesFilter(349-362)
samples/next.js/zenstack/schema.ts (2)
packages/clients/tanstack-query/test/schemas/basic/schema-lite.ts (2)
schema(175-175)SchemaType(176-176)samples/next.js/zenstack/schema-lite.ts (2)
schema(109-109)SchemaType(110-110)
packages/orm/src/client/crud-types.ts (2)
packages/orm/src/utils/type-utils.ts (3)
NullableIf(5-5)OrArray(52-52)NonEmptyArray(54-54)packages/schema/src/schema.ts (3)
SchemaDef(11-19)GetModels(120-120)NonRelationFields(204-208)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build-test (20.x, sqlite)
- GitHub Check: build-test (20.x, postgresql)
- GitHub Check: claude-review
🔇 Additional comments (23)
samples/next.js/zenstack/schema-lite.ts (4)
9-9: LGTM! Internal constant extraction supports branding pattern.The refactor to an internal
_schemaconstant enables the branded type export pattern while maintaining identical runtime behavior. This is a clean separation of concerns between the data structure and its public type interface.
106-108: LGTM! Well-implemented branded type pattern.The branded
Schematype using an optional__brandfield is a standard TypeScript technique for nominal typing. This provides type-level distinction without runtime overhead, since the optional field doesn't need to exist at runtime. The pattern is consistent with other schema files in the codebase.
109-109: LGTM! Public export correctly applies branded type.The export statement correctly assigns the internal
_schemato the publicschemaexport with the brandedSchematype. This maintains the same runtime value while applying type-level branding for enhanced type safety.
110-110: LGTM! Type alias correctly updated for consistency.The
SchemaTypeexport now directly references theSchemabranded type instead of inferring fromtypeof schema. This is clearer and maintains consistency with the branding pattern implemented throughout the codebase.packages/orm/src/client/crud/dialects/base-dialect.ts (2)
686-693: Boolean filter generic update looks correctUsing
BooleanFilter<boolean, boolean>and recursing with the same type aligns with the updatedBooleanFilterdefinition and keeps runtime behavior unchanged. Restricting keys to['equals', 'not']still matches the intended non-aggregate boolean where filters.
699-706: DateTime filter generic update looks correctThe new signature
DateTimeFilter<boolean, boolean>and recursive cast are consistent with the newDateTimeFiltertype and mirror the number filter behavior. No functional changes introduced.samples/orm/zenstack/schema.ts (1)
9-9: Branded schema pattern looks consistent and safeUsing an internal
_schemawithas const satisfies SchemaDef, a brandedSchematype, and exportingschema: Schema/SchemaType = Schemamatches the new pattern elsewhere and keeps runtime shape unchanged while improving type identity. No issues spotted here.Also applies to: 244-248
samples/next.js/zenstack/schema.ts (1)
9-9: Next.js schema branding matches the shared patternThe introduction of
_schema, brandedSchema, and theschema: Schema/SchemaType = Schemaexports is consistent with other generated schemas and should preserve all existing consumers while avoiding type expansion.Also applies to: 115-119
packages/sdk/src/ts-schema-generator.ts (1)
151-216: AST generation correctly updated for branded schema exportsThe new statements (_schema declaration with
as const satisfies SchemaDef, brandedSchemaalias,schema: Schema = _schema, andSchemaType = Schema) accurately generate the desired TS pattern and align with the hand-written schema files, without changing runtime behavior.packages/orm/src/client/crud-types.ts (1)
658-683: CRUD arg type simplification is consistent with updated API surfaceThe simplification of
FindManyArgs/FindFirstArgs/FindUniqueArgs, create/update/delete args, andCountArgs/AggregateArgs/GroupByArgsto drop the extra generic parameter and rely on the more direct shapes in this file aligns with the updated method signatures incontract.ts. The use ofSelectIncludeOmit<Schema, Model, ...>and the newCreateManyAndReturnArgs/UpdateManyAndReturnArgsshapes also looks internally consistent.I’d just recommend running a full
tscover the repo to ensure no remaining references to the old 3‑parameter arg types (e.g.,<Schema, Model, true>) remain in downstream code.Also applies to: 806-834, 945-952, 958-963, 983-997, 1068-1085
packages/orm/src/client/contract.ts (1)
246-799: Client contract generics are correctly aligned with the new CRUD arg typesAll model operation methods now constrain
Tagainst the simplified arg aliases (FindManyArgs<Schema, Model>,CreateArgs<Schema, Model>, etc.) and still useSelectSubset/SubsetplusSimplifiedModelResultfor results, which matches the refactored definitions incrud-types.ts. The transactional and plugin-related parts remain untouched.Looks coherent and should be backward compatible at the call‑site level as long as everything compiles after the arg type changes.
tests/e2e/orm/schemas/typing/schema.ts (1)
9-351: Schema branding and re-export pattern looks correctRebinding the schema object to
_schema, introducing the brandedSchematype, and exportingschema: Schema/SchemaType = Schemakeeps the runtime shape intact and matches the new standardized pattern used elsewhere. I don’t see any issues here.Please rerun TypeScript type-checks for the typing E2E suite to confirm there are no unexpected type incompatibilities with consumers of
schema/SchemaType.tests/e2e/apps/rally/zenstack/schema.ts (1)
9-2527: Rally schema branding change is consistent and non-breakingThe switch to an internal
_schema, brandedSchematype, andschema: Schema/SchemaType = Schemaexport is consistent with other schema files and shouldn’t affect runtime behavior or callers importingschema/SchemaType.Given how widely this schema is used in the rally E2E app, please run the rally test suite or at least a type-check over its usages to ensure everything still compiles cleanly.
tests/e2e/orm/schemas/basic/schema.ts (1)
9-291: Basic ORM schema branding matches the new standardUsing
_schemaplus the internal brandedSchematype and exportingschema: SchemaandSchemaType = Schemais consistent with the rest of the codebase and keeps the existing runtime/data shape. No issues spotted.Please ensure the ORM “basic” E2E tests and type-checks are run so any accidental breakage in downstream
SchemaTypeconsumers is caught.tests/e2e/orm/schemas/petstore/schema.ts (1)
9-161: Petstore schema branding refactor looks goodThe refactor to
_schemawith a brandedSchematype and theschema/SchemaTypeexports follows the same pattern as other schemas and should be a pure typing-surface change without runtime impact.As this schema participates in authorization rules, please run the petstore-related E2E tests to confirm policy logic still type-checks and behaves as expected.
tests/e2e/orm/schemas/name-mapping/schema.ts (1)
9-127: Name‑mapping schema branding aligns with prior patternThe move to an internal
_schemaplus brandedSchematype, and exportingschema: Schema/SchemaType = Schema, is consistent with the rest of the PR and keeps the mapped-name structure untouched.Because this schema is specifically about name-mapping, please rerun the corresponding E2E/name-mapping tests to verify that any tooling relying on mapped names still works correctly with the branded
SchemaType.tests/e2e/orm/schemas/delegate/schema.ts (1)
9-469: BrandedSchemasurface looks correct and preserves runtime behaviorUsing an internal
_schemathatsatisfies SchemaDefand exportingschema: Schema = _schemawithtype Schema = typeof _schema & { __brand?: 'schema' }keeps the runtime object unchanged while giving you a stable, branded schema type. This stays structurally compatible withSchemaDefand matches the pattern used in other schema files.packages/clients/tanstack-query/test/schemas/basic/schema-lite.ts (1)
9-176: Consistent schema branding for test schema-liteThis file follows the same
_schema+ brandedSchema+SchemaType = Schemapattern as the other generated schemas, keeping the runtime shape andSchemaDefcompatibility intact while stabilizing the exported type.tests/e2e/orm/schemas/default-auth/schema.ts (1)
9-126: Default-auth schema branding matches new patternThe introduction of
_schema, the brandedSchematype, and exportingschema: Schema = _schemacleanly updates the type surface while leaving the generated schema data andSchemaDefcompatibility unchanged.tests/e2e/orm/schemas/todo/schema.ts (1)
9-400: Todo schema export now correctly uses brandedSchemaWrapping the existing literal in
_schemaand exposingschema: Schema = _schemawithSchemaType = Schemais consistent with the rest of the codebase and should simplify typing without affecting E2E behavior.packages/clients/tanstack-query/src/svelte.ts (1)
141-211: Approve: hook generics updated to two-parameter arg types and verified
All tanstack-query hook signatures use XxxArgs<Schema, Model>; crud-types.ts exposes matching two-parameter aliases and a repo scan found no remaining true 3‑arity usages (delete.ts hits were nested-generic false positives).packages/clients/tanstack-query/src/vue.ts (1)
125-195: Vue ModelQueryHooks generics now correctly mirror updated ORM arg typesThe removal of the third generic parameter from all
*Args<Schema, Model>constraints is consistent with the updated ORM type aliases and keeps theSimplifiedModelResult<Schema, Model, T, ...>flow intact. No runtime paths are impacted, and the Vue hooks surface stays aligned with React/Svelte.packages/clients/tanstack-query/src/react.ts (1)
148-253: React ModelQueryHooks type simplification is consistent and non-breakingAll hook generics now use the simplified
*Args<Schema, Model>aliases (including suspense/infinite variants), matching the ORM definitions and other clients. This is a clean type-only change with no runtime impact and keeps the React hooks surface consistent across the SDK.
|
Claude finished @ymc9's task —— View job PR Review: TypeScript Type Display SimplificationI've completed my review of this pull request focusing on code quality, potential issues, performance, and security considerations. Todo List:
Analysis SummaryThis PR implements a well-architected TypeScript type simplification that should significantly improve developer experience by making generated types more readable and reducing TypeScript compiler overhead. ✅ StrengthsType System Improvements
Code Quality
|
Summary by CodeRabbit
Refactor
Type Improvements