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

refactor: Remove deprecated promise cancel #2996

Merged
merged 10 commits into from
Nov 22, 2021
4 changes: 4 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 @@ -148,6 +148,10 @@ Since these plugins are no longer experimental, their import paths have also bee
+ import { createAsyncStoragePersister } from 'react-query/createAsyncStoragePersister'
```

### The `cancel` method on promises is no longer supported

The [old `cancel` method](../guides/query-cancellation#old-cancel-function) that allowed you to define a `cancel` function on promises, which was then used by the library to support query cancellation, has been removed. We recommend to use the [newer API](../guides/query-cancellation) (introduced with v3.30.0) for query cancellation that uses the [`AbortController` API](https://developer.mozilla.org/en-US/docs/Web/API/AbortController) internally and provides you with an [`AbortSignal` instance](https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal) for your query function to support query cancellation.

## New Features 🚀

### Mutation Cache Garbage Collection
Expand Down
16 changes: 3 additions & 13 deletions src/core/infiniteQueryBehavior.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { QueryBehavior } from './query'
import { isCancelable } from './retryer'

import type {
InfiniteData,
QueryFunctionContext,
Expand Down Expand Up @@ -73,11 +73,6 @@ export function infiniteQueryBehavior<
buildNewPages(pages, param, page, previous)
)

if (isCancelable(queryFnResult)) {
const promiseAsAny = promise as any
promiseAsAny.cancel = queryFnResult.cancel
}

return promise
}

Expand Down Expand Up @@ -148,15 +143,10 @@ export function infiniteQueryBehavior<
pageParams: newPageParams,
}))

const finalPromiseAsAny = finalPromise as any

finalPromiseAsAny.cancel = () => {
context.signal?.addEventListener('abort', () => {
cancelled = true
abortController?.abort()
if (isCancelable(promise)) {
promise.cancel()
}
}
})

return finalPromise
}
Expand Down
32 changes: 21 additions & 11 deletions src/core/query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ export interface FetchContext<
> {
fetchFn: () => unknown | Promise<unknown>
fetchOptions?: FetchOptions
signal?: AbortSignal
TkDodo marked this conversation as resolved.
Show resolved Hide resolved
options: QueryOptions<TQueryFnData, TError, TData, any>
queryKey: EnsuredQueryKey<TQueryKey>
state: QueryState<TData, TError>
Expand Down Expand Up @@ -316,7 +317,7 @@ export class Query<
// If the transport layer does not support cancellation
// we'll let the query continue so the result can be cached
if (this.retryer) {
if (this.retryer.isTransportCancelable || this.abortSignalConsumed) {
if (this.abortSignalConsumed) {
this.retryer.cancel({ revert: true })
} else {
this.retryer.cancelRetry()
Expand Down Expand Up @@ -382,16 +383,23 @@ export class Query<
meta: this.meta,
}

Object.defineProperty(queryFnContext, 'signal', {
enumerable: true,
get: () => {
if (abortController) {
this.abortSignalConsumed = true
return abortController.signal
}
return undefined
},
})
// Adds an enumerable signal property to the object that
// which sets abortSignalConsumed to true when the signal
// is read.
const addSignalProperty = (object: unknown) => {
Object.defineProperty(object, 'signal', {
enumerable: true,
get: () => {
if (abortController) {
this.abortSignalConsumed = true
return abortController.signal
}
return undefined
},
})
}

addSignalProperty(queryFnContext)

// Create fetch function
const fetchFn = () => {
Expand All @@ -412,6 +420,8 @@ export class Query<
meta: this.meta,
}

addSignalProperty(context)

if (this.options.behavior?.onFetch) {
this.options.behavior?.onFetch(context)
}
Expand Down
21 changes: 0 additions & 21 deletions src/core/retryer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,6 @@ type RetryDelayFunction<TError = unknown> = (
function defaultRetryDelay(failureCount: number) {
return Math.min(1000 * 2 ** failureCount, 30000)
}

interface Cancelable {
cancel(): void
}

export function isCancelable(value: any): value is Cancelable {
return typeof value?.cancel === 'function'
}

export class CancelledError {
revert?: boolean
silent?: boolean
Expand All @@ -65,7 +56,6 @@ export class Retryer<TData = unknown, TError = unknown> {
failureCount: number
isPaused: boolean
isResolved: boolean
isTransportCancelable: boolean
promise: Promise<TData>

private abort?: () => void
Expand All @@ -86,7 +76,6 @@ export class Retryer<TData = unknown, TError = unknown> {
this.failureCount = 0
this.isPaused = false
this.isResolved = false
this.isTransportCancelable = false
this.promise = new Promise<TData>((outerResolve, outerReject) => {
promiseResolve = outerResolve
promiseReject = outerReject
Expand Down Expand Up @@ -144,19 +133,9 @@ export class Retryer<TData = unknown, TError = unknown> {
reject(new CancelledError(cancelOptions))

this.abort?.()

// Cancel transport if supported
if (isCancelable(promiseOrValue)) {
try {
promiseOrValue.cancel()
} catch {}
}
}
}

// Check if the transport layer support cancellation
this.isTransportCancelable = isCancelable(promiseOrValue)

Promise.resolve(promiseOrValue)
.then(resolve)
.catch(error => {
Expand Down
37 changes: 0 additions & 37 deletions src/core/tests/query.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -329,43 +329,6 @@ describe('query', () => {
expect(isCancelledError(error)).toBe(true)
})

test('should call cancel() fn if it was provided and not continue when last observer unsubscribed', async () => {
const key = queryKey()

const cancel = jest.fn()

queryClient.prefetchQuery(key, async () => {
const promise = new Promise((resolve, reject) => {
sleep(100).then(() => resolve('data'))
cancel.mockImplementation(() => {
reject(new Error('Cancelled'))
})
}) as any
promise.cancel = cancel
return promise
})

await sleep(10)

// Subscribe and unsubscribe to simulate cancellation because the last observer unsubscribed
const observer = new QueryObserver(queryClient, {
queryKey: key,
enabled: false,
})
const unsubscribe = observer.subscribe()
unsubscribe()

await sleep(100)

const query = queryCache.find(key)!

expect(cancel).toHaveBeenCalled()
expect(query.state).toMatchObject({
data: undefined,
status: 'idle',
})
})

test('should not continue if explicitly cancelled', async () => {
const key = queryKey()

Expand Down
28 changes: 18 additions & 10 deletions src/core/tests/queryClient.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -927,7 +927,7 @@ describe('queryClient', () => {

test('should cancel ongoing fetches if cancelRefetch option is set (default value)', async () => {
const key = queryKey()
const cancelFn = jest.fn()
const abortFn = jest.fn()
let fetchCount = 0
const observer = new QueryObserver(queryClient, {
queryKey: key,
Expand All @@ -936,25 +936,29 @@ describe('queryClient', () => {
})
observer.subscribe()

queryClient.fetchQuery(key, () => {
queryClient.fetchQuery(key, ({ signal }) => {
const promise = new Promise(resolve => {
fetchCount++
setTimeout(() => resolve(5), 10)
if (signal) {
signal.addEventListener('abort', abortFn)
}
})
// @ts-expect-error
promise.cancel = cancelFn

return promise
})

await queryClient.refetchQueries()
observer.destroy()
expect(cancelFn).toHaveBeenCalledTimes(1)
if (typeof AbortSignal === 'function') {
expect(abortFn).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()
const abortFn = jest.fn()
let fetchCount = 0
const observer = new QueryObserver(queryClient, {
queryKey: key,
Expand All @@ -963,19 +967,23 @@ describe('queryClient', () => {
})
observer.subscribe()

queryClient.fetchQuery(key, () => {
queryClient.fetchQuery(key, ({ signal }) => {
const promise = new Promise(resolve => {
fetchCount++
setTimeout(() => resolve(5), 10)
if (signal) {
signal.addEventListener('abort', abortFn)
}
})
// @ts-expect-error
promise.cancel = cancelFn

return promise
})

await queryClient.refetchQueries(undefined, { cancelRefetch: false })
observer.destroy()
expect(cancelFn).toHaveBeenCalledTimes(0)
if (typeof AbortSignal === 'function') {
expect(abortFn).toHaveBeenCalledTimes(0)
}
expect(fetchCount).toBe(1)
})
})
Expand Down
9 changes: 5 additions & 4 deletions src/reactjs/tests/useInfiniteQuery.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1782,14 +1782,13 @@ describe('useInfiniteQuery', () => {
const key = queryKey()
let cancelFn: jest.Mock = jest.fn()

const queryFn = () => {
const queryFn = ({ signal }: { signal?: AbortSignal }) => {
const promise = new Promise<string>((resolve, reject) => {
cancelFn = jest.fn(() => reject('Cancelled'))
signal?.addEventListener('abort', cancelFn)
sleep(10).then(() => resolve('OK'))
})

;(promise as any).cancel = cancelFn

return promise
}

Expand All @@ -1811,6 +1810,8 @@ describe('useInfiniteQuery', () => {

await waitFor(() => rendered.getByText('off'))

expect(cancelFn).toHaveBeenCalled()
if (typeof AbortSignal === 'function') {
expect(cancelFn).toHaveBeenCalled()
}
})
})
9 changes: 5 additions & 4 deletions src/reactjs/tests/useQuery.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3952,14 +3952,13 @@ describe('useQuery', () => {
const key = queryKey()
let cancelFn: jest.Mock = jest.fn()

const queryFn = () => {
const queryFn = ({ signal }: { signal?: AbortSignal }) => {
const promise = new Promise<string>((resolve, reject) => {
cancelFn = jest.fn(() => reject('Cancelled'))
signal?.addEventListener('abort', cancelFn)
sleep(10).then(() => resolve('OK'))
})

;(promise as any).cancel = cancelFn

return promise
}

Expand All @@ -3981,7 +3980,9 @@ describe('useQuery', () => {

await waitFor(() => rendered.getByText('off'))

expect(cancelFn).toHaveBeenCalled()
if (typeof AbortSignal === 'function') {
expect(cancelFn).toHaveBeenCalled()
}
})

it('should cancel the query if the signal was consumed and there are no more subscriptions', async () => {
Expand Down