diff --git a/.changeset/mutation-flight-scoped-subscription.md b/.changeset/mutation-flight-scoped-subscription.md new file mode 100644 index 0000000000..4c525813c6 --- /dev/null +++ b/.changeset/mutation-flight-scoped-subscription.md @@ -0,0 +1,12 @@ +--- +'@tanstack/solid-query': patch +--- + +fix: scope useMutation's mutation-cache subscription to the flight. The +hook subscribed at mount with a bare reactive `client()` read (tripping +Solid's STRICT_READ_UNTRACKED dev diagnostic) and held the subscription +for its whole life even though the listener only matters while a mutation +is in flight. The subscription now starts in `run` against the same +client the mutation is built on and ends at settle: nothing reactive is +read at setup, the listener and the mutation can never sit on different +clients, and idle hooks hold no cache subscription. diff --git a/packages/solid-query/src/__tests__/useMutation.test.tsx b/packages/solid-query/src/__tests__/useMutation.test.tsx index bfaadc105d..b39dbbbbf7 100644 --- a/packages/solid-query/src/__tests__/useMutation.test.tsx +++ b/packages/solid-query/src/__tests__/useMutation.test.tsx @@ -105,6 +105,64 @@ describe('useMutation', () => { consoleMock.mockRestore() }) + it('should not emit a strict-read diagnostic on mount', () => { + const consoleMock = vi + .spyOn(console, 'warn') + .mockImplementation(() => undefined) + + function Page() { + const mutation = useMutation(() => ({ + mutationFn: () => Promise.resolve('mutation'), + })) + + return + } + + renderWithClient(queryClient, () => ) + + expect( + consoleMock.mock.calls.filter((args) => + args.some((arg) => String(arg).includes('STRICT_READ_UNTRACKED')), + ), + ).toEqual([]) + + consoleMock.mockRestore() + }) + + it('should hold no mutation-cache subscription while idle', async () => { + function Page() { + const mutation = useMutation(() => ({ + mutationFn: () => sleep(10).then(() => 'result'), + })) + + return ( +
+ + status: {mutation.status} +
+ ) + } + + const cache = queryClient.getMutationCache() + const listeners = () => + (cache as unknown as { listeners: Set }).listeners.size + const idle = listeners() + + const rendered = renderWithClient(queryClient, () => ) + // Mounting alone subscribes nothing. + expect(listeners()).toBe(idle) + + fireEvent.click(rendered.getByRole('button', { name: /mutate/i })) + await vi.advanceTimersByTimeAsync(0) + // In flight: exactly one flight listener. + expect(listeners()).toBe(idle + 1) + + await vi.advanceTimersByTimeAsync(10) + expect(rendered.getByText('status: success')).toBeInTheDocument() + // Settled: the flight listener is gone. + expect(listeners()).toBe(idle) + }) + it('should be able to call `onSuccess` and `onSettled` after each successful mutate', async () => { let countRef = 0 const [count, setCount] = createSignal(0) diff --git a/packages/solid-query/src/useMutation.ts b/packages/solid-query/src/useMutation.ts index 1d0b2f7b5c..f875340a20 100644 --- a/packages/solid-query/src/useMutation.ts +++ b/packages/solid-query/src/useMutation.ts @@ -114,20 +114,20 @@ export function useMutation< * Flight metadata (retry failures, offline pause) lives on the Mutation * instance and changes during the await gap; a version bump per cache * event for the active mutation keeps reads current without cloning - * observer results. + * observer results. The subscription is per-flight, created in `run` + * against the same client the mutation is built on: nothing reactive is + * read at setup (no client read to untrack), the listener and the + * mutation can never sit on different clients, and an idle hook holds no + * cache subscription at all. */ let activeMutation: Mutation | null = null + let unsubscribeFlight: (() => void) | null = null const [flightVersion, setFlightVersion] = createSignal(0) - if (!isServer) { - const unsubscribe = client() - .getMutationCache() - .subscribe((event) => { - if ('mutation' in event && event.mutation === activeMutation) { - setFlightVersion((v) => v + 1) - } - }) - onCleanup(unsubscribe) - } + if (!isServer) + onCleanup(() => { + unsubscribeFlight?.() + unsubscribeFlight = null + }) async function* run( variables: TVariables, @@ -144,19 +144,33 @@ export function useMutation< if (!isServer) setFlight({ variables }) opts.onMutate?.(variables, callbackContext) - const mutation = client() - .getMutationCache() - .build(client(), { - ...opts, - // Options-level callbacks re-run below, inside the transaction — - // executing them here (inside the await gap) would let their - // cache writes and invalidations escape the atomic settle. - onMutate: undefined, - onSuccess: undefined, - onError: undefined, - onSettled: undefined, - }) as unknown as Mutation + const c = untrack(client) + const mutation = c.getMutationCache().build(c, { + ...opts, + // Options-level callbacks re-run below, inside the transaction — + // executing them here (inside the await gap) would let their + // cache writes and invalidations escape the atomic settle. + onMutate: undefined, + onSuccess: undefined, + onError: undefined, + onSettled: undefined, + }) as unknown as Mutation activeMutation = mutation + if (!isServer) { + // A rapid re-mutate replaces the previous flight's listener. + unsubscribeFlight?.() + unsubscribeFlight = c.getMutationCache().subscribe((event) => { + if ('mutation' in event && event.mutation === activeMutation) { + setFlightVersion((v) => v + 1) + } + }) + } + const settleFlight = () => { + if (activeMutation !== mutation) return + activeMutation = null + unsubscribeFlight?.() + unsubscribeFlight = null + } /** * Failure settle, shared by both paths. Callback routing mirrors @@ -202,7 +216,7 @@ export function useMutation< variables, submittedAt: mutation.state.submittedAt, }) - activeMutation = null + settleFlight() throw error } @@ -250,7 +264,7 @@ export function useMutation< variables, submittedAt: mutation.state.submittedAt, }) - activeMutation = null + settleFlight() return result }