fix(core): do not refetch disabled queries (#3223)

* fix(core): do not refetch disabled queries with refetchQueries or invalidateQueries + refetchType "inactive" disabled queries (=queries that have observers which are all enabled:false) are matched as "inactive"; this is okay when searching for them via findAll or for removeQueries, but the docs clearly state that refetchQueries / invalidateQueries do not refetch disabled queries, and that the only way to refetch them is via refetch returned from useQuery; this is important when using enabled to signal that some dependencies are not yet ready some tests needed to be adapted because we used disabled observer + refetchQueries a lot. The easiest way to emulate the observers we wanted here was mostly with initialData + staleTime, and to get a real inactive query, we just need to subscribe + unsubscribe immediately * fix(core): do not refetch disabled queries add tests for refetchQueries + disabled * fix(core): do not refetch disabled queries update test to make more sense - title said disabled queries, but we had no disabled query; test now does the opposite of what it did before, but that's what this PR does :)
TanStack · Jan 23, 2022 · c0fc916 · c0fc916
1 parent d4b6afc
commit c0fc916
Show file tree

Hide file tree

Showing 5 changed files with 83 additions and 57 deletions.
diff --git a/src/core/query.ts b/src/core/query.ts
@@ -250,6 +250,10 @@ export class Query<
     return this.observers.some(observer => observer.options.enabled !== false)
   }
 
