diff --git a/.changeset/eager-dingos-retry.md b/.changeset/eager-dingos-retry.md new file mode 100644 index 00000000000..67f8e9a5f79 --- /dev/null +++ b/.changeset/eager-dingos-retry.md @@ -0,0 +1,6 @@ +--- +'@tanstack/react-query': patch +'@tanstack/preact-query': patch +--- + +fix(react-query, preact-query): retry queries caught by an error boundary when they mount after the reset wave diff --git a/packages/preact-query/src/QueryErrorResetBoundary.tsx b/packages/preact-query/src/QueryErrorResetBoundary.tsx index 10e854edba4..45208f2ec6a 100644 --- a/packages/preact-query/src/QueryErrorResetBoundary.tsx +++ b/packages/preact-query/src/QueryErrorResetBoundary.tsx @@ -10,14 +10,18 @@ import { useContext, useState } from 'preact/hooks' export type QueryErrorResetFunction = () => void /** - * Returns whether the boundary has been reset and not yet cleared. + * Returns whether the boundary has been reset and not yet cleared. When a `queryHash` is + * given, only checks whether the boundary has been reset since that query last cleared its + * own reset state. */ -export type QueryErrorIsResetFunction = () => boolean +export type QueryErrorIsResetFunction = (queryHash?: string) => boolean /** * Clears the reset state, so queries know not to try again until the boundary is reset again. + * When a `queryHash` is given, only that query's view of the reset state is cleared, so + * sibling queries can still observe the reset. */ -export type QueryErrorClearResetFunction = () => void +export type QueryErrorClearResetFunction = (queryHash?: string) => void /** * The value a `QueryErrorResetBoundary` shares through context, used to reset query errors within @@ -44,16 +48,38 @@ export interface QueryErrorResetBoundaryValue { * @returns The `clearReset`, `isReset`, and `reset` functions of the boundary. */ function createValue(): QueryErrorResetBoundaryValue { - let isReset = false + // Each `reset()` starts a new reset generation and every query tracks the + // generation it last cleared, so one mounted query cannot consume the reset + // signal before other queries have seen it. + let resetId = 0 + let resetPending = false + const clearedQueries = new Map() return { - clearReset: () => { - isReset = false + clearReset: (queryHash) => { + if (queryHash === undefined) { + resetId = 0 + resetPending = false + clearedQueries.clear() + } else { + clearedQueries.set(queryHash, resetId) + resetPending = false + } }, reset: () => { - isReset = true + resetId += 1 + resetPending = true }, - isReset: () => { - return isReset + isReset: (queryHash) => { + if (queryHash === undefined) { + return resetPending + } + const clearedResetId = clearedQueries.get(queryHash) + // Queries that never cleared their own reset state follow the shared + // reset wave (cleared by the first mounted query), while queries that + // did clear it keep their own pending state until they observe it. + return clearedResetId === undefined + ? resetPending + : resetId > clearedResetId }, } } diff --git a/packages/preact-query/src/__tests__/QueryResetErrorBoundary.test.tsx b/packages/preact-query/src/__tests__/QueryResetErrorBoundary.test.tsx index 43c1e9df144..39e1172af54 100644 --- a/packages/preact-query/src/__tests__/QueryResetErrorBoundary.test.tsx +++ b/packages/preact-query/src/__tests__/QueryResetErrorBoundary.test.tsx @@ -1,5 +1,5 @@ import { queryKey, sleep } from '@tanstack/query-test-utils' -import { fireEvent } from '@testing-library/preact' +import { act, fireEvent } from '@testing-library/preact' import { Suspense } from 'preact/compat' import { useEffect, useState } from 'preact/hooks' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' @@ -732,6 +732,95 @@ describe('QueryErrorResetBoundary', () => { }) }) + it('should retry an errored query that mounts after a sibling query already consumed the reset', async () => { + const consoleErrorMock = vi + .spyOn(console, 'error') + .mockImplementation(() => undefined) + const key = queryKey() + const siblingKey = queryKey() + + let succeed = false + let showPage!: (show: boolean) => void + + function ErroredPage() { + const { data } = useQuery({ + queryKey: key, + queryFn: () => + sleep(10).then(() => { + if (!succeed) throw new Error('Error') + return 'data' + }), + retry: false, + throwOnError: true, + }) + + return
{data}
+ } + + function Sibling() { + useQuery({ + queryKey: siblingKey, + queryFn: () => sleep(10).then(() => 'sibling'), + }) + + return null + } + + function App() { + const [show, setShow] = useState(true) + showPage = setShow + + return ( + + {({ reset }) => ( + ( +
+
error boundary
+ +
+ )} + > + + {show ? : null} +
+ )} +
+ ) + } + + const rendered = renderWithClient(queryClient, ) + + await vi.advanceTimersByTimeAsync(11) + expect(rendered.getByText('error boundary')).toBeInTheDocument() + + // Reset the boundary while the errored query is not rendered, so the + // sibling's mount is the only query that observes the reset. + act(() => { + showPage(false) + }) + fireEvent.click(rendered.getByText('retry')) + await vi.advanceTimersByTimeAsync(11) + + // Mounting the errored query again must still retry it: the sibling + // must not have consumed the reset on its behalf. + succeed = true + act(() => { + showPage(true) + }) + await vi.advanceTimersByTimeAsync(11) + expect(rendered.getByText('data')).toBeInTheDocument() + + consoleErrorMock.mockRestore() + }) + describe('useQueries', () => { it('should retry fetch if the reset error boundary has been reset', async () => { const consoleErrorMock = vi diff --git a/packages/preact-query/src/errorBoundaryUtils.ts b/packages/preact-query/src/errorBoundaryUtils.ts index ff39632f233..d130de59e64 100644 --- a/packages/preact-query/src/errorBoundaryUtils.ts +++ b/packages/preact-query/src/errorBoundaryUtils.ts @@ -42,7 +42,7 @@ export const ensurePreventErrorBoundaryRetry = < if (options.suspense || throwOnError) { // Prevent retrying failed query if the error boundary has not been reset yet - if (!errorResetBoundary.isReset()) { + if (!errorResetBoundary.isReset(options.queryHash)) { options.retryOnMount = false } } @@ -52,13 +52,16 @@ export const ensurePreventErrorBoundaryRetry = < * Clears the reset state of the error boundary after the component mounts, so later errors are * thrown to the boundary again. * @param errorResetBoundary - The value of the nearest `QueryErrorResetBoundary`. + * @param queryHash - The hash(es) of the queries to clear the reset state for. */ export const useClearResetErrorBoundary = ( errorResetBoundary: QueryErrorResetBoundaryValue, + queryHash: string | Array, ) => { useEffect(() => { - errorResetBoundary.clearReset() - }, [errorResetBoundary]) + const queryHashes = Array.isArray(queryHash) ? queryHash : [queryHash] + queryHashes.forEach((hash) => errorResetBoundary.clearReset(hash)) + }, [errorResetBoundary, queryHash]) } /** @@ -90,7 +93,7 @@ export const getHasError = < }) => { return ( result.isError && - !errorResetBoundary.isReset() && + !errorResetBoundary.isReset(query?.queryHash) && !result.isFetching && query && ((suspense && result.data === undefined) || diff --git a/packages/preact-query/src/suspense.ts b/packages/preact-query/src/suspense.ts index 047f21d41f0..21b81f45b23 100644 --- a/packages/preact-query/src/suspense.ts +++ b/packages/preact-query/src/suspense.ts @@ -98,5 +98,5 @@ export const fetchOptimistic = < errorResetBoundary: QueryErrorResetBoundaryValue, ) => observer.fetchOptimistic(defaultedOptions).catch(() => { - errorResetBoundary.clearReset() + errorResetBoundary.clearReset(defaultedOptions.queryHash) }) diff --git a/packages/preact-query/src/useBaseQuery.ts b/packages/preact-query/src/useBaseQuery.ts index b0ac07196cc..59bca812c66 100644 --- a/packages/preact-query/src/useBaseQuery.ts +++ b/packages/preact-query/src/useBaseQuery.ts @@ -88,7 +88,7 @@ export function useBaseQuery< ensureSuspenseTimers(defaultedOptions) ensurePreventErrorBoundaryRetry(defaultedOptions, errorResetBoundary, query) - useClearResetErrorBoundary(errorResetBoundary) + useClearResetErrorBoundary(errorResetBoundary, defaultedOptions.queryHash) const [observer] = useState( () => diff --git a/packages/preact-query/src/useQueries.ts b/packages/preact-query/src/useQueries.ts index 58e26f3ceba..ac06015d037 100644 --- a/packages/preact-query/src/useQueries.ts +++ b/packages/preact-query/src/useQueries.ts @@ -361,7 +361,10 @@ export function useQueries< ensurePreventErrorBoundaryRetry(queryOptions, errorResetBoundary, query) }) - useClearResetErrorBoundary(errorResetBoundary) + useClearResetErrorBoundary( + errorResetBoundary, + defaultedQueries.map((queryOptions) => queryOptions.queryHash), + ) const [observer] = useState( () => diff --git a/packages/react-query/src/QueryErrorResetBoundary.tsx b/packages/react-query/src/QueryErrorResetBoundary.tsx index dfde44fb268..0f506119fa6 100644 --- a/packages/react-query/src/QueryErrorResetBoundary.tsx +++ b/packages/react-query/src/QueryErrorResetBoundary.tsx @@ -9,14 +9,18 @@ import * as React from 'react' export type QueryErrorResetFunction = () => void /** - * Returns whether the boundary has been reset and not yet cleared. + * Returns whether the boundary has been reset and not yet cleared. When a `queryHash` is + * given, only checks whether the boundary has been reset since that query last cleared its + * own reset state. */ -export type QueryErrorIsResetFunction = () => boolean +export type QueryErrorIsResetFunction = (queryHash?: string) => boolean /** * Clears the reset state, so queries know not to try again until the boundary is reset again. + * When a `queryHash` is given, only that query's view of the reset state is cleared, so + * sibling queries can still observe the reset. */ -export type QueryErrorClearResetFunction = () => void +export type QueryErrorClearResetFunction = (queryHash?: string) => void /** * The value a `QueryErrorResetBoundary` shares through context, used to reset query errors within @@ -43,26 +47,38 @@ export interface QueryErrorResetBoundaryValue { * @returns The `clearReset`, `isReset`, and `reset` functions of the boundary. */ function createValue(): QueryErrorResetBoundaryValue { - let isReset = false + // Each `reset()` starts a new reset generation and every query tracks the + // generation it last cleared, so one mounted query cannot consume the reset + // signal before other queries have seen it. + let resetId = 0 + let resetPending = false + const clearedQueries = new Map() return { - /** - * Clears the reset state, so queries know not to try again until the boundary is reset again. - */ - clearReset: () => { - isReset = false + clearReset: (queryHash) => { + if (queryHash === undefined) { + resetId = 0 + resetPending = false + clearedQueries.clear() + } else { + clearedQueries.set(queryHash, resetId) + resetPending = false + } }, - /** - * Resets any query errors within the boundary, so queries know they can try again. - */ reset: () => { - isReset = true + resetId += 1 + resetPending = true }, - /** - * Returns whether the boundary has been reset and not yet cleared. - * @returns `true` if the boundary has been reset and not yet cleared. - */ - isReset: () => { - return isReset + isReset: (queryHash) => { + if (queryHash === undefined) { + return resetPending + } + const clearedResetId = clearedQueries.get(queryHash) + // Queries that never cleared their own reset state follow the shared + // reset wave (cleared by the first mounted query), while queries that + // did clear it keep their own pending state until they observe it. + return clearedResetId === undefined + ? resetPending + : resetId > clearedResetId }, } } diff --git a/packages/react-query/src/__tests__/QueryResetErrorBoundary.test.tsx b/packages/react-query/src/__tests__/QueryResetErrorBoundary.test.tsx index 9479cad27b3..979368ddfe9 100644 --- a/packages/react-query/src/__tests__/QueryResetErrorBoundary.test.tsx +++ b/packages/react-query/src/__tests__/QueryResetErrorBoundary.test.tsx @@ -808,6 +808,95 @@ describe('QueryErrorResetBoundary', () => { }) }) + it('should retry an errored query that mounts after a sibling query already consumed the reset', async () => { + const consoleErrorMock = vi + .spyOn(console, 'error') + .mockImplementation(() => undefined) + const key = queryKey() + const siblingKey = queryKey() + + let succeed = false + let showPage!: (show: boolean) => void + + function ErroredPage() { + const { data } = useQuery({ + queryKey: key, + queryFn: () => + sleep(10).then(() => { + if (!succeed) throw new Error('Error') + return 'data' + }), + retry: false, + throwOnError: true, + }) + + return
{data}
+ } + + function Sibling() { + useQuery({ + queryKey: siblingKey, + queryFn: () => sleep(10).then(() => 'sibling'), + }) + + return null + } + + function App() { + const [show, setShow] = React.useState(true) + showPage = setShow + + return ( + + {({ reset }) => ( + ( +
+
error boundary
+ +
+ )} + > + + {show ? : null} +
+ )} +
+ ) + } + + const rendered = renderWithClient(queryClient, ) + + await vi.advanceTimersByTimeAsync(11) + expect(rendered.getByText('error boundary')).toBeInTheDocument() + + // Reset the boundary while the errored query is not rendered, so the + // sibling's mount is the only query that observes the reset. + act(() => { + showPage(false) + }) + fireEvent.click(rendered.getByText('retry')) + await vi.advanceTimersByTimeAsync(11) + + // Mounting the errored query again must still retry it: the sibling + // must not have consumed the reset on its behalf. + succeed = true + act(() => { + showPage(true) + }) + await vi.advanceTimersByTimeAsync(11) + expect(rendered.getByText('data')).toBeInTheDocument() + + consoleErrorMock.mockRestore() + }) + describe('useQueries', () => { it('should retry fetch if the reset error boundary has been reset', async () => { const consoleErrorMock = vi diff --git a/packages/react-query/src/errorBoundaryUtils.ts b/packages/react-query/src/errorBoundaryUtils.ts index 2d6824178a4..ede16a326a7 100644 --- a/packages/react-query/src/errorBoundaryUtils.ts +++ b/packages/react-query/src/errorBoundaryUtils.ts @@ -42,7 +42,7 @@ export const ensurePreventErrorBoundaryRetry = < if (options.suspense || throwOnError) { // Prevent retrying failed query if the error boundary has not been reset yet - if (!errorResetBoundary.isReset()) { + if (!errorResetBoundary.isReset(options.queryHash)) { options.retryOnMount = false } } @@ -52,13 +52,16 @@ export const ensurePreventErrorBoundaryRetry = < * Clears the reset state of the error boundary after the component mounts, so later errors are * thrown to the boundary again. * @param errorResetBoundary - The value of the nearest `QueryErrorResetBoundary`. + * @param queryHash - The hash(es) of the queries to clear the reset state for. */ export const useClearResetErrorBoundary = ( errorResetBoundary: QueryErrorResetBoundaryValue, + queryHash: string | Array, ) => { React.useEffect(() => { - errorResetBoundary.clearReset() - }, [errorResetBoundary]) + const queryHashes = Array.isArray(queryHash) ? queryHash : [queryHash] + queryHashes.forEach((hash) => errorResetBoundary.clearReset(hash)) + }, [errorResetBoundary, queryHash]) } /** @@ -90,7 +93,7 @@ export const getHasError = < }) => { return ( result.isError && - !errorResetBoundary.isReset() && + !errorResetBoundary.isReset(query?.queryHash) && !result.isFetching && query && ((suspense && result.data === undefined) || diff --git a/packages/react-query/src/suspense.ts b/packages/react-query/src/suspense.ts index 6d0534bef8b..e35dcb4f764 100644 --- a/packages/react-query/src/suspense.ts +++ b/packages/react-query/src/suspense.ts @@ -97,5 +97,5 @@ export const fetchOptimistic = < errorResetBoundary: QueryErrorResetBoundaryValue, ) => observer.fetchOptimistic(defaultedOptions).catch(() => { - errorResetBoundary.clearReset() + errorResetBoundary.clearReset(defaultedOptions.queryHash) }) diff --git a/packages/react-query/src/useBaseQuery.ts b/packages/react-query/src/useBaseQuery.ts index ce1a615aff7..ebd62c0e3f0 100644 --- a/packages/react-query/src/useBaseQuery.ts +++ b/packages/react-query/src/useBaseQuery.ts @@ -91,7 +91,7 @@ export function useBaseQuery< ensureSuspenseTimers(defaultedOptions) ensurePreventErrorBoundaryRetry(defaultedOptions, errorResetBoundary, query) - useClearResetErrorBoundary(errorResetBoundary) + useClearResetErrorBoundary(errorResetBoundary, defaultedOptions.queryHash) const [observer] = React.useState( () => diff --git a/packages/react-query/src/useQueries.ts b/packages/react-query/src/useQueries.ts index 6d5886698c2..849c6a77f6d 100644 --- a/packages/react-query/src/useQueries.ts +++ b/packages/react-query/src/useQueries.ts @@ -413,7 +413,10 @@ export function useQueries< ensurePreventErrorBoundaryRetry(queryOptions, errorResetBoundary, query) }) - useClearResetErrorBoundary(errorResetBoundary) + useClearResetErrorBoundary( + errorResetBoundary, + defaultedQueries.map((queryOptions) => queryOptions.queryHash), + ) const [observer] = React.useState( () =>