V4: streamline cancel refetch (#2937)

* feat: streamline cancelRefetch

the following functions now default to true for cancelRefetch:

- refetchQueries (+invalidateQueries, + resetQueries)
- query.refetch
- fetchNextPage (unchanged)
- fetchPreviousPage (unchanged)

* feat: streamline cancelRefetch

make sure that refetchOnReconnect and refetchOnWindowFocus do not cancel already running requests

* feat: streamline cancelRefetch

update tests

refetch and invalidate now both cancel previous queries, which is intended, so we get more calls to the queryFn in these cases

* feat: streamline cancelRefetch

add more tests for cancelRefetch behavior

* feat: streamline cancelRefetch

update docs and migration guide

* feat: streamline cancelRefetch

simplify conditions by moving the ?? true default down to fetch on observer level; all 3 callers (fetchNextPage, fetchPreviousPage and refetch) just pass their options down and adhere to this default; refetch also only has 3 callers:
- refetch from useQuery, where we want the default
- onOnline and onFocus, where we now explicitly pass false to keep the previous behavior

and add more tests

* feat: streamline cancelRefetch

we always call this.fetch() with options, so we can just as well make the mandatory

also, streamline signatures by destructing values that can't be forwarded (and use empty object as default value) in options and just spread the rest

* feat: streamline cancelRefetch

fix types for refetch

it was accidentally made too wide and allowed all refetchFilters, like `predicate`; but with `refetch` on an obserserver, there is nothing to filter for, except the page, so that is what we need to accept via `RefetchPageFilters`

* feat: streamline cancelRefetch

refetch never took a queryKey as param - it is always bound to the observer

Author: TkDodo

docs/src/pages/guides/migrating-to-react-query-4.md

@@ -14,6 +14,34 @@ With version [3.22.0](https://github.com/tannerlinsley/react-query/releases/tag/
 + import { dehydrate, hydrate, useHydrate, Hydrate } from 'react-query'
 ```
 
+### Consistent behavior for `cancelRefetch`
 
+The `cancelRefetch` can be passed to all functions that imperatively fetch a query, namely:
 
+- `queryClient.refetchQueries`
+  - `queryClient.invalidateQueries`
+  - `queryClient.resetQueries`
+- `refetch` returned from `useQuery`
+- `fetchNetPage` and `fetchPreviousPage` returned from `useInfiniteQuery`
 
+Except for `fetchNetxPage` and `fetchPreviousPage`, this flag was defaulting to `false`, which was inconsistent and potentially troublesome: Calling `refetchQueries` or `invalidateQueries` after a mutation might not yield the latest result if a previous slow fetch was already ongoing, because this refetch would have been skipped.
+
+We believe that if a query is actively refetched by some code you write, it should, per default, re-start the fetch.
+
+That is why this flag now defaults to _true_ for all methods mentioned above. It also means that if you call `refetchQueries` twice in a row, without awaiting it, it will now cancel the first fetch and re-start it with the second one:
+
+```
+queryClient.refetchQueries({ queryKey: ['todos'] })
+// this will abort the previous refetch and start a new fetch
+queryClient.refetchQueries({ queryKey: ['todos'] })
+```
+
+You can opt-out of this behaviour by explicitly passing `cancelRefetch:false`:
+
+```
+queryClient.refetchQueries({ queryKey: ['todos'] })
+// this will not abort the previous refetch - it will just be ignored
+queryClient.refetchQueries({ queryKey: ['todos'] }, { cancelRefetch: false })
+```
+
+> Note: There is no change in behaviour for automatically triggered fetches, e.g. because a query mounts or because of a window focus refetch.

docs/src/pages/reference/QueryClient.md

@@ -295,8 +295,10 @@ await queryClient.invalidateQueries('posts', {
 - `options?: InvalidateOptions`:
   - `throwOnError?: boolean`
     - When set to `true`, this method will throw if any of the query refetch tasks fail.
-  - cancelRefetch?: boolean
-    - When set to `true`, then the current request will be cancelled before a new request is made
+  - `cancelRefetch?: boolean`
+    - Defaults to `true`
+      - Per default, a currently running request will be cancelled before a new request is made
+    - When set to `false`, no refetch will be made if there is already a request running.
 
 ## `queryClient.refetchQueries`
 
@@ -328,8 +330,10 @@ await queryClient.refetchQueries(['posts', 1], { active: true, exact: true })
 - `options?: RefetchOptions`:
   - `throwOnError?: boolean`
     - When set to `true`, this method will throw if any of the query refetch tasks fail.
-  - cancelRefetch?: boolean
-    - When set to `true`, then the current request will be cancelled before a new request is made
+  - `cancelRefetch?: boolean`
+    - Defaults to `true`
+      - Per default, a currently running request will be cancelled before a new request is made
+    - When set to `false`, no refetch will be made if there is already a request running.
 
 **Returns**
 
@@ -396,8 +400,10 @@ queryClient.resetQueries(queryKey, { exact: true })
 - `options?: ResetOptions`:
   - `throwOnError?: boolean`
     - When set to `true`, this method will throw if any of the query refetch tasks fail.
-  - cancelRefetch?: boolean
-    - When set to `true`, then the current request will be cancelled before a new request is made
+  - `cancelRefetch?: boolean`
+    - Defaults to `true`
+      - Per default, a currently running request will be cancelled before a new request is made
+    - When set to `false`, no refetch will be made if there is already a request running.
 
 **Returns**
 

docs/src/pages/reference/useQuery.md

@@ -236,6 +236,9 @@ const result = useQuery({
 - `refetch: (options: { throwOnError: boolean, cancelRefetch: boolean }) => Promise<UseQueryResult>`
   - A function to manually refetch the query.
   - If the query errors, the error will only be logged. If you want an error to be thrown, pass the `throwOnError: true` option
-  - If `cancelRefetch` is `true`, then the current request will be cancelled before a new request is made
+  - `cancelRefetch?: boolean`
+    - Defaults to `true`
+      - Per default, a currently running request will be cancelled before a new request is made
+    - When set to `false`, no refetch will be made if there is already a request running.
 - `remove: () => void`
   - A function to remove the query from the cache.

src/core/infiniteQueryObserver.ts

@@ -39,7 +39,7 @@ export class InfiniteQueryObserver<
 
   // Type override
   protected fetch!: (
-    fetchOptions?: ObserverFetchOptions
+    fetchOptions: ObserverFetchOptions
   ) => Promise<InfiniteQueryObserverResult<TData, TError>>
 
   // eslint-disable-next-line @typescript-eslint/no-useless-constructor
@@ -90,28 +90,27 @@ export class InfiniteQueryObserver<
     >
   }
 
-  fetchNextPage(
-    options?: FetchNextPageOptions
-  ): Promise<InfiniteQueryObserverResult<TData, TError>> {
+  fetchNextPage({ pageParam, ...options }: FetchNextPageOptions = {}): Promise<
+    InfiniteQueryObserverResult<TData, TError>
+  > {
     return this.fetch({
-      // TODO consider removing `?? true` in future breaking change, to be consistent with `refetch` API (see https://github.com/tannerlinsley/react-query/issues/2617)
-      cancelRefetch: options?.cancelRefetch ?? true,
-      throwOnError: options?.throwOnError,
+      ...options,
       meta: {
-        fetchMore: { direction: 'forward', pageParam: options?.pageParam },
+        fetchMore: { direction: 'forward', pageParam },
       },
     })
   }
 
-  fetchPreviousPage(
-    options?: FetchPreviousPageOptions
-  ): Promise<InfiniteQueryObserverResult<TData, TError>> {
+  fetchPreviousPage({
+    pageParam,
+    ...options
+  }: FetchPreviousPageOptions = {}): Promise<
+    InfiniteQueryObserverResult<TData, TError>
+  > {
     return this.fetch({
-      // TODO consider removing `?? true` in future breaking change, to be consistent with `refetch` API (see https://github.com/tannerlinsley/react-query/issues/2617)
-      cancelRefetch: options?.cancelRefetch ?? true,
-      throwOnError: options?.throwOnError,
+      ...options,
       meta: {
-        fetchMore: { direction: 'backward', pageParam: options?.pageParam },
+        fetchMore: { direction: 'backward', pageParam },
       },
     })
   }

src/core/query.ts

@@ -297,7 +297,7 @@ export class Query<
     const observer = this.observers.find(x => x.shouldFetchOnWindowFocus())
 
     if (observer) {
-      observer.refetch()
+      observer.refetch({ cancelRefetch: false })
     }
 
     // Continue fetch if currently paused
@@ -308,7 +308,7 @@ export class Query<
     const observer = this.observers.find(x => x.shouldFetchOnReconnect())
 
     if (observer) {
-      observer.refetch()
+      observer.refetch({ cancelRefetch: false })
     }
 
     // Continue fetch if currently paused

src/core/queryClient.ts

@@ -291,6 +291,7 @@ export class QueryClient {
       this.queryCache.findAll(filters).map(query =>
         query.fetch(undefined, {
           ...options,
+          cancelRefetch: options?.cancelRefetch ?? true,
           meta: { refetchPage: filters?.refetchPage },
         })
       )

src/core/queryObserver.ts

@@ -1,4 +1,4 @@
-import { RefetchQueryFilters } from './types'
+import { RefetchPageFilters } from './types'
 import {
   isServer,
   isValidTimeout,
@@ -278,12 +278,15 @@ export class QueryObserver<
     this.client.getQueryCache().remove(this.currentQuery)
   }
 
-  refetch<TPageData>(
-    options?: RefetchOptions & RefetchQueryFilters<TPageData>
-  ): Promise<QueryObserverResult<TData, TError>> {
+  refetch<TPageData>({
+    refetchPage,
+    ...options
+  }: RefetchOptions & RefetchPageFilters<TPageData> = {}): Promise<
+    QueryObserverResult<TData, TError>
+  > {
     return this.fetch({
       ...options,
-      meta: { refetchPage: options?.refetchPage },
+      meta: { refetchPage },
     })
   }
 
@@ -314,9 +317,12 @@ export class QueryObserver<
   }
 
   protected fetch(
-    fetchOptions?: ObserverFetchOptions
+    fetchOptions: ObserverFetchOptions
   ): Promise<QueryObserverResult<TData, TError>> {
-    return this.executeFetch(fetchOptions).then(() => {
+    return this.executeFetch({
+      ...fetchOptions,
+      cancelRefetch: fetchOptions.cancelRefetch ?? true,
+    }).then(() => {
       this.updateResult()
       return this.currentResult
     })

src/core/tests/queryClient.test.tsx

@@ -694,7 +694,8 @@ describe('queryClient', () => {
       queryClient.invalidateQueries(key1)
       await queryClient.refetchQueries({ stale: true })
       unsubscribe()
-      expect(queryFn1).toHaveBeenCalledTimes(2)
+      // fetchQuery, observer mount, invalidation (cancels observer mount) and refetch
+      expect(queryFn1).toHaveBeenCalledTimes(4)
       expect(queryFn2).toHaveBeenCalledTimes(1)
     })
 
@@ -711,7 +712,10 @@ describe('queryClient', () => {
         queryFn: queryFn1,
       })
       const unsubscribe = observer.subscribe()
-      await queryClient.refetchQueries({ active: true, stale: true })
+      await queryClient.refetchQueries(
+        { active: true, stale: true },
+        { cancelRefetch: false }
+      )
       unsubscribe()
       expect(queryFn1).toHaveBeenCalledTimes(2)
       expect(queryFn2).toHaveBeenCalledTimes(1)
@@ -940,9 +944,10 @@ describe('queryClient', () => {
       expect(queryFn2).toHaveBeenCalledTimes(1)
     })
 
-    test('should cancel ongoing fetches if cancelRefetch option is passed', async () => {
+    test('should cancel ongoing fetches if cancelRefetch option is set (default value)', async () => {
       const key = queryKey()
       const cancelFn = jest.fn()
+      let fetchCount = 0
       const observer = new QueryObserver(queryClient, {
         queryKey: key,
         enabled: false,
@@ -952,16 +957,45 @@ describe('queryClient', () => {
 
       queryClient.fetchQuery(key, () => {
         const promise = new Promise(resolve => {
+          fetchCount++
           setTimeout(() => resolve(5), 10)
         })
         // @ts-expect-error
         promise.cancel = cancelFn
         return promise
       })
 
-      await queryClient.refetchQueries(undefined, { cancelRefetch: true })
+      await queryClient.refetchQueries()
       observer.destroy()
       expect(cancelFn).toHaveBeenCalledTimes(1)
+      expect(fetchCount).toBe(2)
+    })
+
+    test('should not cancel ongoing fetches if cancelRefetch option is set to false', async () => {
+      const key = queryKey()
+      const cancelFn = jest.fn()
+      let fetchCount = 0
+      const observer = new QueryObserver(queryClient, {
+        queryKey: key,
+        enabled: false,
+        initialData: 1,
+      })
+      observer.subscribe()
+
+      queryClient.fetchQuery(key, () => {
+        const promise = new Promise(resolve => {
+          fetchCount++
+          setTimeout(() => resolve(5), 10)
+        })
+        // @ts-expect-error
+        promise.cancel = cancelFn
+        return promise
+      })
+
+      await queryClient.refetchQueries(undefined, { cancelRefetch: false })
+      observer.destroy()
+      expect(cancelFn).toHaveBeenCalledTimes(0)
+      expect(fetchCount).toBe(1)
     })
   })
 

src/core/tests/queryObserver.test.tsx

@@ -622,7 +622,7 @@ describe('queryObserver', () => {
       select: () => selectedData,
     })
 
-    await observer.refetch({ queryKey: key })
+    await observer.refetch()
     expect(observer.getCurrentResult().data).toBe(selectedData)
 
     unsubscribe()