From a9299409785cfd87c14c3d636bfd32b866b6d35a Mon Sep 17 00:00:00 2001 From: Nicholas Date: Wed, 23 Sep 2026 14:44:00 -0700 Subject: [PATCH] fix(svelte-query): unsubscribe the observer when its effect root is disposed The initial subscription had no teardown outside a component, so an observer created inside $effect.root survived the disposer. --- .../svelte-query-effect-root-cleanup.md | 5 + .../src/createBaseQuery.svelte.ts | 5 + .../observer-cleanup.svelte.test.ts | 186 ++++++++++++++++++ 3 files changed, 196 insertions(+) create mode 100644 .changeset/svelte-query-effect-root-cleanup.md create mode 100644 packages/svelte-query/tests/createQuery/observer-cleanup.svelte.test.ts diff --git a/.changeset/svelte-query-effect-root-cleanup.md b/.changeset/svelte-query-effect-root-cleanup.md new file mode 100644 index 00000000000..7ff8aaef8ca --- /dev/null +++ b/.changeset/svelte-query-effect-root-cleanup.md @@ -0,0 +1,5 @@ +--- +'@tanstack/svelte-query': patch +--- + +fix(svelte-query): unsubscribe the observer when an `$effect.root` holding a query is disposed diff --git a/packages/svelte-query/src/createBaseQuery.svelte.ts b/packages/svelte-query/src/createBaseQuery.svelte.ts index 401a4718140..4161c709980 100644 --- a/packages/svelte-query/src/createBaseQuery.svelte.ts +++ b/packages/svelte-query/src/createBaseQuery.svelte.ts @@ -92,6 +92,11 @@ export function createBaseQuery< return unsubscribe }, ) + // ...and because the above only returns a cleanup from its second run onwards, tear the initial + // subscription down with an effect of its own, so disposing an `$effect.root` unsubscribes it. + try { + $effect.pre(() => () => unsubscribe()) + } catch (e) {} // ...and finally also cleanup via onDestroy because that one runs on the server whereas $effect.pre does not. // (in a try-catch because it theoretically can be called in a non-component context - that should not happen // but it would be a breaking change technically to error out here. SSR-safe because this wouldn't be called during SSR if it was not in a component) diff --git a/packages/svelte-query/tests/createQuery/observer-cleanup.svelte.test.ts b/packages/svelte-query/tests/createQuery/observer-cleanup.svelte.test.ts new file mode 100644 index 00000000000..6fd3c1bb689 --- /dev/null +++ b/packages/svelte-query/tests/createQuery/observer-cleanup.svelte.test.ts @@ -0,0 +1,186 @@ +import { render } from '@testing-library/svelte' +import { flushSync } from 'svelte' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { + QueryClient, + createInfiniteQuery, + createQuery, +} from '../../src/index.js' +import Base from './Base.svelte' + +describe('effect root query cleanup', () => { + let client: QueryClient + + beforeEach(() => { + vi.useFakeTimers() + client = new QueryClient({ defaultOptions: { queries: { retry: false } } }) + }) + + afterEach(() => { + client.clear() + vi.useRealTimers() + }) + + describe.each(['query', 'infinite query'] as const)('%s', (kind) => { + it.each(['success', 'error', 'disabled'] as const)( + 'removes the observer after disposing a %s query root', + async (state) => { + const key = ['root-cleanup', kind, state] + const queryFn = vi.fn(() => { + if (state === 'error') + return Promise.reject(new Error('expected failure')) + return Promise.resolve('data') + }) + const options = () => ({ + queryKey: key, + queryFn, + enabled: state !== 'disabled', + gcTime: 10, + initialPageParam: 0, + getNextPageParam: () => undefined, + }) + const dispose = $effect.root(() => { + if (kind === 'query') createQuery(options, () => client) + else createInfiniteQuery(options, () => client) + }) + + try { + flushSync() + await vi.advanceTimersByTimeAsync(0) + const query = client.getQueryCache().find({ queryKey: key })! + expect(query.getObserversCount()).toBe(1) + expect(query.state.status).toBe( + state === 'disabled' ? 'pending' : state, + ) + + dispose() + + expect.soft(query.getObserversCount()).toBe(0) + await vi.advanceTimersByTimeAsync(20) + expect(client.getQueryCache().find({ queryKey: key })).toBeUndefined() + } finally { + dispose() + } + }, + ) + + it('does not retain an observer when disposed before effects run', () => { + const key = ['early-root-cleanup', kind] + const options = () => ({ + queryKey: key, + enabled: false, + initialPageParam: 0, + getNextPageParam: () => undefined, + }) + const dispose = $effect.root(() => { + if (kind === 'query') createQuery(options, () => client) + else createInfiniteQuery(options, () => client) + }) + + try { + dispose() + flushSync() + expect( + client.getQueryCache().find({ queryKey: key })!.getObserversCount(), + ).toBe(0) + } finally { + dispose() + } + }) + }) + + it('aborts a signal-consuming request when its root is disposed', () => { + let signal: AbortSignal | undefined + const dispose = $effect.root(() => { + createQuery( + () => ({ + queryKey: ['pending-root'], + queryFn: (context) => { + signal = context.signal + return new Promise(() => {}) + }, + }), + () => client, + ) + }) + + try { + flushSync() + expect(signal?.aborted).toBe(false) + dispose() + expect(signal?.aborted).toBe(true) + } finally { + dispose() + } + }) + + it('stops interval refetches when its root is disposed', async () => { + const queryFn = vi.fn(() => Promise.resolve('data')) + const dispose = $effect.root(() => { + createQuery( + () => ({ + queryKey: ['polling-root'], + queryFn, + refetchInterval: 10, + refetchIntervalInBackground: true, + }), + () => client, + ) + }) + + try { + flushSync() + await vi.advanceTimersByTimeAsync(0) + expect(queryFn).toHaveBeenCalledTimes(1) + dispose() + await vi.advanceTimersByTimeAsync(30) + expect(queryFn).toHaveBeenCalledTimes(1) + } finally { + dispose() + } + }) + + it('cleans up a normal component subscription on unmount', async () => { + const key = ['component-control'] + const view = render(Base, { + queryClient: client, + options: () => ({ + queryKey: key, + queryFn: () => Promise.resolve('data'), + }), + }) + await vi.advanceTimersByTimeAsync(0) + const query = client.getQueryCache().find({ queryKey: key })! + expect(query.getObserversCount()).toBe(1) + await view.unmount() + expect(query.getObserversCount()).toBe(0) + }) + + it('cleans up after the subscription has changed clients', () => { + const replacement = new QueryClient() + let selected = $state(client) + const key = ['changed-client-control'] + const dispose = $effect.root(() => { + createQuery( + () => ({ queryKey: key, enabled: false }), + () => selected, + ) + }) + + try { + flushSync() + const original = client.getQueryCache().find({ queryKey: key })! + expect(original.getObserversCount()).toBe(1) + selected = replacement + flushSync() + expect(original.getObserversCount()).toBe(0) + const next = replacement.getQueryCache().find({ queryKey: key })! + expect(next.getObserversCount()).toBe(1) + dispose() + expect(next.getObserversCount()).toBe(0) + } finally { + dispose() + replacement.clear() + } + }) +})