Repository: tannerlinsley/react-query (ts)

Issue:
useQueries causes constant rerenders
**Describe the bug**
When using useQueries, the owning component rerenders repeatedly.

**To Reproduce**
Steps to reproduce the behavior:
1. call useQueries()
2. put a console log in the render method, or add a useeffect:
```ts
  const result = useQueries(...);
  useEffect(() => {
    console.log('result changed!');
  }, [result]);
```

**Expected behavior**
The component should not be rerendered constantly


**Desktop (please complete the following information):**
 - OS: MacOS
 - Browser:  Chrome
 - Version: 96


**Additional context**

This is due to a missing `useMemo()`: https://github.com/tannerlinsley/react-query/blob/master/src/react/useQueries.ts#L121

The problem is that later in that function, a useEffect fires, which updates state, which causes everything to rerender, which causes this array to be generated again.

A simple `useMemo()` fixes the problem. Working on a PR now

Pull request:
Add useMemo() to useQueries() to avoid constant rerendering
fixes #2991 

Patch:
diff --git a/.all-contributorsrc b/.all-contributorsrc
index b8ad28671..432147811 100644
--- a/.all-contributorsrc
+++ b/.all-contributorsrc
@@ -174,6 +174,16 @@
       "contributions": [
         "code"
       ]
+    },
+    {
+      "login": "babycourageous",
+      "name": "Rene Dellefont",
+      "avatar_url": "https://avatars.githubusercontent.com/u/14936212?v=4",
+      "profile": "https://github.com/babycourageous",
+      "contributions": [
+        "code",
+        "doc"
+      ]
     }
   ],
   "contributorsPerLine": 7,
diff --git a/README.md b/README.md
index 3e902d293..cab9ac311 100644
--- a/README.md
+++ b/README.md
@@ -79,6 +79,7 @@ Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/d
     <td align="center"><a href="https://katt.dev"><img src="https://avatars.githubusercontent.com/u/459267?v=4?s=100" width="100px;" alt=""/><br /><sub><b>Alex Johansson</b></sub></a><br /><a href="https://github.com/tannerlinsley/react-query/commits?author=KATT" title="Code">💻</a></td>
     <td align="center"><a href="http://www.eddyvinck.nl"><img src="https://avatars.githubusercontent.com/u/23434753?v=4?s=100" width="100px;" alt=""/><br /><sub><b>Eddy</b></sub></a><br /><a href="https://github.com/tannerlinsley/react-query/commits?author=EddyVinck" title="Code">💻</a></td>
     <td align="center"><a href="http://prateeksurana.me"><img src="https://avatars.githubusercontent.com/u/21277179?v=4?s=100" width="100px;" alt=""/><br /><sub><b>Prateek Surana</b></sub></a><br /><a href="https://github.com/tannerlinsley/react-query/commits?author=prateek3255" title="Code">💻</a></td>
+    <td align="center"><a href="https://github.com/babycourageous"><img src="https://avatars.githubusercontent.com/u/14936212?v=4?s=100" width="100px;" alt=""/><br /><sub><b>Rene Dellefont</b></sub></a><br /><a href="https://github.com/tannerlinsley/react-query/commits?author=babycourageous" title="Code">💻</a> <a href="https://github.com/tannerlinsley/react-query/commits?author=babycourageous" title="Documentation">📖</a></td>
   </tr>
 </table>
 
diff --git a/docs/src/pages/guides/query-keys.md b/docs/src/pages/guides/query-keys.md
index eb1cdc67b..f9239ea91 100644
--- a/docs/src/pages/guides/query-keys.md
+++ b/docs/src/pages/guides/query-keys.md
@@ -34,7 +34,7 @@ When a query needs more information to uniquely describe its data, you can use a
 useQuery(['todo', 5], ...)
 // queryKey === ['todo', 5]
 
-// And individual todo in a "preview" format
+// An individual todo in a "preview" format
 useQuery(['todo', 5, { preview: true }], ...)
 // queryKey === ['todo', 5, { preview: true }]
 
