Skip to content
Open
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
7 changes: 7 additions & 0 deletions .changeset/silly-tires-brake.md
Original file line number Diff line number Diff line change
@@ -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.
70 changes: 54 additions & 16 deletions packages/preact-query/src/__tests__/useSuspenseQuery.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 <div>data: {state.data}</div>
}

const rendered = renderWithClient(
queryClient,
<Suspense fallback="loading">
<Page />
</Suspense>,
)

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()

Expand Down Expand Up @@ -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')
Expand All @@ -624,16 +663,9 @@ describe('useSuspenseQuery', () => {

function App() {
const { reset } = useQueryErrorResetBoundary()
const [succeed, setSucceed] = useState(true)

return (
<div>
<button
aria-label="set-fail"
onClick={() => {
setSucceed(false)
}}
/>
<button
aria-label="fail"
onClick={() => {
Expand All @@ -645,7 +677,7 @@ describe('useSuspenseQuery', () => {
fallbackRender={() => <div>error boundary</div>}
>
<Suspense fallback="loading">
<Page succeed={succeed} />
<Page />
</Suspense>
</ErrorBoundary>
</div>
Expand All @@ -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(
Expand Down
10 changes: 8 additions & 2 deletions packages/preact-query/src/errorBoundaryUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,17 @@ export const ensurePreventErrorBoundaryRetry = <
TQueryKey
>,
errorResetBoundary: QueryErrorResetBoundaryValue,
query: Query<TQueryFnData, TError, TQueryData, TQueryKey> | 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()) {
Expand Down
22 changes: 12 additions & 10 deletions packages/preact-query/src/useBaseQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -72,7 +81,7 @@ export function useBaseQuery<
: 'optimistic'

ensureSuspenseTimers(defaultedOptions)
ensurePreventErrorBoundaryRetry(defaultedOptions, errorResetBoundary)
ensurePreventErrorBoundaryRetry(defaultedOptions, errorResetBoundary, query)

useClearResetErrorBoundary(errorResetBoundary)

Expand Down Expand Up @@ -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,
})
) {
Expand All @@ -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
Expand Down
7 changes: 4 additions & 3 deletions packages/preact-query/src/useQueries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
31 changes: 31 additions & 0 deletions packages/query-core/src/__tests__/query.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
4 changes: 2 additions & 2 deletions packages/query-core/src/hydration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
}
Expand Down
14 changes: 12 additions & 2 deletions packages/query-core/src/query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -29,6 +30,7 @@ import type {
} from './types'
import type { QueryObserver } from './queryObserver'
import type { Retryer } from './retryer'
import type { Thenable } from './thenable'

// TYPES

Expand Down Expand Up @@ -169,6 +171,7 @@ export class Query<
#cache: QueryCache
#client: QueryClient
#retryer?: Retryer<TData>
#promise: Thenable<TData>
observers: Array<QueryObserver<any, any, any, any, any>>
#defaultOptions?: QueryOptions<TQueryFnData, TError, TData, TQueryKey>
#abortSignalConsumed: boolean
Expand All @@ -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 {
Expand All @@ -196,8 +201,12 @@ export class Query<
return this.#queryType
}

get promise(): Promise<TData> | undefined {
return this.#retryer?.promise
get promise(): Promise<TData> {
return this.#promise
}

#updatePromise(): void {
this.#promise = updateThenable(this.#promise, this.state)
}

setOptions(
Expand Down Expand Up @@ -696,6 +705,7 @@ export class Query<
}

this.state = reducer(this.state)
this.#updatePromise()

notifyManager.batch(() => {
this.observers.forEach((observer) => {
Expand Down
Loading
Loading