Skip to content

fix(types): onMutateResult is always defined in onSuccess callback#9677

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
TkDodo merged 2 commits into from
Sep 23, 2025
Merged

fix(types): onMutateResult is always defined in onSuccess callback #9677

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7aad042
fix(types): onMutateResult is always defined in onSuccess callback
TkDodo Sep 23, 2025
0ee6f3d
chore: fix tests
TkDodo Sep 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions packages/angular-query-experimental/src/__tests__/mutation-options.test-d.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -67,9 +67,7 @@ describe('mutationOptions', () => {
return { name: 'onMutateResult' }
},
onSuccess: (_data, _variables, onMutateResult) => {
expectTypeOf(onMutateResult).toEqualTypeOf<
{ name: string } | undefined
>()
expectTypeOf(onMutateResult).toEqualTypeOf<{ name: string }>()
},
})
})
Expand Down
96 changes: 96 additions & 0 deletions packages/query-core/src/__tests__/mutation.test-d.tsx
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
import { describe, expectTypeOf, it } from 'vitest'
import { QueryClient } from '../queryClient'
import { MutationObserver } from '../mutationObserver'

describe('mutation', () => {
describe('onMutate', () => {
it('should have onMutateResult undefined if undefined is explicitly returned', () => {
new MutationObserver(new QueryClient(), {
mutationFn: (variables: number) => {
return Promise.resolve(String(variables))
},
onMutate: () => {
return undefined
},
onSuccess: (data, variables, onMutateResult) => {
expectTypeOf(data).toEqualTypeOf<string>()
expectTypeOf(variables).toEqualTypeOf<number>()
expectTypeOf(onMutateResult).toEqualTypeOf<undefined>()
},
onError: (_data, _variables, onMutateResult) => {
expectTypeOf(onMutateResult).toEqualTypeOf<undefined>()
},
onSettled: (_data, _error, _variables, onMutateResult) => {
expectTypeOf(onMutateResult).toEqualTypeOf<undefined>()
},
})
})

it('should have unknown onMutateResult if onMutate is left out', () => {
new MutationObserver(new QueryClient(), {
mutationFn: (variables: number) => {
return Promise.resolve(String(variables))
},
onSuccess: (_data, _variables, onMutateResult) => {
expectTypeOf(onMutateResult).toEqualTypeOf<unknown>()
},
onError: (_data, _variables, onMutateResult) => {
expectTypeOf(onMutateResult).toEqualTypeOf<unknown>()
},
onSettled: (_data, _error, _variables, onMutateResult) => {
expectTypeOf(onMutateResult).toEqualTypeOf<unknown>()
},
})
})

it('should infer onMutateResult', () => {
new MutationObserver(new QueryClient(), {
mutationFn: (variables: number) => {
return Promise.resolve(String(variables))
},
onMutate: () => {
return Promise.resolve({ foo: 'bar' })
},
onSuccess: (_data, _variables, onMutateResult) => {
expectTypeOf(onMutateResult).toEqualTypeOf<{ foo: string }>()
},
onError: (_data, _variables, onMutateResult) => {
expectTypeOf(onMutateResult).toEqualTypeOf<
{ foo: string } | undefined
>()
},
onSettled: (_data, _error, _variables, onMutateResult) => {
expectTypeOf(onMutateResult).toEqualTypeOf<
{ foo: string } | undefined
>()
},
})
})

it('should include undefined in the union if explicitly returned', () => {
new MutationObserver(new QueryClient(), {
mutationFn: (variables: number) => {
return Promise.resolve(String(variables))
},
onMutate: () => {
return Math.random() > 0.5 ? { foo: 'bar' } : undefined
},
onSuccess: (_data, _variables, onMutateResult) => {
expectTypeOf(onMutateResult).toEqualTypeOf<
{ foo: string } | undefined
>()
},
onError: (_data, _variables, onMutateResult) => {
expectTypeOf(onMutateResult).toEqualTypeOf<
{ foo: string } | undefined
>()
},
onSettled: (_data, _error, _variables, onMutateResult) => {
expectTypeOf(onMutateResult).toEqualTypeOf<
{ foo: string } | undefined
>()
},
})
})
})
})
2 changes: 1 addition & 1 deletion packages/query-core/src/mutation.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -244,7 +244,7 @@ export class Mutation<
await this.options.onSuccess?.(
data,
variables,
this.state.context,
this.state.context!,

@coderabbitaicoderabbitaiBotSep 23, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

🧩 Analysis chain

Verify runtime safety of non-null assertion.

The non-null assertion on this.state.context! assumes that context is always defined when onSuccess is called. This needs verification that the mutation lifecycle guarantees context is set before onSuccess execution.

Let me verify the mutation execution flow to ensure this assertion is safe:

🏁 Script executed:

#!/bin/bash# Description: Verify mutation execution flow and context lifecycle# Check how context gets set in the mutation stateecho"=== Context assignment in mutation execution ==="
rg -nP --type=ts -A5 -B5 'context.*=' packages/query-core/src/mutation.ts
# Check all places where onSuccess is called to verify context stateecho -e "\n=== onSuccess invocation contexts ==="
rg -nP --type=ts -A3 -B3 'onSuccess\?\(' packages/query-core/src/mutation.ts
# Look for any paths where context might remain undefinedecho -e "\n=== Mutation state initialization ==="
rg -nP --type=ts -A10 'context:\s*undefined' packages/query-core/src/mutation.ts

Length of output: 891

🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"=== mutation.ts (180..280) ==="
nl -ba packages/query-core/src/mutation.ts | sed -n '180,280p'||trueechoecho"=== Search for 'onSuccess' across the repo ==="
rg -n --hidden --no-ignore 'onSuccess'||trueechoecho"=== Search for 'options.onSuccess' and 'this.options.onSuccess' patterns ==="
rg -n "options\.onSuccess"||true
rg -n "this\.options\.onSuccess"||trueechoecho"=== Search for 'onSuccess(' invocation patterns ==="
rg -n "onSuccess\("||trueechoecho"=== Show mutation.ts dispatch handling (lines around 220..340) ==="
nl -ba packages/query-core/src/mutation.ts | sed -n '220,340p'||true

Length of output: 42070

🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"=== show lines with 'state.context' and surrounding ==="
rg -n --hidden --no-ignore "state\.context"||true
rg -n --hidden --no-ignore "this\.state\.context"||trueechoecho"=== mutation.ts (200..260) ==="
sed -n '200,260p' packages/query-core/src/mutation.ts ||trueechoecho"=== mutation.ts (232..260) (redundant) ==="
sed -n '232,260p' packages/query-core/src/mutation.ts ||trueechoecho"=== mutation.ts (240..260) to show onSuccess calls ==="
sed -n '240,260p' packages/query-core/src/mutation.ts ||true

Length of output: 4431

🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"=== Show types.ts around mutation-related types (1100..1300) ==="
sed -n '1100,1300p' packages/query-core/src/types.ts ||trueechoecho"=== Search for 'MutationFunctionContext' definition ==="
rg -n "MutationFunctionContext" packages/query-core/src -S ||true
sed -n '1,240p' packages/query-core/src/mutation.ts ||trueechoecho"=== Show MutationOptions in types.ts (search) ==="
rg -n "type MutationOptions|interface MutationOptions|MutationOptions" packages/query-core/src/types.ts -n -S ||true
rg -n "onMutate" packages/query-core/src/types.ts -S ||true
sed -n '1200,1280p' packages/query-core/src/types.ts ||true

Length of output: 16690

🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"=== mutation.ts (1..200) ==="
sed -n '1,200p' packages/query-core/src/mutation.ts ||trueechoecho"=== mutation.ts (200..500) ==="
sed -n '200,500p' packages/query-core/src/mutation.ts ||trueechoecho"=== Search for '#dispatch(' definition or method ==="
rg -n "dispatch\(" packages/query-core/src/mutation.ts ||trueechoecho"=== Search for 'reducer' or 'state = ' patterns ==="
rg -n "reducer|state =|switch (action.type)" packages/query-core/src/mutation.ts ||true

Length of output: 10875

Make onSuccess's onMutateResult optional or remove the non-null assertion

this.state.context can be undefined when no onMutate ran or when onMutate returns undefined (getDefaultState sets context to undefined and the code only dispatches a new context if it differs), so the non-null assertion at packages/query-core/src/mutation.ts:247 is unsafe.

  • Change MutationOptions.onSuccess third parameter to onMutateResult: TOnMutateResult | undefined in packages/query-core/src/types.ts (align with onError/onSettled and MutateOptions).
  • Remove the '!' in packages/query-core/src/mutation.ts (pass this.state.context directly) or ensure context is always set before calling options.onSuccess.
🤖 Prompt for AI Agents
In packages/query-core/src/mutation.ts around line 247, the code uses a non-null
assertion on this.state.context which can be undefined; update the
MutationOptions.onSuccess third parameter type in
packages/query-core/src/types.ts to accept TOnMutateResult | undefined (matching
onError/onSettled and MutateOptions), then remove the '!' in mutation.ts and
pass this.state.context directly to options.onSuccess; ensure callers handle
undefined context (no other runtime guarantees needed) so the type and usage are
consistent.

mutationFnContext,
)

Expand Down
4 changes: 2 additions & 2 deletions packages/query-core/src/types.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -1113,11 +1113,11 @@ export interface MutationOptions<
onMutate?: (
variables: TVariables,
context: MutationFunctionContext,
) => Promise<TOnMutateResult | undefined> | TOnMutateResult | undefined
) => Promise<TOnMutateResult> | TOnMutateResult
onSuccess?: (
data: TData,
variables: TVariables,
onMutateResult: TOnMutateResult | undefined,
onMutateResult: TOnMutateResult,
context: MutationFunctionContext,
) => Promise<unknown> | unknown
onError?: (
Expand Down
4 changes: 1 addition & 3 deletions packages/react-query/src/__tests__/mutationOptions.test-d.tsx
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -63,9 +63,7 @@ describe('mutationOptions', () => {
return { name: 'onMutateResult' }
},
onSuccess: (_data, _variables, onMutateResult) => {
expectTypeOf(onMutateResult).toEqualTypeOf<
{ name: string } | undefined
>()
expectTypeOf(onMutateResult).toEqualTypeOf<{ name: string }>()
},
})
})
Expand Down
Loading