diff --git a/.changeset/silly-tires-brake.md b/.changeset/silly-tires-brake.md new file mode 100644 index 00000000000..2364d00177d --- /dev/null +++ b/.changeset/silly-tires-brake.md @@ -0,0 +1,7 @@ +--- +'@tanstack/react-query': patch +'@tanstack/preact-query': patch +'@tanstack/query-core': patch +--- + +fix(react): make sure suspense queries can resolve if data gets into the cache in other ways by tracking a promise from the query directly instead of tying it to the fetch that triggered it. diff --git a/packages/preact-query/src/__tests__/useSuspenseQuery.test.tsx b/packages/preact-query/src/__tests__/useSuspenseQuery.test.tsx index d09cc6b8032..777283be5b6 100644 --- a/packages/preact-query/src/__tests__/useSuspenseQuery.test.tsx +++ b/packages/preact-query/src/__tests__/useSuspenseQuery.test.tsx @@ -176,6 +176,45 @@ describe('useSuspenseQuery', () => { expect(queryFn).toHaveBeenCalledTimes(1) }) + it('should resolve suspense with cached data when data is set while fetching', async () => { + const key = queryKey() + + function Page() { + const state = useSuspenseQuery({ + queryKey: key, + queryFn: () => sleep(100).then(() => 'fetched'), + }) + + return
data: {state.data}
+ } + + const rendered = renderWithClient( + queryClient, + + + , + ) + + expect(rendered.getByText('loading')).toBeInTheDocument() + + await act(async () => { + await vi.advanceTimersByTimeAsync(10) + }) + await act(async () => { + queryClient.setQueryData(key, 'cached') + }) + await act(async () => { + await vi.advanceTimersByTimeAsync(0) + }) + + expect(rendered.getByText('data: cached')).toBeInTheDocument() + + await act(async () => { + await vi.advanceTimersByTimeAsync(100) + }) + expect(rendered.getByText('data: fetched')).toBeInTheDocument() + }) + it('should remove query instance when component unmounted', async () => { const key = queryKey() @@ -602,12 +641,12 @@ describe('useSuspenseQuery', () => { .spyOn(console, 'error') .mockImplementation(() => undefined) const key = queryKey() + let succeed = true - function Page({ succeed }: { succeed: boolean }) { + function Page() { const [nonce] = useState(0) - const queryKeys = [`${key}-${succeed}`] const result = useSuspenseQuery({ - queryKey: queryKeys, + queryKey: key, queryFn: () => sleep(10).then(() => { if (!succeed) throw new Error('Suspense Error Bingo') @@ -624,16 +663,9 @@ describe('useSuspenseQuery', () => { function App() { const { reset } = useQueryErrorResetBoundary() - const [succeed, setSucceed] = useState(true) return (
-
@@ -657,16 +689,22 @@ describe('useSuspenseQuery', () => { // render suspense fallback (loading) expect(rendered.getByText('loading')).toBeInTheDocument() // resolve promise -> render Page (rendered) - await vi.advanceTimersByTimeAsync(10) + await act(async () => { + await vi.advanceTimersByTimeAsync(10) + }) expect(rendered.getByText('rendered')).toBeInTheDocument() - // change query result to error by updating state above Suspense - fireEvent.click(rendered.getByLabelText('set-fail')) + // change query result to error + succeed = false // reset query -> and throw error - fireEvent.click(rendered.getByLabelText('fail')) + await act(async () => { + fireEvent.click(rendered.getByLabelText('fail')) + }) // render error boundary fallback (error boundary) - await vi.advanceTimersByTimeAsync(10) + await act(async () => { + await vi.advanceTimersByTimeAsync(10) + }) expect(rendered.getByText('error boundary')).toBeInTheDocument() expect(consoleMock.mock.calls[0]?.[1]).toStrictEqual( diff --git a/packages/preact-query/src/errorBoundaryUtils.ts b/packages/preact-query/src/errorBoundaryUtils.ts index 0b3d561fea6..86fbccd29f1 100644 --- a/packages/preact-query/src/errorBoundaryUtils.ts +++ b/packages/preact-query/src/errorBoundaryUtils.ts @@ -25,11 +25,17 @@ export const ensurePreventErrorBoundaryRetry = < TQueryKey >, errorResetBoundary: QueryErrorResetBoundaryValue, + query: Query | undefined, ) => { + const throwOnError = + query?.state.error && typeof options.throwOnError === 'function' + ? shouldThrowError(options.throwOnError, [query.state.error, query]) + : options.throwOnError + if ( options.suspense || - options.throwOnError || - options.experimental_prefetchInRender + options.experimental_prefetchInRender || + throwOnError ) { // Prevent retrying failed query if the error boundary has not been reset yet if (!errorResetBoundary.isReset()) { diff --git a/packages/preact-query/src/useBaseQuery.ts b/packages/preact-query/src/useBaseQuery.ts index 05ce46e7f6e..ec820b7586b 100644 --- a/packages/preact-query/src/useBaseQuery.ts +++ b/packages/preact-query/src/useBaseQuery.ts @@ -58,6 +58,15 @@ export function useBaseQuery< defaultedOptions, ) + const query = client + .getQueryCache() + .get< + TQueryFnData, + TError, + TQueryData, + TQueryKey + >(defaultedOptions.queryHash) + if (process.env.NODE_ENV !== 'production') { if (!defaultedOptions.queryFn) { console.error( @@ -72,7 +81,7 @@ export function useBaseQuery< : 'optimistic' ensureSuspenseTimers(defaultedOptions) - ensurePreventErrorBoundaryRetry(defaultedOptions, errorResetBoundary) + ensurePreventErrorBoundaryRetry(defaultedOptions, errorResetBoundary, query) useClearResetErrorBoundary(errorResetBoundary) @@ -126,14 +135,7 @@ export function useBaseQuery< result, errorResetBoundary, throwOnError: defaultedOptions.throwOnError, - query: client - .getQueryCache() - .get< - TQueryFnData, - TError, - TQueryData, - TQueryKey - >(defaultedOptions.queryHash), + query, suspense: defaultedOptions.suspense, }) ) { @@ -154,7 +156,7 @@ export function useBaseQuery< ? // Fetch immediately on render in order to ensure `.promise` is resolved even if the component is unmounted fetchOptimistic(defaultedOptions, observer, errorResetBoundary) : // subscribe to the "cache promise" so that we can finalize the currentThenable once data comes in - client.getQueryCache().get(defaultedOptions.queryHash)?.promise + query?.promise promise?.catch(noop).finally(() => { // `.updateResult()` will trigger `.#currentThenable` to finalize diff --git a/packages/preact-query/src/useQueries.ts b/packages/preact-query/src/useQueries.ts index be57d2aebd6..acbf8d41ed2 100644 --- a/packages/preact-query/src/useQueries.ts +++ b/packages/preact-query/src/useQueries.ts @@ -241,9 +241,10 @@ export function useQueries< [queries, client, isRestoring], ) - defaultedQueries.forEach((query) => { - ensureSuspenseTimers(query) - ensurePreventErrorBoundaryRetry(query, errorResetBoundary) + defaultedQueries.forEach((queryOptions) => { + ensureSuspenseTimers(queryOptions) + const query = client.getQueryCache().get(queryOptions.queryHash) + ensurePreventErrorBoundaryRetry(queryOptions, errorResetBoundary, query) }) useClearResetErrorBoundary(errorResetBoundary) diff --git a/packages/query-core/src/__tests__/query.test.tsx b/packages/query-core/src/__tests__/query.test.tsx index f097a11ef7d..7aaa4ebf7da 100644 --- a/packages/query-core/src/__tests__/query.test.tsx +++ b/packages/query-core/src/__tests__/query.test.tsx @@ -58,6 +58,37 @@ describe('query', () => { expect(query.gcTime).toBe(200) }) + it('should resolve the query promise when data is set while fetching', async () => { + const key = queryKey() + let fetchResult: unknown + queryClient + .fetchQuery({ + queryKey: key, + queryFn: () => sleep(100).then(() => 'fetched'), + }) + .then((data) => { + fetchResult = data + }) + + const query = queryCache.find({ queryKey: key })! + let queryPromiseResult: unknown + query.promise.then((data) => { + queryPromiseResult = data + }) + + await vi.advanceTimersByTimeAsync(10) + queryClient.setQueryData(key, 'cached') + await vi.advanceTimersByTimeAsync(0) + + expect(queryPromiseResult).toBe('cached') + expect(fetchResult).toBeUndefined() + expect(query.state.fetchStatus).toBe('fetching') + + await vi.advanceTimersByTimeAsync(100) + await expect(query.promise).resolves.toBe('fetched') + expect(fetchResult).toBe('fetched') + }) + it('should continue retry after focus regain and resolve all promises', async () => { const key = queryKey() diff --git a/packages/query-core/src/hydration.ts b/packages/query-core/src/hydration.ts index 976b5faafee..54731b2556b 100644 --- a/packages/query-core/src/hydration.ts +++ b/packages/query-core/src/hydration.ts @@ -81,7 +81,7 @@ function dehydrateQuery( shouldRedactErrors: (error: unknown) => boolean, ): DehydratedQuery { const dehydratePromise = () => { - const promise = query.promise?.then(serializeData).catch((error) => { + const promise = query.promise.then(serializeData).catch((error) => { if (!shouldRedactErrors(error)) { // Reject original error if it should not be redacted return Promise.reject(error) @@ -99,7 +99,7 @@ function dehydrateQuery( // We need the promise we dehydrate to reject to get the correct result into // the query cache, but we also want to avoid unhandled promise rejections // in whatever environment the prefetches are happening in. - promise?.catch(noop) + promise.catch(noop) return promise } diff --git a/packages/query-core/src/query.ts b/packages/query-core/src/query.ts index 62bc9a16082..e44622b4ebb 100644 --- a/packages/query-core/src/query.ts +++ b/packages/query-core/src/query.ts @@ -11,6 +11,7 @@ import { notifyManager } from './notifyManager' import { CancelledError, canFetch, createRetryer } from './retryer' import { Removable } from './removable' import { infiniteQueryBehavior } from './infiniteQueryBehavior' +import { pendingThenable, updateThenable } from './thenable' import type { QueryCache } from './queryCache' import type { QueryClient } from './queryClient' import type { @@ -29,6 +30,7 @@ import type { } from './types' import type { QueryObserver } from './queryObserver' import type { Retryer } from './retryer' +import type { Thenable } from './thenable' // TYPES @@ -169,6 +171,7 @@ export class Query< #cache: QueryCache #client: QueryClient #retryer?: Retryer + #promise: Thenable observers: Array> #defaultOptions?: QueryOptions #abortSignalConsumed: boolean @@ -186,6 +189,8 @@ export class Query< this.queryHash = config.queryHash this.#initialState = getDefaultState(this.options) this.state = config.state ?? this.#initialState + this.#promise = pendingThenable() + this.#updatePromise() this.scheduleGc() } get meta(): QueryMeta | undefined { @@ -196,8 +201,12 @@ export class Query< return this.#queryType } - get promise(): Promise | undefined { - return this.#retryer?.promise + get promise(): Promise { + return this.#promise + } + + #updatePromise(): void { + this.#promise = updateThenable(this.#promise, this.state) } setOptions( @@ -696,6 +705,7 @@ export class Query< } this.state = reducer(this.state) + this.#updatePromise() notifyManager.batch(() => { this.observers.forEach((observer) => { diff --git a/packages/query-core/src/queryObserver.ts b/packages/query-core/src/queryObserver.ts index 954c969d548..8ccfaacbd08 100644 --- a/packages/query-core/src/queryObserver.ts +++ b/packages/query-core/src/queryObserver.ts @@ -3,7 +3,7 @@ import { environmentManager } from './environmentManager' import { notifyManager } from './notifyManager' import { fetchState } from './query' import { Subscribable } from './subscribable' -import { pendingThenable } from './thenable' +import { pendingThenable, updateThenable } from './thenable' import { isValidTimeout, noop, @@ -17,7 +17,7 @@ import { timeoutManager } from './timeoutManager' import type { ManagedTimerId } from './timeoutManager' import type { FetchOptions, Query, QueryState } from './query' import type { QueryClient } from './queryClient' -import type { PendingThenable, Thenable } from './thenable' +import type { Thenable } from './thenable' import type { DefaultError, DefaultedQueryObserverOptions, @@ -317,7 +317,9 @@ export class QueryObserver< .getQueryCache() .build(this.#client, defaultedOptions) - return query.fetch().then(() => this.createResult(query, defaultedOptions)) + query.fetch().catch(noop) + + return query.promise.then(() => this.createResult(query, defaultedOptions)) } protected fetch( @@ -595,48 +597,11 @@ export class QueryObserver< const nextResult = result as QueryObserverResult if (this.options.experimental_prefetchInRender) { - const hasResultData = nextResult.data !== undefined - const isErrorWithoutData = nextResult.status === 'error' && !hasResultData - const finalizeThenableIfPossible = (thenable: PendingThenable) => { - if (isErrorWithoutData) { - thenable.reject(nextResult.error) - } else if (hasResultData) { - thenable.resolve(nextResult.data as TData) - } - } - - /** - * Create a new thenable and result promise when the results have changed - */ - const recreateThenable = () => { - const pending = - (this.#currentThenable = - nextResult.promise = - pendingThenable()) - - finalizeThenableIfPossible(pending) - } - - const prevThenable = this.#currentThenable - switch (prevThenable.status) { - case 'pending': - // Finalize the previous thenable if it was pending - // and we are still observing the same query - if (query.queryHash === prevQuery.queryHash) { - finalizeThenableIfPossible(prevThenable) - } - break - case 'fulfilled': - if (isErrorWithoutData || nextResult.data !== prevThenable.value) { - recreateThenable() - } - break - case 'rejected': - if (!isErrorWithoutData || nextResult.error !== prevThenable.reason) { - recreateThenable() - } - break - } + this.#currentThenable = nextResult.promise = updateThenable( + this.#currentThenable, + nextResult, + query.queryHash === prevQuery.queryHash, + ) } return nextResult diff --git a/packages/query-core/src/thenable.ts b/packages/query-core/src/thenable.ts index 4b9a1ddaa17..3f105fa4645 100644 --- a/packages/query-core/src/thenable.ts +++ b/packages/query-core/src/thenable.ts @@ -83,6 +83,47 @@ export function pendingThenable(): PendingThenable { return thenable } +export function updateThenable( + thenable: Thenable, + result: { data: T | undefined; error: unknown; status: 'error' | string }, + finalizePending = true, +): Thenable { + const hasData = result.data !== undefined + const isErrorWithoutData = result.status === 'error' && !hasData + + const finalizeThenableIfPossible = (pending: PendingThenable) => { + if (isErrorWithoutData) { + pending.reject(result.error) + } else if (hasData) { + pending.resolve(result.data as T) + } + } + + const recreateThenable = () => { + const pending = pendingThenable() + finalizeThenableIfPossible(pending) + return pending + } + + switch (thenable.status) { + case 'pending': + if (finalizePending) { + finalizeThenableIfPossible(thenable) + } + return thenable + case 'fulfilled': + if (isErrorWithoutData || result.data !== thenable.value) { + return recreateThenable() + } + return thenable + case 'rejected': + if (!isErrorWithoutData || result.error !== thenable.reason) { + return recreateThenable() + } + return thenable + } +} + /** * This function takes a Promise-like input and detects whether the data * is synchronously available or not. diff --git a/packages/react-query/src/__tests__/useSuspenseQuery.test.tsx b/packages/react-query/src/__tests__/useSuspenseQuery.test.tsx index 10583797f3d..d0019967bf4 100644 --- a/packages/react-query/src/__tests__/useSuspenseQuery.test.tsx +++ b/packages/react-query/src/__tests__/useSuspenseQuery.test.tsx @@ -162,6 +162,38 @@ describe('useSuspenseQuery', () => { expect(queryFn).toHaveBeenCalledTimes(1) }) + it('should resolve suspense with cached data when data is set while fetching', async () => { + const key = queryKey() + + function Page() { + const state = useSuspenseQuery({ + queryKey: key, + queryFn: () => sleep(100).then(() => 'fetched'), + }) + + return
data: {state.data}
+ } + + const rendered = renderWithClient( + queryClient, + + + , + ) + + expect(rendered.getByText('loading')).toBeInTheDocument() + + await act(() => vi.advanceTimersByTimeAsync(10)) + await act(async () => { + queryClient.setQueryData(key, 'cached') + }) + + expect(rendered.getByText('data: cached')).toBeInTheDocument() + + await act(() => vi.advanceTimersByTimeAsync(100)) + expect(rendered.getByText('data: fetched')).toBeInTheDocument() + }) + it('should remove query instance when component unmounted', async () => { const key = queryKey()