diff --git a/packages/query-core/src/__tests__/hydration.test.tsx b/packages/query-core/src/__tests__/hydration.test.tsx index 5582e9da16..30a4b9a985 100644 --- a/packages/query-core/src/__tests__/hydration.test.tsx +++ b/packages/query-core/src/__tests__/hydration.test.tsx @@ -1398,8 +1398,5 @@ describe('dehydration and rehydration', () => { // Need to await the original promise or else it will get a cancellation // error and test will fail await originalPromise - - clientQueryClient.clear() - serverQueryClient.clear() }) }) diff --git a/packages/query-core/src/__tests__/infiniteQueryBehavior.test.tsx b/packages/query-core/src/__tests__/infiniteQueryBehavior.test.tsx index 523a30ca20..777466ef38 100644 --- a/packages/query-core/src/__tests__/infiniteQueryBehavior.test.tsx +++ b/packages/query-core/src/__tests__/infiniteQueryBehavior.test.tsx @@ -319,7 +319,9 @@ describe('InfiniteQueryBehavior', () => { // Cancel the query const query = observer.getCurrentQuery() - query.cancel() + await query.cancel() + + vi.advanceTimersByTime(10) expect(observerResult).toMatchObject({ isFetching: false, diff --git a/packages/query-core/src/__tests__/query.test.tsx b/packages/query-core/src/__tests__/query.test.tsx index 25ee912637..ee1bbc45fe 100644 --- a/packages/query-core/src/__tests__/query.test.tsx +++ b/packages/query-core/src/__tests__/query.test.tsx @@ -5,12 +5,12 @@ import { sleep, } from '@tanstack/query-test-utils' import { + CancelledError, Query, QueryClient, QueryObserver, dehydrate, hydrate, - isCancelledError, } from '..' import { hashQueryKeyByOptions } from '../utils' import { mockOnlineManagerIsOnline, setIsServer } from './utils' @@ -190,14 +190,52 @@ describe('query', () => { await promise expect.unreachable() } catch { - expect(isCancelledError(result)).toBe(true) - expect(result instanceof Error).toBe(true) + expect(result).toBeInstanceOf(CancelledError) } finally { // Reset visibilityState to original value visibilityMock.mockRestore() } }) + test('should not throw a CancelledError when fetchQuery is in progress and the last observer unsubscribes when AbortSignal is consumed', async () => { + const key = queryKey() + + const observer = new QueryObserver(queryClient, { + queryKey: key, + queryFn: async () => { + await sleep(100) + return 'data' + }, + }) + + const unsubscribe = observer.subscribe(() => undefined) + await vi.advanceTimersByTimeAsync(100) + + expect(queryCache.find({ queryKey: key })?.state.data).toBe('data') + + const promise = queryClient.fetchQuery({ + queryKey: key, + queryFn: async ({ signal }) => { + await sleep(100) + return 'data2' + String(signal) + }, + }) + + // Ensure the fetch is in progress + await vi.advanceTimersByTimeAsync(10) + + // Unsubscribe while fetch is in progress + unsubscribe() + // await queryClient.cancelQueries() + + await vi.advanceTimersByTimeAsync(90) + + // Fetch should complete successfully without throwing a CancelledError + await expect(promise).resolves.toBe('data') + + expect(queryCache.find({ queryKey: key })?.state.data).toBe('data') + }) + test('should provide context to queryFn', () => { const key = queryKey() @@ -330,7 +368,7 @@ describe('query', () => { expect(signal.aborted).toBe(true) expect(onAbort).toHaveBeenCalledTimes(1) expect(abortListener).toHaveBeenCalledTimes(1) - expect(isCancelledError(error)).toBe(true) + expect(error).toBeInstanceOf(CancelledError) }) test('should not continue if explicitly cancelled', async () => { @@ -362,7 +400,7 @@ describe('query', () => { await vi.advanceTimersByTimeAsync(100) expect(queryFn).toHaveBeenCalledTimes(1) - expect(isCancelledError(error)).toBe(true) + expect(error).toBeInstanceOf(CancelledError) }) test('should not error if reset while pending', async () => { @@ -375,7 +413,18 @@ describe('query', () => { throw new Error() }) - queryClient.fetchQuery({ queryKey: key, queryFn, retry: 3, retryDelay: 10 }) + let error + + queryClient + .fetchQuery({ + queryKey: key, + queryFn, + retry: 3, + retryDelay: 10, + }) + .catch((e) => { + error = e + }) // Ensure the query is pending const query = queryCache.find({ queryKey: key })! @@ -390,6 +439,12 @@ describe('query', () => { expect(queryFn).toHaveBeenCalledTimes(1) // have been called, expect(query.state.error).toBe(null) // not have an error, and expect(query.state.fetchStatus).toBe('idle') // not be loading any longer + expect(query.state.data).toBe(undefined) // have no data + + // the call to fetchQuery must reject + // because it was reset and not reverted + // so it would resolve with undefined otherwise + expect(error).toBeInstanceOf(CancelledError) }) test('should reset to default state when created from hydration', async () => { @@ -426,7 +481,7 @@ describe('query', () => { await vi.advanceTimersByTimeAsync(100) expect(queryFn).toHaveBeenCalledTimes(1) - expect(isCancelledError(query.state.error)).toBe(true) + expect(query.state.error).toBeInstanceOf(CancelledError) const result = query.fetch() await vi.advanceTimersByTimeAsync(50) await expect(result).resolves.toBe('data') @@ -459,7 +514,7 @@ describe('query', () => { await vi.advanceTimersByTimeAsync(10) expect(query.state.error).toBe(error) - expect(isCancelledError(query.state.error)).toBe(false) + expect(query.state.error).not.toBeInstanceOf(CancelledError) }) test('the previous query status should be kept when refetching', async () => { @@ -900,7 +955,7 @@ describe('query', () => { unsubscribe() }) - test('should always revert to idle state (#5958)', async () => { + test('should always revert to idle state (#5968)', async () => { let mockedData = [1] const key = queryKey() @@ -934,24 +989,69 @@ describe('query', () => { const unsubscribe = observer.subscribe(() => undefined) await vi.advanceTimersByTimeAsync(50) // let it resolve + expect(observer.getCurrentResult().data).toBe('1') + expect(observer.getCurrentResult().fetchStatus).toBe('idle') + mockedData = [1, 2] // update "server" state in the background - queryClient.invalidateQueries({ queryKey: key }) - queryClient.invalidateQueries({ queryKey: key }) + void queryClient.invalidateQueries({ queryKey: key }) + await vi.advanceTimersByTimeAsync(5) + void queryClient.invalidateQueries({ queryKey: key }) + await vi.advanceTimersByTimeAsync(5) unsubscribe() // unsubscribe to simulate unmount + await vi.advanceTimersByTimeAsync(5) + + // reverted to previous data and idle fetchStatus + expect(queryCache.find({ queryKey: key })?.state).toMatchObject({ + status: 'success', + data: '1', + fetchStatus: 'idle', + }) // set up a new observer to simulate a mount of new component const newObserver = new QueryObserver(queryClient, { queryKey: key, queryFn, }) - const spy = vi.fn() newObserver.subscribe(({ data }) => spy(data)) await vi.advanceTimersByTimeAsync(60) // let it resolve expect(spy).toHaveBeenCalledWith('1 - 2') }) + test('should not reject a promise when silently cancelled in the background', async () => { + const key = queryKey() + + let x = 0 + + queryClient.setQueryData(key, 'initial') + const queryFn = vi.fn().mockImplementation(async () => { + await sleep(100) + return 'data' + x + }) + + const promise = queryClient.fetchQuery({ + queryKey: key, + queryFn, + }) + + await vi.advanceTimersByTimeAsync(10) + + expect(queryFn).toHaveBeenCalledTimes(1) + + x = 1 + + // cancel ongoing re-fetches + void queryClient.refetchQueries({ queryKey: key }, { cancelRefetch: true }) + + await vi.advanceTimersByTimeAsync(10) + + // The promise should not reject + await vi.waitFor(() => expect(promise).resolves.toBe('data1')) + + expect(queryFn).toHaveBeenCalledTimes(2) + }) + it('should have an error log when queryFn data is not serializable', async () => { const consoleMock = vi.spyOn(console, 'error') diff --git a/packages/query-core/src/__tests__/queryClient.test.tsx b/packages/query-core/src/__tests__/queryClient.test.tsx index b6a5bfa81e..de3572ab3f 100644 --- a/packages/query-core/src/__tests__/queryClient.test.tsx +++ b/packages/query-core/src/__tests__/queryClient.test.tsx @@ -1050,7 +1050,7 @@ describe('queryClient', () => { queryKey: key1, queryFn: () => 'data', }) - queryClient.fetchQuery({ + queryClient.prefetchQuery({ queryKey: key1, queryFn: () => sleep(1000).then(() => 'data2'), }) diff --git a/packages/query-core/src/index.ts b/packages/query-core/src/index.ts index 889c8b02b7..f50cfb1a7a 100644 --- a/packages/query-core/src/index.ts +++ b/packages/query-core/src/index.ts @@ -26,7 +26,6 @@ export { shouldThrowError, } from './utils' export type { MutationFilters, QueryFilters, Updater, SkipToken } from './utils' -export { isCancelledError } from './retryer' export { dehydrate, hydrate, diff --git a/packages/query-core/src/query.ts b/packages/query-core/src/query.ts index 0987564694..623713a106 100644 --- a/packages/query-core/src/query.ts +++ b/packages/query-core/src/query.ts @@ -8,7 +8,7 @@ import { timeUntilStale, } from './utils' import { notifyManager } from './notifyManager' -import { canFetch, createRetryer, isCancelledError } from './retryer' +import { CancelledError, canFetch, createRetryer } from './retryer' import { Removable } from './removable' import type { QueryCache } from './queryCache' import type { QueryClient } from './queryClient' @@ -168,7 +168,6 @@ export class Query< state: QueryState #initialState: QueryState - #revertState?: QueryState #cache: QueryCache #client: QueryClient #retryer?: Retryer @@ -372,7 +371,7 @@ export class Query< } } - fetch( + async fetch( options?: QueryOptions, fetchOptions?: FetchOptions, ): Promise { @@ -485,7 +484,7 @@ export class Query< this.options.behavior?.onFetch(context, this as unknown as Query) // Store state in case the current fetch needs to be reverted - this.#revertState = this.state + const revertState = this.state // Set to fetching state if not already in it if ( @@ -495,32 +494,6 @@ export class Query< this.#dispatch({ type: 'fetch', meta: context.fetchOptions?.meta }) } - const onError = (error: TError | { silent?: boolean }) => { - // Optimistically update state if needed - if (!(isCancelledError(error) && error.silent)) { - this.#dispatch({ - type: 'error', - error: error as TError, - }) - } - - if (!isCancelledError(error)) { - // Notify cache callback - this.#cache.config.onError?.( - error as any, - this as Query, - ) - this.#cache.config.onSettled?.( - this.state.data, - error as any, - this as Query, - ) - } - - // Schedule query gc after fetching - this.scheduleGc() - } - // Try to fetch the data this.#retryer = createRetryer({ initialPromise: fetchOptions?.initialPromise as @@ -528,36 +501,6 @@ export class Query< | undefined, fn: context.fetchFn as () => Promise, abort: abortController.abort.bind(abortController), - onSuccess: (data) => { - if (data === undefined) { - if (process.env.NODE_ENV !== 'production') { - console.error( - `Query data cannot be undefined. Please make sure to return a value other than undefined from your query function. Affected query key: ${this.queryHash}`, - ) - } - onError(new Error(`${this.queryHash} data is undefined`) as any) - return - } - - try { - this.setData(data) - } catch (error) { - onError(error as TError) - return - } - - // Notify cache callback - this.#cache.config.onSuccess?.(data, this as Query) - this.#cache.config.onSettled?.( - data, - this.state.error as any, - this as Query, - ) - - // Schedule query gc after fetching - this.scheduleGc() - }, - onError, onFail: (failureCount, error) => { this.#dispatch({ type: 'failed', failureCount, error }) }, @@ -573,7 +516,65 @@ export class Query< canRun: () => true, }) - return this.#retryer.start() + try { + const data = await this.#retryer.start() + // this is more of a runtime guard + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition + if (data === undefined) { + if (process.env.NODE_ENV !== 'production') { + console.error( + `Query data cannot be undefined. Please make sure to return a value other than undefined from your query function. Affected query key: ${this.queryHash}`, + ) + } + throw new Error(`${this.queryHash} data is undefined`) + } + + this.setData(data) + + // Notify cache callback + this.#cache.config.onSuccess?.(data, this as Query) + this.#cache.config.onSettled?.( + data, + this.state.error as any, + this as Query, + ) + return data + } catch (error) { + if (error instanceof CancelledError) { + if (error.silent) { + // silent cancellation implies a new fetch is going to be started, + // so we hatch onto that promise + return this.#retryer.promise + } else if (error.revert) { + this.setState({ + ...revertState, + fetchStatus: 'idle' as const, + }) + // transform error into reverted state data + return this.state.data! + } + } + this.#dispatch({ + type: 'error', + error: error as TError, + }) + + // Notify cache callback + this.#cache.config.onError?.( + error as any, + this as Query, + ) + this.#cache.config.onSettled?.( + this.state.data, + error as any, + this as Query, + ) + + throw error // rethrow the error for further handling + } finally { + // Schedule query gc after fetching + this.scheduleGc() + } } #dispatch(action: Action): void { @@ -604,8 +605,6 @@ export class Query< fetchMeta: action.meta ?? null, } case 'success': - // If fetching ends successfully, we don't need revertState as a fallback anymore. - this.#revertState = undefined return { ...state, data: action.data, @@ -622,11 +621,6 @@ export class Query< } case 'error': const error = action.error - - if (isCancelledError(error) && error.revert && this.#revertState) { - return { ...this.#revertState, fetchStatus: 'idle' } - } - return { ...state, error, diff --git a/packages/query-core/src/retryer.ts b/packages/query-core/src/retryer.ts index baa93aa5b4..54a1dc3311 100644 --- a/packages/query-core/src/retryer.ts +++ b/packages/query-core/src/retryer.ts @@ -10,8 +10,6 @@ interface RetryerConfig { fn: () => TData | Promise initialPromise?: Promise abort?: () => void - onError?: (error: TError) => void - onSuccess?: (data: TData) => void onFail?: (failureCount: number, error: TError) => void onPause?: () => void onContinue?: () => void @@ -65,10 +63,6 @@ export class CancelledError extends Error { } } -export function isCancelledError(value: any): value is CancelledError { - return value instanceof CancelledError -} - export function createRetryer( config: RetryerConfig, ): Retryer { @@ -104,7 +98,6 @@ export function createRetryer( const resolve = (value: any) => { if (!isResolved) { isResolved = true - config.onSuccess?.(value) continueFn?.() thenable.resolve(value) } @@ -113,7 +106,6 @@ export function createRetryer( const reject = (value: any) => { if (!isResolved) { isResolved = true - config.onError?.(value) continueFn?.() thenable.reject(value) } diff --git a/packages/react-query/src/__tests__/useQuery.test.tsx b/packages/react-query/src/__tests__/useQuery.test.tsx index 893abb03b6..507f1a27e8 100644 --- a/packages/react-query/src/__tests__/useQuery.test.tsx +++ b/packages/react-query/src/__tests__/useQuery.test.tsx @@ -2992,7 +2992,7 @@ describe('useQuery', () => { }, retry: 2, - retryDelay: 100, + retryDelay: 200, }) return ( @@ -3036,7 +3036,8 @@ describe('useQuery', () => { expect(rendered.getByText('error: some error')).toBeInTheDocument(), ) - expect(count).toBe(4) + // 1 original fetch + 2 retries + expect(count).toBe(3) }) it('should restart when observers unmount and remount while waiting for a retry when query was cancelled in between (#3031)', async () => {