+  isDisabled(): boolean {
+    return this.getObserversCount() > 0 && !this.isActive()
+  }
+
   isStale(): boolean {
     return (
       this.state.isInvalidated ||

diff --git a/src/core/queryClient.ts b/src/core/queryClient.ts
@@ -283,13 +283,16 @@ export class QueryClient {
     const [filters, options] = parseFilterArgs(arg1, arg2, arg3)
 
     const promises = notifyManager.batch(() =>
-      this.queryCache.findAll(filters).map(query =>
-        query.fetch(undefined, {
-          ...options,
-          cancelRefetch: options?.cancelRefetch ?? true,
-          meta: { refetchPage: filters?.refetchPage },
-        })
-      )
+      this.queryCache
+        .findAll(filters)
+        .filter(query => !query.isDisabled())
+        .map(query =>
+          query.fetch(undefined, {
+            ...options,
+            cancelRefetch: options?.cancelRefetch ?? true,
+            meta: { refetchPage: filters?.refetchPage },
+          })
+        )
     )
 
     let promise = Promise.all(promises).then(noop)

diff --git a/src/core/tests/queryClient.test.tsx b/src/core/tests/queryClient.test.tsx
@@ -707,6 +707,41 @@ describe('queryClient', () => {
   })
 
   describe('refetchQueries', () => {
+    test('should not refetch if all observers are disabled', async () => {
+      const key = queryKey()
+      const queryFn = jest.fn()
+      await queryClient.fetchQuery(key, queryFn)
+      const observer1 = new QueryObserver(queryClient, {
+        queryKey: key,
+        queryFn,
+        enabled: false,
+      })
+      observer1.subscribe(() => undefined)
+      await queryClient.refetchQueries()
+      observer1.destroy()
+      expect(queryFn).toHaveBeenCalledTimes(1)
+    })
+    test('should refetch if at least one observer is enabled', async () => {
+      const key = queryKey()
+      const queryFn = jest.fn()
+      await queryClient.fetchQuery(key, queryFn)
+      const observer1 = new QueryObserver(queryClient, {
+        queryKey: key,
+        queryFn,
+        enabled: false,
+      })
+      const observer2 = new QueryObserver(queryClient, {
+        queryKey: key,
+        queryFn,
+        refetchOnMount: false,
+      })
+      observer1.subscribe(() => undefined)
+      observer2.subscribe(() => undefined)
+      await queryClient.refetchQueries()
+      observer1.destroy()
+      observer2.destroy()
+      expect(queryFn).toHaveBeenCalledTimes(2)
+    })
     test('should refetch all queries when no arguments are given', async () => {
       const key1 = queryKey()
       const key2 = queryKey()
@@ -716,11 +751,13 @@ describe('queryClient', () => {
       await queryClient.fetchQuery(key2, queryFn2)
       const observer1 = new QueryObserver(queryClient, {
         queryKey: key1,
-        enabled: false,
+        staleTime: Infinity,
+        initialData: 'initial',
       })
       const observer2 = new QueryObserver(queryClient, {
         queryKey: key1,
-        enabled: false,
+        staleTime: Infinity,
+        initialData: 'initial',
       })
       observer1.subscribe(() => undefined)
       observer2.subscribe(() => undefined)
@@ -964,13 +1001,14 @@ describe('queryClient', () => {
         queryKey: key1,
         queryFn: queryFn1,
         staleTime: Infinity,
-        enabled: false,
+        refetchOnMount: false,
       })
       const unsubscribe = observer.subscribe(() => undefined)
-      queryClient.invalidateQueries(key1, {
+      unsubscribe()
+
+      await queryClient.invalidateQueries(key1, {
         refetchType: 'inactive',
       })
-      unsubscribe()
       expect(queryFn1).toHaveBeenCalledTimes(2)
       expect(queryFn2).toHaveBeenCalledTimes(1)
     })
@@ -1002,23 +1040,19 @@ describe('queryClient', () => {
       let fetchCount = 0
       const observer = new QueryObserver(queryClient, {
         queryKey: key,
-        enabled: false,
+        queryFn: ({ signal }) => {
+          return new Promise(resolve => {
+            fetchCount++
+            setTimeout(() => resolve(5), 10)
+            if (signal) {
+              signal.addEventListener('abort', abortFn)
+            }
+          })
+        },
         initialData: 1,
       })
       observer.subscribe(() => undefined)
 
-      queryClient.fetchQuery(key, ({ signal }) => {
-        const promise = new Promise(resolve => {
-          fetchCount++
-          setTimeout(() => resolve(5), 10)
-          if (signal) {
-            signal.addEventListener('abort', abortFn)
-          }
-        })
-
-        return promise
-      })
-
       await queryClient.refetchQueries()
       observer.destroy()
       if (typeof AbortSignal === 'function') {
@@ -1033,23 +1067,19 @@ describe('queryClient', () => {
       let fetchCount = 0
       const observer = new QueryObserver(queryClient, {
         queryKey: key,
-        enabled: false,
+        queryFn: ({ signal }) => {
+          return new Promise(resolve => {
+            fetchCount++
+            setTimeout(() => resolve(5), 10)
+            if (signal) {
+              signal.addEventListener('abort', abortFn)
+            }
+          })
+        },
         initialData: 1,
       })
       observer.subscribe(() => undefined)
 
-      queryClient.fetchQuery(key, ({ signal }) => {
-        const promise = new Promise(resolve => {
-          fetchCount++
-          setTimeout(() => resolve(5), 10)
-          if (signal) {
-            signal.addEventListener('abort', abortFn)
-          }
-        })
-
-        return promise
-      })
-
       await queryClient.refetchQueries(undefined, { cancelRefetch: false })
       observer.destroy()
       if (typeof AbortSignal === 'function') {

diff --git a/src/devtools/devtools.tsx b/src/devtools/devtools.tsx
@@ -703,8 +703,6 @@ export const ReactQueryDevtoolsPanel = React.forwardRef<
             }}
           >
             {queries.map((query, i) => {
-              const isDisabled =
-                query.getObserversCount() > 0 && !query.isActive()
               return (
                 <div
                   key={query.queryHash || i}
@@ -747,7 +745,7 @@ export const ReactQueryDevtoolsPanel = React.forwardRef<
                   >
                     {query.getObserversCount()}
                   </div>
-                  {isDisabled ? (
+                  {query.isDisabled() ? (
                     <div
                       style={{
                         flex: '0 0 auto',

diff --git a/src/reactjs/tests/useQuery.test.tsx b/src/reactjs/tests/useQuery.test.tsx
@@ -1180,7 +1180,10 @@ describe('useQuery', () => {
           await sleep(1)
           return 'fetched'
         },
-        { enabled: false }
+        {
+          initialData: 'initial',
+          staleTime: Infinity,
+        }
       )
 
       results.push(result)
@@ -1266,7 +1269,7 @@ describe('useQuery', () => {
     })
   })
 
-  it('should update disabled query when updated with invalidateQueries', async () => {
+  it('should not update disabled query when refetched with refetchQueries', async () => {
     const key = queryKey()
     const states: UseQueryResult<number>[] = []
     let count = 0
@@ -1295,27 +1298,15 @@ describe('useQuery', () => {
 
     renderWithClient(queryClient, <Page />)
 
-    await sleep(100)
+    await sleep(50)
 
-    expect(states.length).toBe(3)
+    expect(states.length).toBe(1)
     expect(states[0]).toMatchObject({
       data: undefined,
       isFetching: false,
       isSuccess: false,
       isStale: true,
     })
-    expect(states[1]).toMatchObject({
-      data: undefined,
-      isFetching: true,
-      isSuccess: false,
-      isStale: true,
-    })
-    expect(states[2]).toMatchObject({
-      data: 1,
-      isFetching: false,
-      isSuccess: true,
-      isStale: true,
-    })
   })
 
   it('should not refetch disabled query when invalidated with invalidateQueries', async () => {