fix(useQuery): cleanup queries even if they have been fetching (#2950)

* fix(useQuery): cleanup queries even if they have been fetching

* fix(useQuery): cleanup queries even if they have been fetching

do not re-schedule garbage collection if a query is fetching and we never had any observers subscribed; this is necessary to make suspense work, because with suspense, we always throw before we subscribe, so the garbage collection would prematurely remove the query, resulting in an infinite loop

* fix(useQuery): cleanup queries even if they have been fetching

only schedule garbage collection when the last observer unsubscribes rather than in the constructor

this works around the suspense issue because we only add observers after we've loaded as we throw before that.

Author: TkDodo

src/core/query.ts

@@ -175,7 +175,6 @@ export class Query<
     this.initialState = config.state || this.getDefaultState(this.options)
     this.state = this.initialState
     this.meta = config.meta
-    this.scheduleGc()
   }
 
   private setOptions(
@@ -214,8 +213,12 @@ export class Query<
   }
 
   private optionalRemove() {
-    if (!this.observers.length && !this.state.isFetching) {
-      this.cache.remove(this)
+    if (!this.observers.length) {
+      if (this.state.isFetching) {
+        this.scheduleGc()
+      } else {
+        this.cache.remove(this)
+      }
     }
   }
 

src/core/tests/query.test.tsx

@@ -544,6 +544,29 @@ describe('query', () => {
     expect(queryCache.find(key)).toBeUndefined()
   })
 
+  test('should be garbage collected later when unsubscribed and query is fetching', async () => {
+    const key = queryKey()
+    const observer = new QueryObserver(queryClient, {
+      queryKey: key,
+      queryFn: async () => {
+        await sleep(20)
+        return 'data'
+      },
+      cacheTime: 10,
+    })
+    const unsubscribe = observer.subscribe()
+    await sleep(20)
+    expect(queryCache.find(key)).toBeDefined()
+    observer.refetch()
+    unsubscribe()
+    await sleep(10)
+    // unsubscribe should not remove even though cacheTime has elapsed b/c query is still fetching
+    expect(queryCache.find(key)).toBeDefined()
+    await sleep(10)
+    // should be removed after an additional staleTime wait
+    expect(queryCache.find(key)).toBeUndefined()
+  })
+
   test('should not be garbage collected unless there are no subscribers', async () => {
     const key = queryKey()
     const observer = new QueryObserver(queryClient, {