diff --git a/.changeset/release-settled-mutation-retryer.md b/.changeset/release-settled-mutation-retryer.md new file mode 100644 index 0000000000..e2beaaf5be --- /dev/null +++ b/.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. diff --git a/packages/query-core/src/__tests__/mutations.test.tsx b/packages/query-core/src/__tests__/mutations.test.tsx index 506f404fbf..8ad7f64477 100644 --- a/packages/query-core/src/__tests__/mutations.test.tsx +++ b/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') + }) }) diff --git a/packages/query-core/src/mutation.ts b/packages/query-core/src/mutation.ts index 2483b56336..6682ce3cce 100644 --- a/packages/query-core/src/mutation.ts +++ b/packages/query-core/src/mutation.ts @@ -165,8 +165,11 @@ export class Mutation< continue(): Promise { 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) } }