fix(tanstack): improve mutation mutateAsync typing #368
Conversation
WalkthroughRefactors TanStack Query client integrations (React, Svelte, Vue) to introduce new public types Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 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 |
|
Claude finished @ymc9's task —— View job Code Review: TanStack Query Mutation Typing ImprovementsI've completed a comprehensive review of this PR that improves mutation Todo List:
🎯 SummaryThis is an excellent PR that addresses a critical typing issue with ✅ Strengths1. Consistent Architecture
2. Excellent Type Safety
3. Comprehensive Test Coverage
4. Clean Implementation
🔧 Technical Implementation DetailsType System ImprovementsThe core improvement is in export type TrimDelegateModelOperations<
Schema extends SchemaDef,
Model extends GetModels<Schema>,
T extends Record<string, unknown>,
> = IsDelegateModel<Schema, Model> extends true ? Omit<T, OperationsIneligibleForDelegateModels> : T;This type correctly handles delegate models by removing operations that aren't applicable to them. Mutation Typing EnhancementThe key innovation is the export type ModelMutationModelResult<...> = Omit<ModelMutationResult<...>, 'mutateAsync'> & {
mutateAsync<T extends TArgs>(
args: T,
options?: ModelMutationOptions<ModelResult<Schema, Model, T>, T>,
): Promise<Array extends true ? ModelResult<Schema, Model, T>[] : ModelResult<Schema, Model, T>>;
};This provides precise typing that infers the return type based on the actual arguments passed to 🛡️ Security & Performance AssessmentSecurity: ✅ No Concerns
Performance: ✅ Excellent
🧪 Test Quality AssessmentComprehensive CoverageThe new typing tests (
📦 Package ManagementThe version bump to 🚀 Minor Suggestions for Future Improvements
🎯 ConclusionThis is a high-quality PR that significantly improves the developer experience without any drawbacks:
The implementation is clean, well-tested, and follows best practices. This PR represents exactly the kind of improvement that makes ZenStack's TanStack Query integration more developer-friendly and type-safe. Recommendation: ✅ Approve and merge |
There was a problem hiding this comment.
Pull Request Overview
This PR refactors type definitions in the TanStack Query client to improve type safety and maintainability. The changes include extracting ineligible operations for delegate models into a reusable type, introducing a utility type for trimming delegate model operations, and enhancing mutation return types to provide better type inference for the mutateAsync method.
- Extracted delegate model operation restrictions into a named type for reusability
- Added
TrimDelegateModelOperationsutility type to conditionally omit operations based on model type - Enhanced mutation hooks to return more specific types with overloaded
mutateAsyncsignatures - Added comprehensive type tests for React, Vue, and Svelte implementations
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/orm/src/client/contract.ts | Extracts delegate model ineligible operations into a reusable named type |
| packages/clients/tanstack-query/src/utils/types.ts | Adds utility type for trimming operations from delegate models |
| packages/clients/tanstack-query/src/react.ts | Updates type definitions to use new mutation result type and operation trimming utility |
| packages/clients/tanstack-query/src/vue.ts | Updates type definitions to use new mutation result type and operation trimming utility |
| packages/clients/tanstack-query/src/svelte.ts | Updates type definitions to use new mutation result type and operation trimming utility |
| packages/clients/tanstack-query/test/react-typing-test.ts | Adds comprehensive type tests for React hooks |
| packages/clients/tanstack-query/test/vue-typing-test.ts | Adds comprehensive type tests for Vue hooks |
| packages/clients/tanstack-query/test/svelte-typing-test.ts | Adds comprehensive type tests for Svelte hooks |
| packages/clients/tanstack-query/package.json | Minor reordering of devDependencies (no functional change) |
💡 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: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
packages/clients/tanstack-query/package.json(1 hunks)packages/clients/tanstack-query/src/react.ts(4 hunks)packages/clients/tanstack-query/src/svelte.ts(4 hunks)packages/clients/tanstack-query/src/utils/types.ts(2 hunks)packages/clients/tanstack-query/src/vue.ts(4 hunks)packages/clients/tanstack-query/test/react-typing-test.ts(1 hunks)packages/clients/tanstack-query/test/svelte-typing-test.ts(1 hunks)packages/clients/tanstack-query/test/vue-typing-test.ts(1 hunks)packages/orm/src/client/contract.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (8)
packages/clients/tanstack-query/src/utils/types.ts (2)
packages/schema/src/schema.ts (3)
SchemaDef(10-18)GetModels(109-109)IsDelegateModel(264-267)packages/orm/src/client/contract.ts (1)
OperationsIneligibleForDelegateModels(798-798)
packages/clients/tanstack-query/test/vue-typing-test.ts (3)
packages/clients/tanstack-query/src/react.ts (1)
useClientQueries(257-262)packages/clients/tanstack-query/src/svelte.ts (1)
useClientQueries(212-217)packages/clients/tanstack-query/src/vue.ts (1)
useClientQueries(201-206)
packages/clients/tanstack-query/test/react-typing-test.ts (1)
packages/clients/tanstack-query/src/react.ts (1)
useClientQueries(257-262)
packages/clients/tanstack-query/test/svelte-typing-test.ts (1)
packages/clients/tanstack-query/src/svelte.ts (1)
useClientQueries(212-217)
packages/clients/tanstack-query/src/react.ts (2)
packages/orm/src/client/crud-types.ts (3)
SelectIncludeOmit(453-457)ModelResult(135-179)FindArgs(648-664)packages/clients/tanstack-query/src/utils/types.ts (1)
TrimDelegateModelOperations(24-28)
packages/orm/src/client/contract.ts (1)
packages/schema/src/schema.ts (3)
SchemaDef(10-18)GetModels(109-109)IsDelegateModel(264-267)
packages/clients/tanstack-query/src/vue.ts (3)
packages/clients/tanstack-query/src/react.ts (9)
ModelQueryOptions(94-94)ModelQueryResult(96-96)ModelInfiniteQueryOptions(103-106)ModelInfiniteQueryResult(108-108)ModelMutationOptions(119-120)ModelMutationResult(122-122)ModelMutationModelResult(124-134)ClientHooks(136-138)ModelQueryHooks(142-252)packages/orm/src/client/crud-types.ts (7)
SelectIncludeOmit(453-457)ModelResult(135-179)FindUniqueArgs(669-671)SelectSubset(509-515)FindArgs(648-664)CreateArgs(677-682)Subset(505-507)packages/clients/tanstack-query/src/utils/types.ts (1)
TrimDelegateModelOperations(24-28)
packages/clients/tanstack-query/src/svelte.ts (5)
packages/clients/tanstack-query/src/react.ts (5)
ModelMutationModelResult(124-134)ModelMutationResult(122-122)ModelMutationOptions(119-120)ClientHooks(136-138)ModelQueryHooks(142-252)packages/schema/src/schema.ts (2)
SchemaDef(10-18)GetModels(109-109)packages/orm/src/client/crud-types.ts (3)
SelectIncludeOmit(453-457)ModelResult(135-179)FindArgs(648-664)packages/clients/tanstack-query/src/utils/types.ts (1)
TrimDelegateModelOperations(24-28)packages/common-helpers/src/lower-case-first.ts (1)
lowerCaseFirst(1-3)
⏰ 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). (4)
- GitHub Check: Upload results
- GitHub Check: build-test (20.x, postgresql)
- GitHub Check: build-test (20.x, sqlite)
- GitHub Check: claude-review
🔇 Additional comments (18)
packages/clients/tanstack-query/package.json (1)
68-85: Dependency reordering is acceptable.The reordering of
@tanstack/svelte-queryafter@tanstack/vue-queryis a trivial organizational change with no functional impact. Versions remain consistent with peer dependency constraints.packages/clients/tanstack-query/src/utils/types.ts (1)
24-28: Delegate trim logic looks correct
TrimDelegateModelOperationscleanly reusesOperationsIneligibleForDelegateModelsso delegate models automatically drop unsupported hook keys without affecting others. Nice reuse of existing schema predicates.packages/orm/src/client/contract.ts (1)
798-804: Shared ineligible-operations alias fits wellCentralizing
OperationsIneligibleForDelegateModelshere and reusing it in the delegate conditional keeps the ORM and client typings consistent—looks good.packages/clients/tanstack-query/test/react-typing-test.ts (1)
41-76: Solid mutateAsync coverageAppreciate the explicit
mutate/mutateAsyncchecks (plus the negative@ts-expect-errorcases)—this exercises the new typing surface thoroughly.packages/clients/tanstack-query/test/vue-typing-test.ts (1)
39-74: Vue typings well exercisedThanks for mirroring the mutation edge cases here—the promise chains and negative checks give good confidence in the Vue surface too.
packages/clients/tanstack-query/test/svelte-typing-test.ts (1)
41-86: Svelte store path verifiedThe Svelte variant hits the same mutation/query permutations via
get(...), so the store-based API surface is well covered.packages/clients/tanstack-query/src/react.ts (6)
39-39: LGTM! New imports support enhanced typing.The new imports
SelectIncludeOmitandTrimDelegateModelOperationsare correctly added to support the refined public API surface and delegate model operation filtering.Also applies to: 60-60
124-134: Well-designed conditional return type for mutateAsync.The
ModelMutationModelResulttype elegantly handles both single and array returns through theArraytype parameter. The genericT extends TArgsallows proper refinement at the call site, enabling operations likecreateManyAndReturnto correctly type their array results.
136-138: Clean public API mapping.The
ClientHookstype provides a clear, model-centric interface by mapping each model name (uncapitalized) to its corresponding query hooks.
142-252: Comprehensive hook interface with delegate model filtering.The
ModelQueryHookstype definition is thorough and correctly:
- Wraps all hooks with
TrimDelegateModelOperationsto filter ineligible operations for delegate models- Uses
ModelMutationModelResultfor mutation hooks that return model data- Correctly sets
Array = trueforcreateManyAndReturnandupdateManyAndReturnoperations- Maintains consistency between query, mutation, and aggregate operations
257-261: Public API aligned with new ClientHooks type.The return type correctly changed to
ClientHooks<Schema>, aligning the public surface with the new typing layer while preserving runtime behavior.
424-431: Behavior is intentional and well-tested—no action needed.The default
getNextPageParam: () => undefinedis by design. When this function returnsundefined(ornullin v5), TanStack Query interprets it as "no next page" and stops pagination. The test at react-query.test.tsx (lines 121–132) explicitly verifies this: callinguseInfiniteFindManywithgetNextPageParam: () => nullproduces a single page andhasNextPage = false. This safe-by-default approach requires users to explicitly configure pagination via options if they want multiple pages. The pattern is consistent across all three framework implementations (React, Vue, Svelte), confirming it's intentional.packages/clients/tanstack-query/src/vue.ts (6)
33-33: LGTM! Imports consistent with React implementation.The same imports are correctly added to support the Vue typing improvements.
Also applies to: 55-55
85-99: Proper Vue reactivity support for option types.The option types are correctly wrapped with
MaybeRefOrGetterandUnwrapRefto support Vue's reactive values, allowing users to pass refs or computed values as options.
103-113: ModelMutationModelResult correctly adapted for Vue.The type mirrors the React implementation but uses Vue's
ModelMutationResultreturn type, maintaining consistency across framework integrations.
115-196: Vue hook interface matches React implementation.The
ClientHooksandModelQueryHookstypes are correctly adapted for Vue:
- Properly wrapped with
TrimDelegateModelOperations- Uses Vue-specific option types with reactivity support
- Excludes Suspense variants (not applicable to Vue)
- Maintains same operation coverage as React
201-205: Public API updated consistently with React.The return type correctly changed to
ClientHooks<Schema>, maintaining parity with the React implementation.
323-332: Same pagination default as React—verify intent.Line 332 applies the same default
getNextPageParambehavior as React. This consistency is good, but the same verification question applies: confirm that disabling pagination by default when options are omitted is the intended behavior.This mirrors the React implementation behavior—refer to the verification script in the react.ts review.
|
Claude finished @ymc9's task —— View job Code Review: TanStack Query Mutation Typing ImprovementsI've completed a comprehensive review of this PR that improves mutation Todo List:
🎯 SummaryThis is an excellent PR that addresses a critical typing issue with ✅ Strengths1. Consistent Architecture
2. Excellent Type Safety
3. Comprehensive Test Coverage
4. Clean Implementation
🔧 Technical Implementation DetailsType System ImprovementsThe core improvement is in export type TrimDelegateModelOperations<
Schema extends SchemaDef,
Model extends GetModels<Schema>,
T extends Record<string, unknown>,
> = IsDelegateModel<Schema, Model> extends true ? Omit<T, OperationsIneligibleForDelegateModels> : T;This type correctly handles delegate models by removing operations that aren't applicable to them. Mutation Typing EnhancementThe key innovation is the export type ModelMutationModelResult<...> = Omit<ModelMutationResult<...>, 'mutateAsync'> & {
mutateAsync<T extends TArgs>(
args: T,
options?: ModelMutationOptions<ModelResult<Schema, Model, T>, T>,
): Promise<Array extends true ? ModelResult<Schema, Model, T>[] : ModelResult<Schema, Model, T>>;
};This provides precise typing that infers the return type based on the actual arguments passed to 🛡️ Security & Performance AssessmentSecurity: ✅ No Concerns
Performance: ✅ Excellent
🧪 Test Quality AssessmentComprehensive CoverageThe new typing tests (
📦 Package ManagementThe version bump to 🚀 Minor Suggestions for Future Improvements
🎯 ConclusionThis is a high-quality PR that significantly improves the developer experience without any drawbacks:
The implementation is clean, well-tested, and follows best practices. This PR represents exactly the kind of improvement that makes ZenStack's TanStack Query integration more developer-friendly and type-safe. Recommendation: ✅ Approve and merge |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
packages/clients/tanstack-query/src/svelte.ts (1)
112-124:mutateAsyncreturn type must include| undefined.The
mutateAsyncmethod promises to returnModelResult(single or array), but the underlyingfetcherfunction can returnundefinedwhen read-back policies reject (seepackages/clients/tanstack-query/src/utils/common.tsLine 135). Consumers who callmutateAsyncand directly access properties on the result will encounter runtime errors whenundefinedis returned.Restore the possibility of
undefined:export type ModelMutationModelResult< Schema extends SchemaDef, Model extends GetModels<Schema>, TArgs extends SelectIncludeOmit<Schema, Model, boolean>, Array extends boolean = false, > = Readable< Omit<UnwrapStore<ModelMutationResult<ModelResult<Schema, Model, TArgs>, TArgs>>, 'mutateAsync'> & { mutateAsync<T extends TArgs>( args: T, options?: ModelMutationOptions<ModelResult<Schema, Model, T>, T>, - ): Promise<Array extends true ? ModelResult<Schema, Model, T>[] : ModelResult<Schema, Model, T>>; + ): Promise<(Array extends true ? ModelResult<Schema, Model, T>[] : ModelResult<Schema, Model, T>) | undefined>; } >;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/clients/tanstack-query/src/react.ts(10 hunks)packages/clients/tanstack-query/src/svelte.ts(8 hunks)packages/clients/tanstack-query/src/utils/common.ts(1 hunks)packages/clients/tanstack-query/src/vue.ts(9 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
packages/clients/tanstack-query/src/react.ts (3)
packages/orm/src/client/crud-types.ts (3)
SelectIncludeOmit(453-457)ModelResult(135-179)FindArgs(648-664)packages/clients/tanstack-query/src/utils/types.ts (1)
TrimDelegateModelOperations(24-28)packages/clients/tanstack-query/src/utils/common.ts (3)
fetcher(128-150)getQueryKey(171-181)makeUrl(202-214)
packages/clients/tanstack-query/src/svelte.ts (4)
packages/clients/tanstack-query/src/react.ts (8)
ModelMutationModelResult(124-134)ModelMutationResult(122-122)ModelMutationOptions(119-120)ClientHooks(136-138)ModelQueryHooks(142-252)ModelQueryOptions(94-94)ModelQueryResult(96-96)useInternalMutation(483-544)packages/orm/src/client/crud-types.ts (6)
SelectIncludeOmit(453-457)ModelResult(135-179)FindUniqueArgs(669-671)SelectSubset(509-515)FindArgs(648-664)Subset(505-507)packages/clients/tanstack-query/src/utils/types.ts (1)
TrimDelegateModelOperations(24-28)packages/clients/tanstack-query/src/utils/common.ts (3)
fetcher(128-150)makeUrl(202-214)ExtraMutationOptions(81-96)
packages/clients/tanstack-query/src/vue.ts (5)
packages/clients/tanstack-query/src/utils/common.ts (1)
fetcher(128-150)packages/schema/src/schema.ts (2)
SchemaDef(10-18)GetModels(109-109)packages/orm/src/client/crud-types.ts (2)
SelectIncludeOmit(453-457)ModelResult(135-179)packages/clients/tanstack-query/src/utils/types.ts (1)
TrimDelegateModelOperations(24-28)packages/common-helpers/src/lower-case-first.ts (1)
lowerCaseFirst(1-3)
⏰ 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: claude-review
- GitHub Check: build-test (20.x, postgresql)
- GitHub Check: build-test (20.x, sqlite)
🔇 Additional comments (3)
packages/clients/tanstack-query/src/svelte.ts (1)
349-357: Good defensive default for infinite query options.Providing a safe default
getNextPageParamwhenoptionsisundefinedprevents runtime errors and simplifies the API surface.packages/clients/tanstack-query/src/vue.ts (1)
323-331: Good defensive default for infinite query options.Providing a safe default
getNextPageParamwhenoptionsisundefinedprevents runtime errors and improves API ergonomics.packages/clients/tanstack-query/src/react.ts (1)
424-430: Good defensive default for infinite query options.Providing a safe default
getNextPageParamwhenoptionsisundefinedprevents runtime errors and improves the developer experience.
Summary by CodeRabbit
New Features
Improvements
Tests