From 5edd617eac7d27ef4850b101f31281d6079c9de2 Mon Sep 17 00:00:00 2001
From: Dominik Dorfmeister <office@dorfmeister.cc>
Date: Fri, 11 Oct 2024 20:21:58 +0200
Subject: [PATCH] fix(core): correctly gc query when suspense is used when
 query unmounts while it's still fetching (#8168)

* fix(core): correctly gc query when suspense is used when query unmounts while it's still fetching

the `isFetchingOptimistic` workaround was added to make sure some "thing" I don't remember anymore works in suspense; we added tests for that;

however, in the meantime, we added a minimum gcTime for suspense queries, which also solves that problem (tests still work); this makes the `isFetchingOptimistic` workaround unnecessary, and this workaround is actually what causes the unmount issue

* chore: try to stabilize a flaky test
---
 packages/query-core/src/query.ts              | 15 +---
 packages/query-core/src/queryObserver.ts      |  1 -
 .../src/__tests__/suspense.test.tsx           | 76 ++++++++++++++++++-
 .../src/__tests__/useMutationState.test.tsx   | 24 +++---
 4 files changed, 90 insertions(+), 26 deletions(-)

diff --git a/packages/query-core/src/query.ts b/packages/query-core/src/query.ts
index 9850c75160..3485d8ce28 100644
--- a/packages/query-core/src/query.ts
+++ b/packages/query-core/src/query.ts
@@ -163,7 +163,6 @@ export class Query<
   queryHash: string
   options!: QueryOptions<TQueryFnData, TError, TData, TQueryKey>
   state: QueryState<TData, TError>
-  isFetchingOptimistic?: boolean
 
   #initialState: QueryState<TData, TError>
   #revertState?: QueryState<TData, TError>
@@ -482,11 +481,8 @@ export class Query<
         )
       }
 
-      if (!this.isFetchingOptimistic) {
-        // Schedule query gc after fetching
-        this.scheduleGc()
-      }
-      this.isFetchingOptimistic = false
+      // Schedule query gc after fetching
+      this.scheduleGc()
     }
 
     // Try to fetch the data
@@ -522,11 +518,8 @@ export class Query<
           this as Query<any, any, any, any>,
         )
 
-        if (!this.isFetchingOptimistic) {
-          // Schedule query gc after fetching
-          this.scheduleGc()
-        }
-        this.isFetchingOptimistic = false
+        // Schedule query gc after fetching
+        this.scheduleGc()
       },
       onError,
       onFail: (failureCount, error) => {
diff --git a/packages/query-core/src/queryObserver.ts b/packages/query-core/src/queryObserver.ts
index 37442ce42f..5a4db60269 100644
--- a/packages/query-core/src/queryObserver.ts
+++ b/packages/query-core/src/queryObserver.ts
@@ -320,7 +320,6 @@ export class QueryObserver<
     const query = this.#client
       .getQueryCache()
       .build(this.#client, defaultedOptions)
-    query.isFetchingOptimistic = true
 
     return query.fetch().then(() => this.createResult(query, defaultedOptions))
   }
diff --git a/packages/react-query/src/__tests__/suspense.test.tsx b/packages/react-query/src/__tests__/suspense.test.tsx
index 13d4eadd8c..8607b0bde2 100644
--- a/packages/react-query/src/__tests__/suspense.test.tsx
+++ b/packages/react-query/src/__tests__/suspense.test.tsx
@@ -1,5 +1,5 @@
-import { describe, expect, it, vi } from 'vitest'
-import { fireEvent, waitFor } from '@testing-library/react'
+import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'
+import { act, fireEvent, waitFor } from '@testing-library/react'
 import * as React from 'react'
 import { ErrorBoundary } from 'react-error-boundary'
 import {
@@ -1235,4 +1235,76 @@ describe('useSuspenseQueries', () => {
     expect(count).toBe(1)
     consoleMock.mockRestore()
   })
+
+  describe('gc (with fake timers)', () => {
+    beforeAll(() => {
+      vi.useFakeTimers()
+    })
+
+    afterAll(() => {
+      vi.useRealTimers()
+    })
+
+    it('should gc when unmounted while fetching with low gcTime (#8159)', async () => {
+      const key = queryKey()
+
+      function Page() {
+        return (
+          <React.Suspense fallback="loading">
+            <Component />
+          </React.Suspense>
+        )
+      }
+
+      function Component() {
+        const { data } = useSuspenseQuery({
+          queryKey: key,
+          queryFn: async () => {
+            await sleep(3000)
+            return 'data'
+          },
+          gcTime: 1000,
+        })
+
+        return <div>{data}</div>
+      }
+
+      function Page2() {
+        return <div>page2</div>
+      }
+
+      function App() {
+        const [show, setShow] = React.useState(true)
+        return (
+          <div>
+            {show ? <Page /> : <Page2 />}
+            <button onClick={() => setShow(false)}>hide</button>
+          </div>
+        )
+      }
+
+      const rendered = renderWithClient(queryClient, <App />)
+
+      await act(() => vi.advanceTimersByTimeAsync(200))
+
+      rendered.getByText('loading')
+
+      // unmount while still fetching
+      fireEvent.click(rendered.getByText('hide'))
+
+      await act(() => vi.advanceTimersByTimeAsync(800))
+
+      rendered.getByText('page2')
+
+      // wait for query to be resolved
+      await act(() => vi.advanceTimersByTimeAsync(2000))
+
+      expect(queryClient.getQueryData(key)).toBe('data')
+
+      // wait for gc
+      await act(() => vi.advanceTimersByTimeAsync(1000))
+
+      expect(queryClient.getQueryData(key)).toBe(undefined)
+    })
+  })
 })
diff --git a/packages/react-query/src/__tests__/useMutationState.test.tsx b/packages/react-query/src/__tests__/useMutationState.test.tsx
index 24a50de691..32fb793cc0 100644
--- a/packages/react-query/src/__tests__/useMutationState.test.tsx
+++ b/packages/react-query/src/__tests__/useMutationState.test.tsx
@@ -7,7 +7,6 @@ import {
   createQueryClient,
   doNotExecute,
   renderWithClient,
-  setActTimeout,
   sleep,
 } from './utils'
 import type { MutationState, MutationStatus } from '@tanstack/query-core'
@@ -27,26 +26,24 @@ describe('useIsMutating', () => {
       const { mutate: mutate1 } = useMutation({
         mutationKey: ['mutation1'],
         mutationFn: async () => {
-          await sleep(150)
+          await sleep(50)
           return 'data'
         },
       })
       const { mutate: mutate2 } = useMutation({
         mutationKey: ['mutation2'],
         mutationFn: async () => {
-          await sleep(50)
+          await sleep(10)
           return 'data'
         },
       })
 
-      React.useEffect(() => {
-        mutate1()
-        setActTimeout(() => {
-          mutate2()
-        }, 50)
-      }, [mutate1, mutate2])
-
-      return null
+      return (
+        <div>
+          <button onClick={() => mutate1()}>mutate1</button>
+          <button onClick={() => mutate2()}>mutate2</button>
+        </div>
+      )
     }
 
     function Page() {
@@ -58,7 +55,10 @@ describe('useIsMutating', () => {
       )
     }
 
-    renderWithClient(queryClient, <Page />)
+    const rendered = renderWithClient(queryClient, <Page />)
+    fireEvent.click(rendered.getByRole('button', { name: /mutate1/i }))
+    await sleep(10)
+    fireEvent.click(rendered.getByRole('button', { name: /mutate2/i }))
     await waitFor(() => expect(isMutatingArray).toEqual([0, 1, 2, 1, 0]))
   })