fix(types): propagate generic type params to useMutationState select callback#10373
fix(types): propagate generic type params to useMutationState select callback#10373Zelys-DFKH wants to merge 6 commits into
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 Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThis PR propagates generic type parameters into the useMutationState select callback by adding MutationTypeFromResult and a TMutation generic across adapters and tests. ChangesuseMutationState generic propagation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Suggested reviewers
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 unit tests (beta)
⚔️ Resolve merge conflicts
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 57bb423
☁️ Nx Cloud last updated this comment at |
![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
🧹 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>
2 participants
🎯 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