diff --git a/.changeset/tame-parrots-shave.md b/.changeset/tame-parrots-shave.md new file mode 100644 index 00000000000..2a425dd35fd --- /dev/null +++ b/.changeset/tame-parrots-shave.md @@ -0,0 +1,5 @@ +--- +'@tanstack/svelte-query': minor +--- + +feat(svelte-query): propagate errors from `createQuery`/`createInfiniteQuery` to the nearest `` when `throwOnError` is set diff --git a/docs/framework/svelte/quick-start.md b/docs/framework/svelte/quick-start.md index b5f04143b5c..76236a397e0 100644 --- a/docs/framework/svelte/quick-start.md +++ b/docs/framework/svelte/quick-start.md @@ -146,6 +146,6 @@ createQuery(() => ({ ``` - Errors can be caught and reset using Svelte's native `` component. - Set `throwOnError` option to `true` to make sure errors are thrown to the `` component. + Set `throwOnError` option to `true` on `createQuery`/`createInfiniteQuery` to make sure errors are thrown to the `` component. - Since property tracking is handled through Svelte's fine-grained reactivity, options like `notifyOnChangeProps` are not needed diff --git a/packages/svelte-query/src/createBaseQuery.svelte.ts b/packages/svelte-query/src/createBaseQuery.svelte.ts index 03fc6b28db4..28179169f22 100644 --- a/packages/svelte-query/src/createBaseQuery.svelte.ts +++ b/packages/svelte-query/src/createBaseQuery.svelte.ts @@ -1,3 +1,4 @@ +import { shouldThrowError } from '@tanstack/query-core' import { useIsRestoring } from './useIsRestoring.js' import { useQueryClient } from './useQueryClient.js' import { createRawRef } from './containers.svelte.js' @@ -71,10 +72,22 @@ export function createBaseQuery< createResult(), ) + // Separate trigger so the throw-effect below can react to result updates + // without reading `query.isError`/`isFetching` there, which would mark them + // as tracked on the `trackResult` proxy and permanently widen + // `notifyOnChangeProps` for every consumer of this query. This still + // notifies reliably once `throwOnError` is set, because `QueryObserver` + // force-adds `'error'` to the notified props in that case (see + // `queryObserver.ts`), regardless of what any consumer has read. + let resultVersion = $state(0) + $effect(() => { const unsubscribe = isRestoring.current ? () => undefined - : observer.subscribe(() => update(createResult())) + : observer.subscribe(() => { + update(createResult()) + resultVersion++ + }) observer.updateResult() return unsubscribe }) @@ -100,8 +113,28 @@ export function createBaseQuery< // // this could technically be its own effect but that doesn't seem necessary update(createResult()) + resultVersion++ }, ) + $effect(() => { + // Must throw from inside this reaction, not from the `subscribe` callback + // above (which runs outside any active Svelte reaction) — otherwise + // `` never sees the error. + void resultVersion + const currentResult = observer.getCurrentResult() + + if ( + currentResult.isError && + !currentResult.isFetching && + shouldThrowError(resolvedOptions.throwOnError, [ + currentResult.error, + observer.getCurrentQuery(), + ]) + ) { + throw currentResult.error + } + }) + return query } diff --git a/packages/svelte-query/tests/createInfiniteQuery/ErrorBoundary.svelte b/packages/svelte-query/tests/createInfiniteQuery/ErrorBoundary.svelte new file mode 100644 index 00000000000..aa708d087a2 --- /dev/null +++ b/packages/svelte-query/tests/createInfiniteQuery/ErrorBoundary.svelte @@ -0,0 +1,28 @@ + + + {}}> + + {#snippet failed(error, _reset)} +
+ {error instanceof Error ? error.message : String(error)} +
+ {/snippet} + diff --git a/packages/svelte-query/tests/createInfiniteQuery/ErrorBoundaryContent.svelte b/packages/svelte-query/tests/createInfiniteQuery/ErrorBoundaryContent.svelte new file mode 100644 index 00000000000..aabb11b558e --- /dev/null +++ b/packages/svelte-query/tests/createInfiniteQuery/ErrorBoundaryContent.svelte @@ -0,0 +1,14 @@ + + +
{query.status}
diff --git a/packages/svelte-query/tests/createInfiniteQuery/createInfiniteQuery.svelte.test.ts b/packages/svelte-query/tests/createInfiniteQuery/createInfiniteQuery.svelte.test.ts index 4ecf75b351c..18a09ed2f08 100644 --- a/packages/svelte-query/tests/createInfiniteQuery/createInfiniteQuery.svelte.test.ts +++ b/packages/svelte-query/tests/createInfiniteQuery/createInfiniteQuery.svelte.test.ts @@ -1,10 +1,12 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { fireEvent, render } from '@testing-library/svelte' import { QueryClient } from '@tanstack/query-core' +import { queryKey } from '@tanstack/query-test-utils' import { ref } from '../utils.svelte.js' import Base from './Base.svelte' import Select from './Select.svelte' import ChangeClient from './ChangeClient.svelte' +import ErrorBoundary from './ErrorBoundary.svelte' import InitialData from './InitialData.svelte' import type { QueryObserverResult } from '@tanstack/query-core' @@ -171,4 +173,108 @@ describe('createInfiniteQuery', () => { rendered.getByText('Data: {"pages":[7,8],"pageParams":[7,8]}'), ).toBeInTheDocument() }) + + it('should throw error to the nearest svelte:boundary when throwOnError is true', async () => { + const key = queryKey() + const consoleMock = vi + .spyOn(console, 'error') + .mockImplementation(() => undefined) + + const rendered = render(ErrorBoundary, { + props: { + queryClient, + options: () => ({ + queryKey: key, + queryFn: () => Promise.reject(new Error('Error test')), + getNextPageParam: () => undefined, + initialPageParam: 0, + retry: false, + throwOnError: true, + }), + }, + }) + + await vi.advanceTimersByTimeAsync(0) + expect(rendered.getByTestId('error-boundary')).toHaveTextContent( + 'Error test', + ) + + consoleMock.mockRestore() + }) + + it('should throw error to the nearest svelte:boundary when throwOnError function returns true', async () => { + const key = queryKey() + const consoleMock = vi + .spyOn(console, 'error') + .mockImplementation(() => undefined) + + const rendered = render(ErrorBoundary, { + props: { + queryClient, + options: () => ({ + queryKey: key, + queryFn: () => Promise.reject(new Error('Local Error')), + getNextPageParam: () => undefined, + initialPageParam: 0, + retry: false, + throwOnError: (err: Error) => err.message === 'Local Error', + }), + }, + }) + + await vi.advanceTimersByTimeAsync(0) + expect(rendered.getByTestId('error-boundary')).toHaveTextContent( + 'Local Error', + ) + + consoleMock.mockRestore() + }) + + it('should not throw to the nearest svelte:boundary when throwOnError function returns false', async () => { + const key = queryKey() + + const rendered = render(ErrorBoundary, { + props: { + queryClient, + options: () => ({ + queryKey: key, + queryFn: () => Promise.reject(new Error('Local Error')), + getNextPageParam: () => undefined, + initialPageParam: 0, + retry: false, + throwOnError: (err: Error) => err.message !== 'Local Error', + }), + }, + }) + + await vi.advanceTimersByTimeAsync(0) + expect(rendered.queryByTestId('error-boundary')).not.toBeInTheDocument() + expect(rendered.getByTestId('status')).toHaveTextContent('error') + }) + + it('should throw error to the nearest svelte:boundary when queryFn rejects with a falsy error and throwOnError is in use', async () => { + const key = queryKey() + const consoleMock = vi + .spyOn(console, 'error') + .mockImplementation(() => undefined) + + const rendered = render(ErrorBoundary, { + props: { + queryClient, + options: () => ({ + queryKey: key, + queryFn: () => Promise.reject(), + getNextPageParam: () => undefined, + initialPageParam: 0, + retry: false, + throwOnError: true, + }), + }, + }) + + await vi.advanceTimersByTimeAsync(0) + expect(rendered.getByTestId('error-boundary')).toBeInTheDocument() + + consoleMock.mockRestore() + }) }) diff --git a/packages/svelte-query/tests/createQuery/ErrorBoundary.svelte b/packages/svelte-query/tests/createQuery/ErrorBoundary.svelte new file mode 100644 index 00000000000..ddc610d5d2a --- /dev/null +++ b/packages/svelte-query/tests/createQuery/ErrorBoundary.svelte @@ -0,0 +1,28 @@ + + + {}}> + + {#snippet failed(error, _reset)} +
+ {error instanceof Error ? error.message : String(error)} +
+ {/snippet} + diff --git a/packages/svelte-query/tests/createQuery/ErrorBoundaryChangeClient.svelte b/packages/svelte-query/tests/createQuery/ErrorBoundaryChangeClient.svelte new file mode 100644 index 00000000000..18f53abbdab --- /dev/null +++ b/packages/svelte-query/tests/createQuery/ErrorBoundaryChangeClient.svelte @@ -0,0 +1,29 @@ + + + {}}> + + {#snippet failed(error, _reset)} +
+ {error instanceof Error ? error.message : String(error)} +
+ {/snippet} + diff --git a/packages/svelte-query/tests/createQuery/ErrorBoundaryChangeClientContent.svelte b/packages/svelte-query/tests/createQuery/ErrorBoundaryChangeClientContent.svelte new file mode 100644 index 00000000000..eb1f3314795 --- /dev/null +++ b/packages/svelte-query/tests/createQuery/ErrorBoundaryChangeClientContent.svelte @@ -0,0 +1,16 @@ + + +
{query.status}
diff --git a/packages/svelte-query/tests/createQuery/ErrorBoundaryContent.svelte b/packages/svelte-query/tests/createQuery/ErrorBoundaryContent.svelte new file mode 100644 index 00000000000..83096385992 --- /dev/null +++ b/packages/svelte-query/tests/createQuery/ErrorBoundaryContent.svelte @@ -0,0 +1,14 @@ + + +
{query.status}
diff --git a/packages/svelte-query/tests/createQuery/createQuery.svelte.test.ts b/packages/svelte-query/tests/createQuery/createQuery.svelte.test.ts index 7c6983a4b12..089fc487b34 100644 --- a/packages/svelte-query/tests/createQuery/createQuery.svelte.test.ts +++ b/packages/svelte-query/tests/createQuery/createQuery.svelte.test.ts @@ -18,6 +18,8 @@ import { } from '../../src/index.js' import { promiseWithResolvers, withEffectRoot } from '../utils.svelte.js' import Base from './Base.svelte' +import ErrorBoundary from './ErrorBoundary.svelte' +import ErrorBoundaryChangeClient from './ErrorBoundaryChangeClient.svelte' import Counter from './Counter.svelte' import IsRestoring from './IsRestoring.svelte' import Select from './Select.svelte' @@ -1243,6 +1245,41 @@ describe('createQuery', () => { }), ) + it( + 'should not widen tracked props for unrelated data-only consumers after an error occurs', + withEffectRoot(async () => { + const key = queryKey() + const dataOnlyRuns: Array = [] + + const query = createQuery( + () => ({ + queryKey: key, + queryFn: () => Promise.reject(new Error('fail')), + retry: false, + // `false` never satisfies `shouldThrowError`, so the query settles + // into an error state without throwing — this is the case where + // the throw-effect's `!query.isFetching` check (guarded behind + // `query.isError`) reads `isFetching` and, unless read from the + // untracked result, would mark it tracked from then on. + throwOnError: false, + }), + () => queryClient, + ) + + // This effect only ever reads `data`. Once the query above has settled + // into an error state, `isFetching` transitions on a later refetch must + // not cause this unrelated, data-only effect to re-run. + $effect(() => { + dataOnlyRuns.push(query.data) + }) + + await vi.advanceTimersByTimeAsync(0) + await query.refetch() + + expect(dataOnlyRuns).toHaveLength(1) + }), + ) + it( 'should always re-render if we are tracking props but not using any', withEffectRoot(async () => { @@ -1593,6 +1630,149 @@ describe('createQuery', () => { expect(rendered.getByTestId('error')).toHaveTextContent('Local Error') }) + it('should throw error to the nearest svelte:boundary when throwOnError is true', async () => { + const key = queryKey() + const consoleMock = vi + .spyOn(console, 'error') + .mockImplementation(() => undefined) + + const rendered = render(ErrorBoundary, { + props: { + queryClient, + options: () => ({ + queryKey: key, + queryFn: () => Promise.reject(new Error('Error test')), + retry: false, + throwOnError: true, + }), + }, + }) + + await vi.advanceTimersByTimeAsync(0) + expect(rendered.getByTestId('error-boundary')).toHaveTextContent( + 'Error test', + ) + + consoleMock.mockRestore() + }) + + it('should throw error to the nearest svelte:boundary when throwOnError function returns true', async () => { + const key = queryKey() + const consoleMock = vi + .spyOn(console, 'error') + .mockImplementation(() => undefined) + + const rendered = render(ErrorBoundary, { + props: { + queryClient, + options: () => ({ + queryKey: key, + queryFn: () => Promise.reject(new Error('Local Error')), + retry: false, + throwOnError: (err: Error) => err.message === 'Local Error', + }), + }, + }) + + await vi.advanceTimersByTimeAsync(0) + expect(rendered.getByTestId('error-boundary')).toHaveTextContent( + 'Local Error', + ) + + consoleMock.mockRestore() + }) + + it('should throw error to the nearest svelte:boundary when queryFn rejects with a falsy error and throwOnError is in use', async () => { + const key = queryKey() + const consoleMock = vi + .spyOn(console, 'error') + .mockImplementation(() => undefined) + + const rendered = render(ErrorBoundary, { + props: { + queryClient, + options: () => ({ + queryKey: key, + queryFn: () => Promise.reject(), + retry: false, + throwOnError: true, + }), + }, + }) + + await vi.advanceTimersByTimeAsync(0) + expect(rendered.getByTestId('error-boundary')).toBeInTheDocument() + + consoleMock.mockRestore() + }) + + it('should throw a cached error to the nearest svelte:boundary without refetching', async () => { + const key = queryKey() + const consoleMock = vi + .spyOn(console, 'error') + .mockImplementation(() => undefined) + + // Pre-populate the cache with an error result via a first, unmounted subscriber. + const first = render(Base, { + props: { + queryClient, + options: () => ({ + queryKey: key, + queryFn: () => Promise.reject(new Error('Pre-existing error')), + retry: false, + throwOnError: false, + }), + }, + }) + await vi.advanceTimersByTimeAsync(0) + first.unmount() + + // Now mount a NEW component subscribing to the same key with throwOnError: true. + // `enabled: false` guarantees no new fetch happens on mount, so the only way + // this passes is if the throw-effect fires off the PRE-EXISTING cached error. + const queryFn = vi.fn(() => Promise.reject(new Error('should not fetch'))) + const rendered = render(ErrorBoundary, { + props: { + queryClient, + options: () => ({ + queryKey: key, + queryFn, + retry: false, + enabled: false, + throwOnError: true, + }), + }, + }) + + await vi.advanceTimersByTimeAsync(0) + expect(rendered.getByTestId('error-boundary')).toHaveTextContent( + 'Pre-existing error', + ) + expect(queryFn).not.toHaveBeenCalled() + + consoleMock.mockRestore() + }) + + it( + 'should update with data if we observe no properties and throwOnError', + withEffectRoot(async () => { + const key = queryKey() + + const query = createQuery( + () => ({ + queryKey: key, + queryFn: () => Promise.resolve('data'), + throwOnError: true, + }), + () => queryClient, + ) + + await vi.advanceTimersByTimeAsync(0) + expect(queryClient.isFetching()).toBe(0) + expect(query.data).toBe('data') + }), + ) + it( 'should support changing provided query client', withEffectRoot(() => { @@ -1624,6 +1804,61 @@ describe('createQuery', () => { }), ) + it('should throw a cached error to the nearest svelte:boundary when the query client changes', async () => { + const key = queryKey() + const consoleMock = vi + .spyOn(console, 'error') + .mockImplementation(() => undefined) + + const queryClient1 = new QueryClient() + const queryClient2 = new QueryClient() + + // Pre-populate queryClient2's cache with an error result via a first, + // unmounted subscriber, so switching to it never needs to fetch. + const first = render(Base, { + props: { + queryClient: queryClient2, + options: () => ({ + queryKey: key, + queryFn: () => Promise.reject(new Error('Pre-existing error')), + retry: false, + throwOnError: false, + }), + }, + }) + await vi.advanceTimersByTimeAsync(0) + first.unmount() + + let currentClient = $state(queryClient1) + const queryFn = vi.fn(() => Promise.reject(new Error('should not fetch'))) + + const rendered = render(ErrorBoundaryChangeClient, { + props: { + queryClient: queryClient1, + currentClient: () => currentClient, + options: () => ({ + queryKey: key, + queryFn, + retry: false, + enabled: false, + throwOnError: true, + }), + }, + }) + await vi.advanceTimersByTimeAsync(0) + expect(rendered.queryByTestId('error-boundary')).not.toBeInTheDocument() + + currentClient = queryClient2 + await vi.advanceTimersByTimeAsync(0) + + expect(rendered.getByTestId('error-boundary')).toHaveTextContent( + 'Pre-existing error', + ) + expect(queryFn).not.toHaveBeenCalled() + + consoleMock.mockRestore() + }) + it('should not fetch for the duration of the restoring period when isRestoring is true', async () => { const queryFn = vi.fn(() => sleep(10).then(() => 'data'))