diff --git a/docs/src/pages/guides/window-focus-refetching.md b/docs/src/pages/guides/window-focus-refetching.md
index 2d41e1765..dc3b757d3 100644
--- a/docs/src/pages/guides/window-focus-refetching.md
+++ b/docs/src/pages/guides/window-focus-refetching.md
@@ -87,3 +87,7 @@ focusManager.setFocused(true)
 // Fallback to the default focus check
 focusManager.setFocused(undefined)
 ```
+
+## Pitfalls & Caveats
+
+Some browser internal dialogue windows, such as spawned by `alert()` or file upload dialogues (as created by `<input type="file" />`) might also trigger focus refetching after they close. This can result in unwanted side effects, as the refetching might trigger component unmounts or remounts before your file upload handler is executed. See [this issue on GitHub](https://github.com/tannerlinsley/react-query/issues/2960) for background and possible workarounds.
\ No newline at end of file
diff --git a/src/core/query.ts b/src/core/query.ts
index 336e9b874..c275876e9 100644
--- a/src/core/query.ts
+++ b/src/core/query.ts
@@ -163,9 +163,11 @@ export class Query<
   private observers: QueryObserver<any, any, any, any, any>[]
   private defaultOptions?: QueryOptions<TQueryFnData, TError, TData, TQueryKey>
   private abortSignalConsumed: boolean
+  private hadObservers: boolean
 
   constructor(config: QueryConfig<TQueryFnData, TError, TData, TQueryKey>) {
     this.abortSignalConsumed = false
+    this.hadObservers = false
     this.defaultOptions = config.defaultOptions
     this.setOptions(config.options)
     this.observers = []
@@ -214,8 +216,14 @@ export class Query<
   }
 
   private optionalRemove() {
-    if (!this.observers.length && !this.state.isFetching) {
-      this.cache.remove(this)
+    if (!this.observers.length) {
+      if (this.state.isFetching) {
+        if (this.hadObservers) {
+          this.scheduleGc()
+        }
+      } else {
+        this.cache.remove(this)
+      }
     }
   }
 
@@ -318,6 +326,7 @@ export class Query<
   addObserver(observer: QueryObserver<any, any, any, any, any>): void {
     if (this.observers.indexOf(observer) === -1) {
       this.observers.push(observer)
+      this.hadObservers = true
 
       // Stop the query from being garbage collected
       this.clearGcTimeout()
diff --git a/src/core/queryObserver.ts b/src/core/queryObserver.ts
index f1ab5c8d2..aa36cfd2c 100644
--- a/src/core/queryObserver.ts
+++ b/src/core/queryObserver.ts
@@ -232,24 +232,38 @@ export class QueryObserver<
   }
 
   trackResult(
-    result: QueryObserverResult<TData, TError>
+    result: QueryObserverResult<TData, TError>,
+    defaultedOptions: QueryObserverOptions<
+      TQueryFnData,
+      TError,
+      TData,
+      TQueryData,
+      TQueryKey
+    >
   ): QueryObserverResult<TData, TError> {
     const trackedResult = {} as QueryObserverResult<TData, TError>
 
+    const trackProp = (key: keyof QueryObserverResult) => {
+      if (!this.trackedProps.includes(key)) {
+        this.trackedProps.push(key)
+      }
+    }
+
     Object.keys(result).forEach(key => {
       Object.defineProperty(trackedResult, key, {
         configurable: false,
         enumerable: true,
         get: () => {
-          const typedKey = key as keyof QueryObserverResult
-          if (!this.trackedProps.includes(typedKey)) {
-            this.trackedProps.push(typedKey)
-          }
-          return result[typedKey]
+          trackProp(key as keyof QueryObserverResult)
+          return result[key as keyof QueryObserverResult]
         },
       })
     })
 
+    if (defaultedOptions.useErrorBoundary || defaultedOptions.suspense) {
+      trackProp('error')
+    }
+
     return trackedResult
   }
 
diff --git a/src/react/useBaseQuery.ts b/src/react/useBaseQuery.ts
index 693944fc3..3adb64e05 100644
--- a/src/react/useBaseQuery.ts
+++ b/src/react/useBaseQuery.ts
@@ -144,7 +144,7 @@ export function useBaseQuery<
 
   // Handle result property usage tracking
   if (defaultedOptions.notifyOnChangeProps === 'tracked') {
-    result = observer.trackResult(result)
+    result = observer.trackResult(result, defaultedOptions)
   }
 
   return result
diff --git a/src/react/useQueries.ts b/src/react/useQueries.ts
index 2e7ec90d3..cd2d5d0ad 100644
--- a/src/react/useQueries.ts
+++ b/src/react/useQueries.ts
@@ -1,4 +1,4 @@
-import React from 'react'
+import React, { useMemo } from 'react'
 import { QueryFunction } from '../core/types'
 
 import { notifyManager } from '../core/notifyManager'
@@ -118,14 +118,20 @@ export function useQueries<T extends any[]>(
 
   const queryClient = useQueryClient()
 
-  const defaultedQueries = queries.map(options => {
-    const defaultedOptions = queryClient.defaultQueryObserverOptions(options)
+  const defaultedQueries = useMemo(
+    () =>
+      queries.map(options => {
+        const defaultedOptions = queryClient.defaultQueryObserverOptions(
+          options
+        )
 
-    // Make sure the results are already in fetching state before subscribing or updating options
-    defaultedOptions.optimisticResults = true
+        // Make sure the results are already in fetching state before subscribing or updating options
+        defaultedOptions.optimisticResults = true
 
-    return defaultedOptions
-  })
+        return defaultedOptions
+      }),
+    [queries, queryClient]
+  )
 
   const [observer] = React.useState(
     () => new QueriesObserver(queryClient, defaultedQueries)


Test patch:
diff --git a/src/core/tests/query.test.tsx b/src/core/tests/query.test.tsx
index 583799cfb..efdc4894e 100644
--- a/src/core/tests/query.test.tsx
+++ b/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, {
diff --git a/src/core/tests/queryClient.test.tsx b/src/core/tests/queryClient.test.tsx
index 5e0ee089a..30f8e2996 100644
--- a/src/core/tests/queryClient.test.tsx
+++ b/src/core/tests/queryClient.test.tsx
@@ -544,6 +544,21 @@ describe('queryClient', () => {
 
       consoleMock.mockRestore()
     })
+
+    test('should be garbage collected after cacheTime if unused', async () => {
+      const key = queryKey()
+
+      await queryClient.prefetchQuery(
+        key,
+        async () => {
+          return 'data'
+        },
+        { cacheTime: 10 }
+      )
+      expect(queryCache.find(key)).toBeDefined()
+      await sleep(15)
+      expect(queryCache.find(key)).not.toBeDefined()
+    })
   })
 
   describe('removeQueries', () => {
diff --git a/src/react/tests/QueryResetErrorBoundary.test.tsx b/src/react/tests/QueryResetErrorBoundary.test.tsx
index f73210d25..5fc1e366f 100644
--- a/src/react/tests/QueryResetErrorBoundary.test.tsx
+++ b/src/react/tests/QueryResetErrorBoundary.test.tsx
@@ -592,4 +592,64 @@ describe('QueryErrorResetBoundary', () => {
 
     expect(rendered.queryByText('page')).not.toBeNull()
   })
+
+  it('should show error boundary when using tracked queries even though we do not track the error field', async () => {
+    const key = queryKey()
+
+    let succeed = false
+    const consoleMock = mockConsoleError()
+
+    function Page() {
+      const { data } = useQuery(
+        key,
+        async () => {
+          await sleep(10)
+          if (!succeed) {
+            throw new Error('Error')
+          } else {
+            return 'data'
+          }
+        },
+        {
+          retry: false,
+          useErrorBoundary: true,
+          notifyOnChangeProps: 'tracked',
+        }
+      )
+      return <div>{data}</div>
+    }
+
+    const rendered = renderWithClient(
+      queryClient,
+      <QueryErrorResetBoundary>
+        {({ reset }) => (
+          <ErrorBoundary
+            onReset={reset}
+            fallbackRender={({ resetErrorBoundary }) => (
+              <div>
+                <div>error boundary</div>
+                <button
+                  onClick={() => {
+                    resetErrorBoundary()
+                  }}
+                >
+                  retry
+                </button>
+              </div>
+            )}
+          >
+            <Page />
+          </ErrorBoundary>
+        )}
+      </QueryErrorResetBoundary>
+    )
+
+    await waitFor(() => rendered.getByText('error boundary'))
+    await waitFor(() => rendered.getByText('retry'))
+    succeed = true
+    fireEvent.click(rendered.getByText('retry'))
+    await waitFor(() => rendered.getByText('data'))
+
+    consoleMock.mockRestore()
+  })
 })