Skip to content

Commit 13c3232

Browse files
committed
fix(hydration): fix promise hydration bugs
* Include dehydratedAt and ignore hydrating old promises * This is an approximation of dataUpdatedAt, but is not perfect * Refactor checks for if we should hydrate promises or not to fix infinite loop bug on failed queries * Hydrate already resolved promises synchronously
1 parent 660ab07 commit 13c3232

File tree

3 files changed

+106
-57
lines changed

3 files changed

+106
-57
lines changed

packages/query-core/src/hydration.ts

Lines changed: 73 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { tryResolveSync } from './thenable'
12
import type {
23
DefaultError,
34
MutationKey,
@@ -46,6 +47,10 @@ interface DehydratedQuery {
4647
state: QueryState
4748
promise?: Promise<unknown>
4849
meta?: QueryMeta
50+
// This is only optional because older versions of Query might have dehydrated
51+
// without it which we need to handle for backwards compatibility.
52+
// This should be changed to required in the future.
53+
dehydratedAt?: number
4954
}
5055

5156
export interface DehydratedState {
@@ -74,6 +79,7 @@ function dehydrateQuery(
7479
shouldRedactErrors: (error: unknown) => boolean,
7580
): DehydratedQuery {
7681
return {
82+
dehydratedAt: Date.now(),
7783
state: {
7884
...query.state,
7985
...(query.state.data !== undefined && {
@@ -189,52 +195,73 @@ export function hydrate(
189195
)
190196
})
191197

192-
queries.forEach(({ queryKey, state, queryHash, meta, promise }) => {
193-
let query = queryCache.get(queryHash)
194-
195-
const data =
196-
state.data === undefined ? state.data : deserializeData(state.data)
197-
198-
// Do not hydrate if an existing query exists with newer data
199-
if (query) {
200-
if (query.state.dataUpdatedAt < state.dataUpdatedAt) {
201-
// omit fetchStatus from dehydrated state
202-
// so that query stays in its current fetchStatus
203-
const { fetchStatus: _ignored, ...serializedState } = state
204-
query.setState({
205-
...serializedState,
206-
data,
198+
queries.forEach(
199+
({ queryKey, state, queryHash, meta, promise, dehydratedAt }) => {
200+
const syncData = promise ? tryResolveSync(promise) : undefined
201+
const rawData = state.data === undefined ? syncData?.data : state.data
202+
const data = rawData === undefined ? rawData : deserializeData(rawData)
203+
204+
let query = queryCache.get(queryHash)
205+
const existingQueryIsPending = query?.state.status === 'pending'
206+
207+
// Do not hydrate if an existing query exists with newer data
208+
if (query) {
209+
const hasNewerSyncData =
210+
syncData &&
211+
// We only need this undefined check to handle older dehydration
212+
// payloads that might not have dehydratedAt
213+
dehydratedAt !== undefined &&
214+
dehydratedAt > query.state.dataUpdatedAt
215+
if (
216+
state.dataUpdatedAt > query.state.dataUpdatedAt ||
217+
hasNewerSyncData
218+
) {
219+
// omit fetchStatus from dehydrated state
220+
// so that query stays in its current fetchStatus
221+
const { fetchStatus: _ignored, ...serializedState } = state
222+
query.setState({
223+
...serializedState,
224+
data,
225+
})
226+
}
227+
} else {
228+
// Restore query
229+
query = queryCache.build(
230+
client,
231+
{
232+
...client.getDefaultOptions().hydrate?.queries,
233+
...options?.defaultOptions?.queries,
234+
queryKey,
235+
queryHash,
236+
meta,
237+
},
238+
// Reset fetch status to idle to avoid
239+
// query being stuck in fetching state upon hydration
240+
{
241+
...state,
242+
data,
243+
fetchStatus: 'idle',
244+
status: data !== undefined ? 'success' : state.status,
245+
},
246+
)
247+
}
248+
249+
if (
250+
promise &&
251+
!existingQueryIsPending &&
252+
// Only hydrate promise if no synchronous data was available to hydrate
253+
!data &&
254+
// Only hydrate if dehydration is newer than any existing data
255+
dehydratedAt !== undefined &&
256+
dehydratedAt > query.state.dataUpdatedAt
257+
) {
258+
// this doesn't actually fetch - it just creates a retryer
259+
// which will re-use the passed `initialPromise`
260+
void query.fetch(undefined, {
261+
// RSC transformed promises are not thenable
262+
initialPromise: Promise.resolve(promise).then(deserializeData),
207263
})
208264
}
209-
} else {
210-
// Restore query
211-
query = queryCache.build(
212-
client,
213-
{
214-
...client.getDefaultOptions().hydrate?.queries,
215-
...options?.defaultOptions?.queries,
216-
queryKey,
217-
queryHash,
218-
meta,
219-
},
220-
// Reset fetch status to idle to avoid
221-
// query being stuck in fetching state upon hydration
222-
{
223-
...state,
224-
data,
225-
fetchStatus: 'idle',
226-
},
227-
)
228-
}
229-
230-
if (promise) {
231-
// Note: `Promise.resolve` required cause
232-
// RSC transformed promises are not thenable
233-
const initialPromise = Promise.resolve(promise).then(deserializeData)
234-
235-
// this doesn't actually fetch - it just creates a retryer
236-
// which will re-use the passed `initialPromise`
237-
void query.fetch(undefined, { initialPromise })
238-
}
239-
})
265+
},
266+
)
240267
}

packages/query-core/src/thenable.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,3 +80,30 @@ export function pendingThenable<T>(): PendingThenable<T> {
8080

8181
return thenable
8282
}
83+
84+
/**
85+
* This function takes a Promise-like input and detects whether the data
86+
* is synchronously available or not.
87+
*
88+
* It does not inspect .status, .value or .reason properties of the promise,
89+
* as those are not always available, and the .status of React's promises
90+
* should not be considered part of the public API.
91+
*/
92+
export function tryResolveSync(promise: Promise<unknown> | Thenable<unknown>) {
93+
let data: unknown
94+
95+
promise
96+
.then((result) => {
97+
data = result
98+
return result
99+
})
100+
// This can be unavailable on certain kinds of thenable's
101+
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
102+
?.catch(() => {})
103+
104+
if (data !== undefined) {
105+
return { data }
106+
}
107+
108+
return undefined
109+
}

packages/react-query/src/HydrationBoundary.tsx

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,6 @@ export interface HydrationBoundaryProps {
2424
queryClient?: QueryClient
2525
}
2626

27-
const hasProperty = <TKey extends string>(
28-
obj: unknown,
29-
key: TKey,
30-
): obj is { [k in TKey]: unknown } => {
31-
return typeof obj === 'object' && obj !== null && key in obj
32-
}
33-
3427
export const HydrationBoundary = ({
3528
children,
3629
options = {},
@@ -80,10 +73,11 @@ export const HydrationBoundary = ({
8073
} else {
8174
const hydrationIsNewer =
8275
dehydratedQuery.state.dataUpdatedAt >
83-
existingQuery.state.dataUpdatedAt || // RSC special serialized then-able chunks
84-
(hasProperty(dehydratedQuery.promise, 'status') &&
85-
hasProperty(existingQuery.promise, 'status') &&
86-
dehydratedQuery.promise.status !== existingQuery.promise.status)
76+
existingQuery.state.dataUpdatedAt ||
77+
(dehydratedQuery.promise &&
78+
existingQuery.state.status !== 'pending' &&
79+
dehydratedQuery.dehydratedAt !== undefined &&
80+
dehydratedQuery.dehydratedAt > existingQuery.state.dataUpdatedAt)
8781

8882
const queryAlreadyQueued = hydrationQueue?.find(
8983
(query) => query.queryHash === dehydratedQuery.queryHash,
@@ -116,6 +110,7 @@ export const HydrationBoundary = ({
116110
React.useEffect(() => {
117111
if (hydrationQueue) {
118112
hydrate(client, { queries: hydrationQueue }, optionsRef.current)
113+
// eslint-disable-next-line @eslint-react/hooks-extra/no-direct-set-state-in-use-effect
119114
setHydrationQueue(undefined)
120115
}
121116
}, [client, hydrationQueue])

0 commit comments

Comments
 (0)