Skip to content
Open
Show file tree
Hide file tree
Changes from 5 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/peact-query': patch
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
Comment thread
TkDodo marked this conversation as resolved.
Outdated
'@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