From e23f009c5b3de2d1fc1da8c457bac1f258800619 Mon Sep 17 00:00:00 2001 From: smelt <202003010+wspperrimh@users.noreply.github.com> Date: Sun, 4 Oct 2026 14:12:09 -0700 Subject: [PATCH] fix(react-query,preact-query): retry errored queries that mount after the error boundary reset wave Queries mounting while the reset flag was pending each cleared the shared flag in their mount effect, so the first mounted query consumed the reset before other errored queries could observe it. Track a reset generation and which generation each query last cleared, so every errored query inside the boundary retries once after a reset even when it mounts after siblings already consumed the shared wave. Fixes #2712 --- .changeset/eager-dingos-retry.md | 6 ++ .../src/QueryErrorResetBoundary.tsx | 44 +++++++-- .../QueryResetErrorBoundary.test.tsx | 91 ++++++++++++++++++- .../preact-query/src/errorBoundaryUtils.ts | 11 ++- packages/preact-query/src/suspense.ts | 2 +- packages/preact-query/src/useBaseQuery.ts | 2 +- packages/preact-query/src/useQueries.ts | 5 +- .../src/QueryErrorResetBoundary.tsx | 54 +++++++---- .../QueryResetErrorBoundary.test.tsx | 89 ++++++++++++++++++ .../react-query/src/errorBoundaryUtils.ts | 11 ++- packages/react-query/src/suspense.ts | 2 +- packages/react-query/src/useBaseQuery.ts | 2 +- packages/react-query/src/useQueries.ts | 5 +- 13 files changed, 281 insertions(+), 43 deletions(-) create mode 100644 .changeset/eager-dingos-retry.md 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( () =>