Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
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
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
55 changes: 10 additions & 45 deletions packages/query-core/src/queryObserver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -595,48 +597,11 @@ export class QueryObserver<
const nextResult = result as QueryObserverResult<TData, TError>

if (this.options.experimental_prefetchInRender) {
const hasResultData = nextResult.data !== undefined
const isErrorWithoutData = nextResult.status === 'error' && !hasResultData
const finalizeThenableIfPossible = (thenable: PendingThenable<TData>) => {
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
Expand Down
41 changes: 41 additions & 0 deletions packages/query-core/src/thenable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,47 @@ export function pendingThenable<T>(): PendingThenable<T> {
return thenable
}

export function updateThenable<T>(
thenable: Thenable<T>,
result: { data: T | undefined; error: unknown; status: 'error' | string },
finalizePending = true,
): Thenable<T> {
const hasData = result.data !== undefined
const isErrorWithoutData = result.status === 'error' && !hasData

const finalizeThenableIfPossible = (pending: PendingThenable<T>) => {
if (isErrorWithoutData) {
pending.reject(result.error)
} else if (hasData) {
pending.resolve(result.data as T)
}
}

const recreateThenable = () => {
const pending = pendingThenable<T>()
finalizeThenableIfPossible(pending)
return pending
}

switch (thenable.status) {
case 'pending':
if (finalizePending) {
finalizeThenableIfPossible(thenable)
}
return thenable
Comment on lines +108 to +114

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Recreate the thenable on query-hash switches.

When packages/query-core/src/queryObserver.ts passes false here for a query change, the 'pending' branch reuses the previous query's pending thenable. If the new query is already settled from cache, nextResult.promise stays pending and Suspense can hang indefinitely.

Suggested fix
   switch (thenable.status) {
     case 'pending':
-      if (finalizePending) {
-        finalizeThenableIfPossible(thenable)
-      }
-      return thenable
+      if (!finalizePending) {
+        return recreateThenable()
+      }
+      finalizeThenableIfPossible(thenable)
+      return thenable
     case 'fulfilled':
       if (isErrorWithoutData || result.data !== thenable.value) {
         return recreateThenable()
       }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
switch (thenable.status) {
case 'pending':
if (finalizePending) {
finalizeThenableIfPossible(thenable)
}
return thenable
switch (thenable.status) {
case 'pending':
if (!finalizePending) {
return recreateThenable()
}
finalizeThenableIfPossible(thenable)
return thenable
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/query-core/src/thenable.ts` around lines 108 - 113, The pending
branch in thenable handling is reusing an old promise across query-hash changes,
so update the logic in thenable.ts and the caller in queryObserver.ts to
recreate or refresh the thenable whenever the query identity changes instead of
returning the previous pending instance. Use the query-hash comparison in
QueryObserver and the finalizePending path in
finalizeThenableIfPossible/thenable.status handling to ensure a new settled
query does not inherit a stale pending promise.

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.
Expand Down
32 changes: 32 additions & 0 deletions packages/react-query/src/__tests__/useSuspenseQuery.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 <div>data: {state.data}</div>
}

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

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

Expand Down
Loading