diff --git a/.changeset/fix-query-error-reset-boundary.md b/.changeset/fix-query-error-reset-boundary.md new file mode 100644 index 00000000000..db15a74b492 --- /dev/null +++ b/.changeset/fix-query-error-reset-boundary.md @@ -0,0 +1,6 @@ +--- +'@tanstack/react-query': patch +'@tanstack/preact-query': patch +--- + +fix(error-boundary): only clear a reset boundary after it has actually been reset diff --git a/packages/preact-query/src/QueryErrorResetBoundary.tsx b/packages/preact-query/src/QueryErrorResetBoundary.tsx index 9f5e1045a9a..aca4d46c07c 100644 --- a/packages/preact-query/src/QueryErrorResetBoundary.tsx +++ b/packages/preact-query/src/QueryErrorResetBoundary.tsx @@ -43,18 +43,36 @@ export interface QueryErrorResetBoundaryValue { * requested. * @returns The `clearReset`, `isReset`, and `reset` functions of the boundary. */ -function createValue(): QueryErrorResetBoundaryValue { +function createValue(): QueryErrorResetBoundaryValue & { + _resetCount: () => number + _lastConsumedResetCount: () => number +} { let isReset = false + let resetCount = 0 + let lastConsumedResetCount = 0 + return { clearReset: () => { + if (lastConsumedResetCount === resetCount) { + return + } + isReset = false + lastConsumedResetCount = resetCount }, reset: () => { isReset = true + resetCount++ }, isReset: () => { return isReset }, + _resetCount: () => { + return resetCount + }, + _lastConsumedResetCount: () => { + return lastConsumedResetCount + }, } } diff --git a/packages/preact-query/src/__tests__/QueryResetErrorBoundary.test.tsx b/packages/preact-query/src/__tests__/QueryResetErrorBoundary.test.tsx index 43c1e9df144..7c578b7123f 100644 --- a/packages/preact-query/src/__tests__/QueryResetErrorBoundary.test.tsx +++ b/packages/preact-query/src/__tests__/QueryResetErrorBoundary.test.tsx @@ -13,6 +13,7 @@ import { useSuspenseQueries, useSuspenseQuery, } from '..' +import { useClearResetErrorBoundary } from '../errorBoundaryUtils' import { ErrorBoundary } from './ErrorBoundary' import { renderWithClient } from './utils' @@ -32,6 +33,96 @@ describe('QueryErrorResetBoundary', () => { }) describe('useQuery', () => { + it('should only clear the reset boundary after a reset has happened', () => { + const clearReset = vi.fn() + + function TestComponent() { + useClearResetErrorBoundary({ + clearReset, + isReset: () => false, + reset: vi.fn(), + }) + + return null + } + + renderWithClient(queryClient, ) + + expect(clearReset).not.toHaveBeenCalled() + }) + + it('should only consume a reset once per generation', () => { + let isReset = false + let resetCount = 0 + let lastConsumedResetCount = 0 + const boundary = { + clearReset: vi.fn(() => { + if (lastConsumedResetCount === resetCount) { + return + } + + isReset = false + lastConsumedResetCount = resetCount + }), + isReset: () => isReset, + reset: () => { + isReset = true + resetCount++ + }, + _resetCount: () => resetCount, + _lastConsumedResetCount: () => lastConsumedResetCount, + } + + boundary.reset() + if ( + boundary._lastConsumedResetCount() !== boundary._resetCount() && + boundary.isReset() + ) { + boundary.clearReset() + } + expect(boundary.clearReset).toHaveBeenCalledTimes(1) + + isReset = true + if ( + boundary._lastConsumedResetCount() !== boundary._resetCount() && + boundary.isReset() + ) { + boundary.clearReset() + } + expect(boundary.clearReset).toHaveBeenCalledTimes(1) + }) + + it('should ignore repeated clearReset calls for the same generation', () => { + let isReset = true + let resetCount = 1 + let lastConsumedResetCount = 0 + + const boundary = { + clearReset: vi.fn(() => { + if (lastConsumedResetCount === resetCount) { + return + } + + isReset = false + lastConsumedResetCount = resetCount + }), + isReset: () => isReset, + reset: () => { + isReset = true + resetCount += 1 + }, + _resetCount: () => resetCount, + _lastConsumedResetCount: () => lastConsumedResetCount, + } + + boundary.clearReset() + expect(isReset).toBe(false) + expect(lastConsumedResetCount).toBe(1) + + boundary.clearReset() + expect(lastConsumedResetCount).toBe(1) + }) + it('should retry fetch if the reset error boundary has been reset', async () => { const consoleErrorMock = vi .spyOn(console, 'error') diff --git a/packages/preact-query/src/errorBoundaryUtils.ts b/packages/preact-query/src/errorBoundaryUtils.ts index fecc039f7d3..4cea74fee0e 100644 --- a/packages/preact-query/src/errorBoundaryUtils.ts +++ b/packages/preact-query/src/errorBoundaryUtils.ts @@ -57,7 +57,18 @@ export const useClearResetErrorBoundary = ( errorResetBoundary: QueryErrorResetBoundaryValue, ) => { useEffect(() => { - errorResetBoundary.clearReset() + const resetBoundary = errorResetBoundary as QueryErrorResetBoundaryValue & { + _resetCount?: () => number + _lastConsumedResetCount?: () => number + } + + if ( + resetBoundary._lastConsumedResetCount?.() !== + resetBoundary._resetCount?.() && + errorResetBoundary.isReset() + ) { + errorResetBoundary.clearReset() + } }, [errorResetBoundary]) } diff --git a/packages/react-query/src/QueryErrorResetBoundary.tsx b/packages/react-query/src/QueryErrorResetBoundary.tsx index dfde44fb268..cd2125ef22d 100644 --- a/packages/react-query/src/QueryErrorResetBoundary.tsx +++ b/packages/react-query/src/QueryErrorResetBoundary.tsx @@ -42,20 +42,32 @@ export interface QueryErrorResetBoundaryValue { * requested. * @returns The `clearReset`, `isReset`, and `reset` functions of the boundary. */ -function createValue(): QueryErrorResetBoundaryValue { +function createValue(): QueryErrorResetBoundaryValue & { + _resetCount: () => number + _lastConsumedResetCount: () => number +} { let isReset = false + let resetCount = 0 + let lastConsumedResetCount = 0 + return { /** * Clears the reset state, so queries know not to try again until the boundary is reset again. */ clearReset: () => { + if (lastConsumedResetCount === resetCount) { + return + } + isReset = false + lastConsumedResetCount = resetCount }, /** * Resets any query errors within the boundary, so queries know they can try again. */ reset: () => { isReset = true + resetCount++ }, /** * Returns whether the boundary has been reset and not yet cleared. @@ -64,6 +76,19 @@ function createValue(): QueryErrorResetBoundaryValue { isReset: () => { return isReset }, + /** + * Tracks the current reset generation so a remount can consume only the + * most recent pending reset. + */ + _resetCount: () => { + return resetCount + }, + /** + * Tracks the latest reset generation that has already been consumed. + */ + _lastConsumedResetCount: () => { + return lastConsumedResetCount + }, } } diff --git a/packages/react-query/src/__tests__/QueryResetErrorBoundary.test.tsx b/packages/react-query/src/__tests__/QueryResetErrorBoundary.test.tsx index 9479cad27b3..7ef8e332e21 100644 --- a/packages/react-query/src/__tests__/QueryResetErrorBoundary.test.tsx +++ b/packages/react-query/src/__tests__/QueryResetErrorBoundary.test.tsx @@ -12,6 +12,7 @@ import { useSuspenseQueries, useSuspenseQuery, } from '..' +import { useClearResetErrorBoundary } from '../errorBoundaryUtils' import { renderWithClient } from './utils' describe('QueryErrorResetBoundary', () => { @@ -30,6 +31,96 @@ describe('QueryErrorResetBoundary', () => { }) describe('useQuery', () => { + it('should only clear the reset boundary after a reset has happened', () => { + const clearReset = vi.fn() + + function TestComponent() { + useClearResetErrorBoundary({ + clearReset, + isReset: () => false, + reset: vi.fn(), + }) + + return null + } + + renderWithClient(queryClient, ) + + expect(clearReset).not.toHaveBeenCalled() + }) + + it('should only consume a reset once per generation', () => { + let isReset = false + let resetCount = 0 + let lastConsumedResetCount = 0 + const boundary = { + clearReset: vi.fn(() => { + if (lastConsumedResetCount === resetCount) { + return + } + + isReset = false + lastConsumedResetCount = resetCount + }), + isReset: () => isReset, + reset: () => { + isReset = true + resetCount++ + }, + _resetCount: () => resetCount, + _lastConsumedResetCount: () => lastConsumedResetCount, + } + + boundary.reset() + if ( + boundary._lastConsumedResetCount() !== boundary._resetCount() && + boundary.isReset() + ) { + boundary.clearReset() + } + expect(boundary.clearReset).toHaveBeenCalledTimes(1) + + isReset = true + if ( + boundary._lastConsumedResetCount() !== boundary._resetCount() && + boundary.isReset() + ) { + boundary.clearReset() + } + expect(boundary.clearReset).toHaveBeenCalledTimes(1) + }) + + it('should ignore repeated clearReset calls for the same generation', () => { + let isReset = true + let resetCount = 1 + let lastConsumedResetCount = 0 + + const boundary = { + clearReset: vi.fn(() => { + if (lastConsumedResetCount === resetCount) { + return + } + + isReset = false + lastConsumedResetCount = resetCount + }), + isReset: () => isReset, + reset: () => { + isReset = true + resetCount += 1 + }, + _resetCount: () => resetCount, + _lastConsumedResetCount: () => lastConsumedResetCount, + } + + boundary.clearReset() + expect(isReset).toBe(false) + expect(lastConsumedResetCount).toBe(1) + + boundary.clearReset() + expect(lastConsumedResetCount).toBe(1) + }) + it('should retry fetch if the reset error boundary has been reset', async () => { const consoleErrorMock = vi .spyOn(console, 'error') diff --git a/packages/react-query/src/errorBoundaryUtils.ts b/packages/react-query/src/errorBoundaryUtils.ts index 2d6824178a4..49b08054039 100644 --- a/packages/react-query/src/errorBoundaryUtils.ts +++ b/packages/react-query/src/errorBoundaryUtils.ts @@ -57,7 +57,18 @@ export const useClearResetErrorBoundary = ( errorResetBoundary: QueryErrorResetBoundaryValue, ) => { React.useEffect(() => { - errorResetBoundary.clearReset() + const resetBoundary = errorResetBoundary as QueryErrorResetBoundaryValue & { + _resetCount?: () => number + _lastConsumedResetCount?: () => number + } + + if ( + resetBoundary._lastConsumedResetCount?.() !== + resetBoundary._resetCount?.() && + errorResetBoundary.isReset() + ) { + errorResetBoundary.clearReset() + } }, [errorResetBoundary]) }