fix(error-boundary): avoid clearing reset state without a pending reset

.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

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
+    },
   }
 }
 

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, <TestComponent />)
+
+      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')

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])
 }
 

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
+    },
   }
 }
 

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, <TestComponent />)
+
+      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')

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])
 }