Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

V4: streamline cancel refetch #2937

Merged
merged 9 commits into from
Nov 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions docs/src/pages/guides/migrating-to-react-query-4.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
18 changes: 12 additions & 6 deletions docs/src/pages/reference/QueryClient.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`

Expand Down Expand Up @@ -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**

Expand Down Expand Up @@ -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**

Expand Down
5 changes: 4 additions & 1 deletion docs/src/pages/reference/useQuery.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
29 changes: 14 additions & 15 deletions src/core/infiniteQueryObserver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 },
},
})
}
Expand Down
4 changes: 2 additions & 2 deletions src/core/query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions src/core/queryClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
})
)
Expand Down
20 changes: 13 additions & 7 deletions src/core/queryObserver.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { RefetchQueryFilters } from './types'
import { RefetchPageFilters } from './types'
import {
isServer,
isValidTimeout,
Expand Down Expand Up @@ -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 },
})
}

Expand Down Expand Up @@ -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
})
Expand Down
42 changes: 38 additions & 4 deletions src/core/tests/queryClient.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})

Expand All @@ -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)
Expand Down Expand Up @@ -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,
Expand All @@ -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)
})
})

Expand Down
2 changes: 1 addition & 1 deletion src/core/tests/queryObserver.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -622,7 +622,7 @@ describe('queryObserver', () => {
select: () => selectedData,
})

await observer.refetch({ queryKey: key })
await observer.refetch()
expect(observer.getCurrentResult().data).toBe(selectedData)

unsubscribe()
Expand Down
Loading