fix(query-core): release the retryer once a mutation settles (#11218)

* fix(query-core): release the retryer once a mutation settles

Mutation.execute() never cleared #retryer after settling, so the settled
retryer's promise kept that mutation's result, variables and context
reachable for as long as the MutationCache retained the Mutation. Mirror
the treatment Query.fetch() received in #11163, guarded by an identity
check so a mutation re-executed from a cache callback keeps its own
retryer.

Releasing the retryer alone would change continue(): a settled mutation
no longer has a retryer to continue, so it would fall through to
execute() and run the mutationFn a second time. Both internal callers
filter on state.isPaused and never reach a settled mutation, but
continue() is reachable directly, so the fallback is now gated on the
mutation still being pending -- the same condition execute() already uses
to detect a restored mutation.

Fixes #11216

* test(query-core): assert the settled retryer is actually released

The previous tests only guarded the continue() gating and passed on main
too, so they did not demonstrate the fix. continue() is the only reader
of #retryer outside execute(): while a settled retryer is still held it
hands back that retryer's promise, which resolves with the raw result it
closed over (or rejects with its error). Both new tests fail on main and
pass with the retryer released.

---------

Co-authored-by: Dominik Dorfmeister 🔮 <office@dorfmeister.cc>

Author: iamshahid1997

.changeset/release-settled-mutation-retryer.md

@@ -0,0 +1,5 @@
+---
+'@tanstack/query-core': patch
+---
+
+Release a mutation's retryer once its execution settles, so the settled promise no longer keeps that mutation's result, variables and context in memory for as long as the mutation cache retains it.

packages/query-core/src/__tests__/mutations.test.tsx

@@ -1191,4 +1191,81 @@ describe('mutations', () => {
 
     expect(queryClient.getMutationCache().getAll()).toHaveLength(1)
   })
+
+  it('should release the retryer once a mutation settles', async () => {
+    const observer = new MutationObserver(queryClient, {
+      mutationFn: (text: string) => sleep(10).then(() => text),
+    })
+
+    observer.mutate('data')
+    await vi.advanceTimersByTimeAsync(10)
+
+    const mutation = queryClient.getMutationCache().getAll()[0]!
+    expect(mutation.state.status).toBe('success')
+
+    // continue() is the only reader of the retryer outside execute(): while a
+    // settled retryer is still referenced it hands back that retryer's promise,
+    // which resolves with the raw result it closed over
+    await expect(mutation.continue()).resolves.toBeUndefined()
+  })
+
+  it('should release the retryer of a mutation that settled with an error', async () => {
+    const observer = new MutationObserver(queryClient, {
+      mutationFn: () => sleep(10).then(() => Promise.reject(new Error('oops'))),
+    })
+
+    observer.mutate(undefined).catch(() => undefined)
+    await vi.advanceTimersByTimeAsync(10)
+
+    const mutation = queryClient.getMutationCache().getAll()[0]!
+    expect(mutation.state.status).toBe('error')
+
+    // a retained retryer would hand back its rejected promise here
+    await expect(mutation.continue()).resolves.toBeUndefined()
+  })
+
+  it('should not re-execute a settled mutation when it is continued', async () => {
+    const mutationFn = vi.fn(() => sleep(10).then(() => 'data'))
+    const observer = new MutationObserver(queryClient, { mutationFn })
+
+    observer.mutate()
+    await vi.advanceTimersByTimeAsync(10)
+
+    const mutation = queryClient.getMutationCache().getAll()[0]!
+    expect(mutation.state.status).toBe('success')
+    expect(mutationFn).toHaveBeenCalledTimes(1)
+
+    await mutation.continue()
+    await vi.advanceTimersByTimeAsync(10)
+
+    expect(mutationFn).toHaveBeenCalledTimes(1)
+    expect(mutation.state.status).toBe('success')
+  })
+
+  it('should still continue a restored paused mutation that has no retryer', async () => {
+    const mutationFn = vi.fn(() => sleep(10).then(() => 'data'))
+    // a mutation restored from a dehydrated pending state has no retryer yet
+    const mutation = queryClient.getMutationCache().build(
+      queryClient,
+      { mutationFn },
+      {
+        context: undefined,
+        data: undefined,
+        error: null,
+        failureCount: 0,
+        failureReason: null,
+        isPaused: true,
+        status: 'pending',
+        variables: undefined,
+        submittedAt: Date.now(),
+      },
+    )
+
+    const continued = mutation.continue()
+    await vi.advanceTimersByTimeAsync(10)
+    await continued
+
+    expect(mutationFn).toHaveBeenCalledTimes(1)
+    expect(mutation.state.status).toBe('success')
+  })
 })

packages/query-core/src/mutation.ts

@@ -165,8 +165,11 @@ export class Mutation<
   continue(): Promise<unknown> {
     return (
       this.#retryer?.continue() ??
-      // continuing a mutation assumes that variables are set, mutation must have been dehydrated before
-      this.execute(this.state.variables!)
+      // continuing a mutation assumes that variables are set, mutation must have been dehydrated before.
+      // a settled mutation has no retryer to continue and must not run again
+      (this.state.status === 'pending'
+        ? this.execute(this.state.variables!)
+        : Promise.resolve())
     )
   }
 
@@ -181,7 +184,7 @@ export class Mutation<
       mutationKey: this.options.mutationKey,
     } satisfies MutationFunctionContext
 
-    this.#retryer = createRetryer({
+    const retryer = (this.#retryer = createRetryer({
       fn: () => {
         if (!this.options.mutationFn) {
           return Promise.reject(new Error('No mutationFn found'))
@@ -200,10 +203,10 @@ export class Mutation<
       retryDelay: this.options.retryDelay,
       networkMode: this.options.networkMode,
       canRun: () => this.#mutationCache.canRun(this),
-    })
+    }))
 
     const restored = this.state.status === 'pending'
-    const isPaused = !this.#retryer.canStart()
+    const isPaused = !retryer.canStart()
 
     try {
       if (restored) {
@@ -232,7 +235,7 @@ export class Mutation<
           })
         }
       }
-      const data = await this.#retryer.start()
+      const data = await retryer.start()
 
       // Notify cache callback
       await this.#mutationCache.config.onSuccess?.(
@@ -324,6 +327,11 @@ export class Mutation<
       this.#dispatch({ type: 'error', error: error as TError })
       throw error
     } finally {
+      // The settled retryer's promise would otherwise pin this mutation's
+      // result, variables and context for as long as the cache keeps it
+      if (this.#retryer === retryer) {
+        this.#retryer = undefined
+      }
       this.#mutationCache.runNext(this)
     }
   }