fix(scheduleGarbageCollection): Do not garbage collect newly cached queries of the same hash (#332)

* Add manual/prefetch cleanup test that fails

* Prefetch both queries in second test

* Add cleanup comment that will make the tests pass

* Store when the GC was initiated and compare against updatedAt

* Replace solution with state marker check and streamline test

* Undo setting.json change

* Undo setting.json change

Author: kamranayub

src/queryCache.js

@@ -17,6 +17,7 @@ export const queryCache = makeQueryCache()
 const actionInit = {}
 const actionFailed = {}
 const actionMarkStale = {}
+const actionMarkGC = {}
 const actionFetch = {}
 const actionSuccess = {}
 const actionError = {}
@@ -238,9 +239,12 @@ export function makeQueryCache() {
     }
 
     query.scheduleGarbageCollection = () => {
+      dispatch({ type: actionMarkGC })
       query.cacheTimeout = setTimeout(
         () => {
-          cache.removeQueries(d => d.queryHash === query.queryHash)
+          cache.removeQueries(
+            d => d.state.markedForGarbageCollection && d.queryHash === query.queryHash
+          )
         },
         typeof query.state.data === 'undefined' &&
           query.state.status !== 'error'
@@ -447,6 +451,7 @@ export function defaultQueryReducer(state, action) {
         canFetchMore: false,
         failureCount: 0,
         isStale: action.isStale,
+        markedForGarbageCollection: false,
         data: action.initialData,
         updatedAt: action.hasInitialData ? Date.now() : 0,
       }
@@ -460,6 +465,12 @@ export function defaultQueryReducer(state, action) {
         ...state,
         isStale: true,
       }
+    case actionMarkGC: {
+      return {
+        ...state,
+        markedForGarbageCollection: true,
+      }
+    }
     case actionFetch:
       return {
         ...state,

src/tests/queryCache.test.js

@@ -143,4 +143,34 @@ describe('queryCache', () => {
     await sleep(50)
     expect(query.state.isStale).toBe(false)
   })
+
+  test('query is garbage collected when unsubscribed to', async () => {
+    const queryKey = 'key'
+    const fetchData = () => Promise.resolve('data')
+    await queryCache.prefetchQuery(queryKey, fetchData, { cacheTime: 0 })
+    const query = queryCache.getQuery(queryKey)
+    expect(query.state.markedForGarbageCollection).toBe(false)
+    const unsubscribe = query.subscribe(1)
+    unsubscribe()
+    expect(query.state.markedForGarbageCollection).toBe(true)
+    await sleep(1)
+    expect(queryCache.getQuery(queryKey)).toBeUndefined()
+  })
+
+  test('query is not garbage collected unless markedForGarbageCollection is true', async () => {
+    const queryKey = 'key'
+    const fetchData = () => Promise.resolve(undefined)
+    await queryCache.prefetchQuery(queryKey, fetchData, { cacheTime: 0 })
+    const query = queryCache.getQuery(queryKey)
+    expect(query.state.markedForGarbageCollection).toBe(false)
+    const unsubscribe = query.subscribe(1)
+    unsubscribe()
+    expect(query.state.markedForGarbageCollection).toBe(true)
+    queryCache.clear()
+    queryCache.setQueryData(queryKey, 'data')
+    await sleep(1)
+    const newQuery = queryCache.getQuery(queryKey)
+    expect(newQuery.state.markedForGarbageCollection).toBe(false)
+    expect(newQuery.state.data).toBe('data')
+  })
 })

src/tests/useQuery.test.js

@@ -3,7 +3,6 @@ import {
   act,
   waitForElement,
   fireEvent,
-  cleanup,
 } from '@testing-library/react'
 import * as React from 'react'
 
@@ -13,7 +12,6 @@ import { sleep } from './utils'
 describe('useQuery', () => {
   afterEach(() => {
     queryCache.clear()
-    cleanup()
   })
 
   // See https://github.com/tannerlinsley/react-query/issues/105