fix(types): propagate generic type params to useMutationState select callback - #10373
Conversation
…callback When TResult is a typed MutationState, the select callback parameter now receives the correctly typed Mutation instead of the base Mutation type. Adds a second type param TMutation (defaulting to MutationTypeFromResult<TResult>) to MutationStateOptions across all five framework adapters. Uses a non-distributive conditional type to avoid union expansion when TResult is unresolved. Fixes TanStack#9825
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
🚧 Files skipped from review as they are similar to previous changes (10)
Included review availability: Your plan includes up to 10 reviews per rolling hour; 6 remain after this review. 📝 WalkthroughWalkthrough
ChangesuseMutationState generic propagation
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This localized type-inference change improves the select callback types across the supported adapters without introducing an actionable merge-blocking risk; it is merge-ready after normal checks and review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
|
View your CI Pipeline Execution ↗ for commit 7d161c9
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/svelte-query/src/useMutationState.svelte.ts (1)
34-38: Simplify callback cast by casting the mutation value instead.The current double-cast at line 35 obscures that
options.selectexpectsTMutation(per the type definition). Castingmutation as TMutationat the call site is clearer and type-safe.Suggested refactor
- (options.select - ? (options.select as unknown as (mutation: Mutation) => TResult)( - mutation, - ) - : mutation.state) as TResult, + (options.select + ? options.select(mutation as TMutation) + : mutation.state) as TResult,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/svelte-query/src/useMutationState.svelte.ts` around lines 34 - 38, The double-cast of options.select is confusing; instead cast the mutation value to the expected TMutation and call the select callback directly. Update the conditional expression in useMutationState (where options.select is invoked) to call options.select(mutation as TMutation) and keep the fallback to mutation.state, ensuring the overall expression is still cast to TResult; reference the symbols options.select, mutation, TMutation, TResult, and mutation.state when making the change.
🤖 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/preact-query/src/__tests__/useMutationState.test-d.tsx`:
- Around line 1-8: ESLint import/order wants the local module import before
external package type imports; reorder the imports so the relative import of
useMutationState comes before the type-only import from `@tanstack/query-core`
(i.e., move "import { useMutationState } from '../useMutationState'" above the
"import type { Mutation, MutationState, MutationStatus } from
'@tanstack/query-core'").
---
Nitpick comments:
In `@packages/svelte-query/src/useMutationState.svelte.ts`:
- Around line 34-38: The double-cast of options.select is confusing; instead
cast the mutation value to the expected TMutation and call the select callback
directly. Update the conditional expression in useMutationState (where
options.select is invoked) to call options.select(mutation as TMutation) and
keep the fallback to mutation.state, ensuring the overall expression is still
cast to TResult; reference the symbols options.select, mutation, TMutation,
TResult, and mutation.state when making the change.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b193f4a2-1fbb-40ac-be61-462d3ccab642
📒 Files selected for processing (11)
.changeset/fix-mutation-state-generics.mdpackages/preact-query/src/__tests__/useMutationState.test-d.tsxpackages/preact-query/src/useMutationState.tspackages/react-query/src/__tests__/useMutationState.test-d.tsxpackages/react-query/src/useMutationState.tspackages/solid-query/src/__tests__/useMutationState.test-d.tsxpackages/solid-query/src/useMutationState.tspackages/svelte-query/src/types.tspackages/svelte-query/src/useMutationState.svelte.tspackages/svelte-query/tests/useMutationState/SelectExample.sveltepackages/vue-query/src/useMutationState.ts
Replace the double-cast `(fn as unknown as (m: Mutation) => TResult)(m)` with the direct `fn(m as TMutation)` across all five framework packages. The constraint `TMutation extends Mutation<any, any, any, any>` creates sufficient overlap for the single cast to compile cleanly on TS 5.4–6.0.
|
Hey team, appreciate you all. This has been sitting for a few weeks, and wanted to check in: is there anything blocking review, or should I prioritize other contributions? This one's pretty straightforward—just propagating generic params through No pressure either way—just want to make sure it's on the radar. |
|
thanks, sorry it took so long. It looks pretty similar to #10790 but that one is a bit simpler on the types? |
…order rule Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Hey @TkDodo — took a look at both. The ergonomic difference comes down to which entry point users reach for first. With #10790: // needs annotation on every call
useMutationState({ select: (m: Mutation<User, Error, Vars>) => m.state })With this PR: // one generic, no annotation needed
useMutationState<MutationState<User, Error, Vars>>({ select: (m) => m.state })Both work. The I also found a gap neither PR fixes: passing useMutationState({ filters: mutationOptions({ mutationKey: ['key'], mutationFn: () => Promise.resolve(5) }) })
// currently: Array<MutationState<unknown, Error, unknown, unknown>>
// should be: Array<MutationState<number, Error, void, unknown>>Proper fix needs a phantom type brand on If you want #10790's approach, I can drop |
|
alright, let’s go with your approach. can you fix the conflicts please |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…finition Export MutationTypeFromResult from types.ts so useMutationState.svelte.ts can import it instead of redefining it. Also normalises the else branch to plain Mutation, matching every other adapter. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
🎯 Changes
Closes #9825.
When you pass a typed
MutationState<TData, TError, TVariables>asTResult, theselectcallback now receivesMutation<TData, TError, TVariables, TContext>instead of the baseMutationtype. Before this change you had to cast manually:How it works
MutationStateOptionsgains a second type paramTMutationthat defaults toMutationTypeFromResult<TResult>:The tuple wrapper
[TResult] extends [...]makes the conditional non-distributive, which stops TypeScript from producing a union whenTResultis still unresolved. The second param keeps backward compatibility: callers that do not provideTResultat all get the sameMutationtype they always did.Applied across all five adapters: react, preact, solid, vue, and svelte.
✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
useMutationStateacross React, Preact, Solid, Svelte, and Vue integrations.selectcallback now receives the correctly typed mutation, including data, error, and variable types.