fix(hydration): set dataUpdatedAt when pending query resolves before hydration#10610
fix(hydration): set dataUpdatedAt when pending query resolves before hydration#10610DORI2001 wants to merge 3 commits intoTanStack:mainfrom
Conversation
📝 WalkthroughWalkthroughWhen hydrating queries that were ChangesHydration timestamp correction
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/query-core/src/hydration.ts (1)
233-257:⚠️ Potential issue | 🟠 MajorOlder payloads can still skip pending→success upgrade in existing-query hydration.
When
dehydratedAtis missing (older payload),hasNewerSyncDatais always false (Line 237), andstate.dataUpdatedAtis typically0for pending dehydrated state. That can keep theifon Line 239 false, so the transition at Line 253 and the timestamp fix at Line 256 never run for existing pending queries.Suggested fix
- const hasNewerSyncData = - syncData && - // We only need this undefined check to handle older dehydration - // payloads that might not have dehydratedAt - dehydratedAt !== undefined && - dehydratedAt > query.state.dataUpdatedAt + const hasNewerSyncData = + syncData && + (dehydratedAt !== undefined + ? dehydratedAt > query.state.dataUpdatedAt + : // Older payloads: allow only pending->success upgrade for existing pending-without-data + state.status === 'pending' && + query.state.status === 'pending' && + query.state.data === undefined)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/query-core/src/hydration.ts` around lines 233 - 257, The hydration logic currently skips the pending→success upgrade when dehydratedAt is missing; update the hasNewerSyncData computation to treat an undefined dehydratedAt as “newer” for pending dehydrated states (e.g., set hasNewerSyncData = syncData && (dehydratedAt !== undefined ? dehydratedAt > query.state.dataUpdatedAt : state.status === 'pending')). Then ensure when you call query.setState (same block around query.setState and the pending→success branch) you still use dataUpdatedAt: dehydratedAt ?? Date.now() so the timestamp correction runs even if dehydratedAt was missing. This references the variables dehydratedAt, syncData, state.status, state.dataUpdatedAt and the call to query.setState.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/query-core/src/hydration.ts`:
- Around line 233-257: The hydration logic currently skips the pending→success
upgrade when dehydratedAt is missing; update the hasNewerSyncData computation to
treat an undefined dehydratedAt as “newer” for pending dehydrated states (e.g.,
set hasNewerSyncData = syncData && (dehydratedAt !== undefined ? dehydratedAt >
query.state.dataUpdatedAt : state.status === 'pending')). Then ensure when you
call query.setState (same block around query.setState and the pending→success
branch) you still use dataUpdatedAt: dehydratedAt ?? Date.now() so the timestamp
correction runs even if dehydratedAt was missing. This references the variables
dehydratedAt, syncData, state.status, state.dataUpdatedAt and the call to
query.setState.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 16c83652-7389-4dc8-9f6d-243d04125a88
📒 Files selected for processing (1)
packages/query-core/src/hydration.ts
|
please add a test case that covers this scenario - something that fails on |
…lves before hydration
|
Added two test cases to |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/query-core/src/__tests__/hydration.test.tsx (1)
1808-1848: 💤 Low valueLGTM — new cache-entry test correctly validates the fix.
The synchronous-thenable setup faithfully models a React streaming promise that resolved before
hydrate(), andtoBeGreaterThan(0)will correctly fail onmain(wheredataUpdatedAtstays0) and pass with the fix applied.One minor asymmetry with Test 2: adding a pre-hydration
dataUpdatedAt === 0assertion would make it explicit that the value was0before hydration and becomes> 0after — mirroring the explicit before/after pattern in the existing-cache-entry test.🔍 Optional symmetry improvement
const clientQueryClient = new QueryClient() + const query = clientQueryClient.getQueryCache().find({ queryKey: key }) hydrate(clientQueryClient, dehydrated) - const query = clientQueryClient.getQueryCache().find({ queryKey: key })! + // query is only created during hydrate, so the pre-hydration reference + // must be obtained lazily; alternatively assert post-hoc, as below.Or, more simply, assert
dehydrated.queries[0]?.state.dataUpdatedAtequals0right afterdehydrate(), paralleling what Test 2 does withexpect(query.state.dataUpdatedAt).toBe(0)at line 1882.🤖 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/__tests__/hydration.test.tsx` around lines 1808 - 1848, Add an explicit pre-hydration assertion that the dehydrated query's dataUpdatedAt is 0: after calling dehydrate(serverQueryClient) assert dehydrated.queries[0]?.state.dataUpdatedAt === 0 (or toBe(0)) before calling hydrate(clientQueryClient, dehydrated) so the test mirrors the existing-cache-entry test and demonstrates the value changes to > 0 after hydrate; locate the check around the existing references to dehydrate(), dehydrated, and hydrate() in the test "should set dataUpdatedAt when hydrating a resolved streamed query into a new cache entry".
🤖 Prompt for all review comments with 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.
Nitpick comments:
In `@packages/query-core/src/__tests__/hydration.test.tsx`:
- Around line 1808-1848: Add an explicit pre-hydration assertion that the
dehydrated query's dataUpdatedAt is 0: after calling
dehydrate(serverQueryClient) assert dehydrated.queries[0]?.state.dataUpdatedAt
=== 0 (or toBe(0)) before calling hydrate(clientQueryClient, dehydrated) so the
test mirrors the existing-cache-entry test and demonstrates the value changes to
> 0 after hydrate; locate the check around the existing references to
dehydrate(), dehydrated, and hydrate() in the test "should set dataUpdatedAt
when hydrating a resolved streamed query into a new cache entry".
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e93e9ba7-bd3b-4820-b86b-1ee978a7dc5e
📒 Files selected for processing (1)
packages/query-core/src/__tests__/hydration.test.tsx
2 participants
Problem
When a query is dehydrated while still pending (streamed) but then resolves before hydration runs on the client, the hydration code transitions the query's status from
'pending'to'success'— but it never updatesdataUpdatedAt. It stays at0.This was introduced in #10444, which removed the
query.fetch()call that previously setdataUpdatedAtas a side effect of resolving the promise.Steps to reproduce: dehydrate a pending query (e.g. from a React Server Component), resolve it before sending the response, then hydrate on the client.
query.state.dataUpdatedAtwill be0instead of a real timestamp.Fix
When the pending→success transition is applied (both in the
setStatepath for existing queries and in thequeryCache.buildpath for new queries), adddataUpdatedAt: dehydratedAt ?? Date.now().dehydratedAtis the right value here — it records when the server serialized the resolved data, which is when the data was actually "updated". Falling back toDate.now()handles older dehydration payloads that don't includedehydratedAt.Fixes #10603
Summary by CodeRabbit
Bug Fixes
Tests