fix(react-query/HydrationBoundary): prevent unnecessary refetch during hydration #9968
Conversation
🦋 Changeset detectedLatest commit: bb9cced The changes in this PR will be included in the next version bump. This PR includes changesets to release 19 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📝 WalkthroughWalkthroughThis PR fixes double-fetching during hydration by introducing hydration-aware refetch prevention. When HydrationBoundary hydrates queries with fresh data, the system now tracks pending hydration status and skips unnecessary refetches on mount unless explicitly requested via Changes
Sequence DiagramsequenceDiagram
participant HB as HydrationBoundary
participant UBQ as useBaseQuery/<br/>useQueries
participant QO as QueryObserver
participant Cache as Query Cache
HB->>HB: Track dehydrated queries<br/>Mark existing queries in<br/>hydratingQueriesRef
HB->>HB: Wrap children with<br/>IsHydratingProvider
HB->>Cache: Hydrate new queries
UBQ->>UBQ: Check useIsHydrating()<br/>for queryHash
UBQ->>UBQ: Set _isHydrating=true<br/>if pending
UBQ->>QO: Create observer with<br/>_isHydrating flag
QO->>QO: onSubscribe: Compute<br/>shouldSkipFetchForHydration
QO->>QO: Check: _isHydrating &&<br/>!refetchOnMount='always' &&<br/>!isStale?
alt Data Fresh & Hydrating
QO->>QO: Skip fetch, use<br/>existing data
else Data Stale or Always
QO->>Cache: Perform refetch
end
HB->>HB: After hydration,<br/>clean hydratingQueriesRef
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Suggested Reviewers
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
Comment |
github-actions
bot
added
documentation
|
View your CI Pipeline Execution ↗ for commit bb9cced
☁️ Nx Cloud last updated this comment at |
sukvvon
force-pushed
the
fix/hydration-boundary-double-fetch
branch
from
86745a3 to
8626d42
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #9968 +/- ##
==========================================
+ Coverage 36.86% 42.83% +5.96%
==========================================
Files 245 173 -72
Lines 11102 8286 -2816
Branches 2079 1688 -391
==========================================
- Hits 4093 3549 -544
+ Misses 6484 4347 -2137
+ Partials 525 390 -135 🚀 New features to boost your workflow:
|
sukvvon
force-pushed
the
fix/hydration-boundary-double-fetch
branch
from
cfa5d74 to
695fa28
Compare
…tract 'clearPendingQueries' function
|
Thanks a lot for this. I hope it gets merged soon 🤞 |
|
|
||
| > Note: Only `queries` can be dehydrated with an `HydrationBoundary`. | ||
| > Note: `HydrationBoundary` automatically prevents unnecessary refetching during hydration. Queries being hydrated will not trigger a refetch on mount, unless `refetchOnMount` is explicitly set to `'always'`. |
There was a problem hiding this comment.
Thank you for this PR! I've only had time to skim it so far and it will take some time before I can dig in properly, but I wanted to provide some quick feedback here.
We also need to account for the case where there is some time in between fetching on the server, and hydrating on the client. This commonly happens when the markup is cached, in which case the hydration can happen days after the fetch on the server. In this situation, we do want to render with the stale data initially (this is what the SSR pass did and we need to match it), but we also want to immediately refetch the data because it's stale.
packages/query-core/src/hydration.ts
Outdated
|
|
||
| // WeakSet to track queries that are pending hydration | ||
| // Used to prevent double-fetching when HydrationBoundary defers hydration to useEffect | ||
| export const pendingHydrationQueries: WeakSet<Query<any, any, any, any>> = |
There was a problem hiding this comment.
I don't think we can use a global singleton to keep track of this, what if there are several queryClients? In that case a query might just be about to hydrate into one of these, not all of them.
Annoying as that is, I think we need to use a context for this. Note that there is some prior art with isRestoring.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/react-query/src/HydrationBoundary.tsx`:
- Around line 94-108: Remove the invalid ESLint disable directives referencing
the non-existent rule "react-hooks/refs" in HydrationBoundary.tsx and
useBaseQuery.ts: delete the inline comment before the hydrate call and the block
comments around the loop that adds to hydratingQueriesRef.current (the comments
around the for...of loop that references dehydratedQuery.queryHash) and any
identical directives in useBaseQuery (around the related refs usage); leave the
hydrate(...) call and the hydratingQueriesRef.current.add(...) logic unchanged,
ensuring no other eslint-disable comments are added.
In `@packages/react-query/src/useBaseQuery.ts`:
- Around line 58-63: Remove the invalid ESLint disable comment above the
hydratingQueriesRef access: delete the line "// eslint-disable-next-line
react-hooks/refs" so the code simply checks
hydratingQueriesRef.current.has(defaultedOptions.queryHash) and sets
defaultedOptions._isHydrating = true; ensure no other eslint-disable for
non-existent rules remain near the hydratingQueriesRef usage.
| if (newQueries.length > 0) { | ||
| // It's actually fine to call this with queries/state that already exists | ||
| // in the cache, or is older. hydrate() is idempotent for queries. | ||
| // eslint-disable-next-line react-hooks/refs | ||
| hydrate(client, { queries: newQueries }, optionsRef.current) | ||
| } | ||
|
|
||
| if (existingQueries.length > 0) { | ||
| // Add pending hashes to the mutable ref | ||
| /* eslint-disable react-hooks/refs */ | ||
| for (const dehydratedQuery of existingQueries) { | ||
| hydratingQueriesRef.current.add(dehydratedQuery.queryHash) | ||
| } | ||
| /* eslint-enable react-hooks/refs */ | ||
| return existingQueries |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find ESLint configuration files
echo "=== Checking for ESLint configuration files ==="
fd -t f -e js -e json -e yml -e yaml "eslint" . | head -20
# Check for .eslintrc variants
ls -la . | grep eslint || echo "No ESLint files in root"
# Check package.json for ESLint config
if [ -f package.json ]; then
echo "=== Checking package.json for eslintConfig ==="
jq '.eslintConfig' package.json 2>/dev/null || echo "No eslintConfig in root package.json"
fiRepository: TanStack/query
Length of output: 1194
🏁 Script executed:
#!/bin/bash
# Look for ESLint configuration in packages directory
echo "=== Checking packages for ESLint config ==="
find packages -maxdepth 2 -name ".eslintrc*" -o -name "package.json" | while read file; do
if [[ "$file" == *"package.json"* ]]; then
if grep -q "eslintConfig" "$file" 2>/dev/null; then
echo "Found eslintConfig in: $file"
fi
else
echo "Found ESLint file: $file"
fi
done
# Find the actual HydrationBoundary.tsx file
echo -e "\n=== Looking for HydrationBoundary.tsx ==="
find . -name "HydrationBoundary.tsx" -type fRepository: TanStack/query
Length of output: 193
🏁 Script executed:
#!/bin/bash
# Search for any usage of "react-hooks/refs" in the codebase
echo "=== Searching for react-hooks/refs rule usage ==="
rg "react-hooks/refs" . -nRepository: TanStack/query
Length of output: 687
🏁 Script executed:
#!/bin/bash
# Check the actual HydrationBoundary.tsx file
if [ -f "packages/react-query/src/HydrationBoundary.tsx" ]; then
echo "=== Lines 94-108 of HydrationBoundary.tsx ==="
sed -n '94,108p' packages/react-query/src/HydrationBoundary.tsx
echo ""
echo "=== Lines 136-141 of HydrationBoundary.tsx ==="
sed -n '136,141p' packages/react-query/src/HydrationBoundary.tsx
fiRepository: TanStack/query
Length of output: 1004
🏁 Script executed:
#!/bin/bash
# Check the eslint-plugin-react-hooks rules
echo "=== Checking for eslint-plugin-react-hooks documentation ==="
# First, let's see if it's installed and what version
if [ -d "node_modules/eslint-plugin-react-hooks" ]; then
echo "eslint-plugin-react-hooks is installed"
# Check available rules
if [ -f "node_modules/eslint-plugin-react-hooks/package.json" ]; then
echo "Version:"
jq '.version' node_modules/eslint-plugin-react-hooks/package.json
fi
else
echo "eslint-plugin-react-hooks not found in node_modules"
fi
# Search for documentation or rule definitions
rg "rules:" node_modules/eslint-plugin-react-hooks --type json --type js | head -20 2>/dev/null || echo "Could not find rules definition"Repository: TanStack/query
Length of output: 1355
🌐 Web query:
eslint-plugin-react-hooks rules documentation
💡 Result:
The official documentation for eslint-plugin-react-hooks and its rules is on the React site. Key pages:
- Plugin overview (recommended rules list and plugin notes). [1]
- rules-of-hooks (react-hooks/rules-of-hooks): enforces the two Rules of Hooks (only call Hooks at top level; only call Hooks from React functions). [2]
- exhaustive-deps (react-hooks/exhaustive-deps): validates dependency arrays for useEffect/useMemo/useCallback and config options for custom hooks. [3]
If you want, I can fetch and list every rule and their options/examples from the plugin docs.
🏁 Script executed:
#!/bin/bash
# Get the actual list of rules from eslint-plugin-react-hooks
echo "=== Extracting rules from eslint-plugin-react-hooks ==="
if [ -f "node_modules/eslint-plugin-react-hooks/package.json" ]; then
jq '.main' node_modules/eslint-plugin-react-hooks/package.json
# Check the main export
MAIN_FILE=$(jq -r '.main' node_modules/eslint-plugin-react-hooks/package.json)
if [ -f "node_modules/eslint-plugin-react-hooks/$MAIN_FILE" ]; then
echo "=== Checking rules in main export ==="
grep -A 50 "rules:" "node_modules/eslint-plugin-react-hooks/$MAIN_FILE" | head -70
fi
fiRepository: TanStack/query
Length of output: 164
🏁 Script executed:
#!/bin/bash
# Check the main eslint.config.js to see react-hooks configuration
echo "=== Checking root eslint.config.js ==="
cat eslint.config.jsRepository: TanStack/query
Length of output: 2186
🏁 Script executed:
#!/bin/bash
# Check if react-hooks/refs rule causes lint errors
# Look for how ESLint is configured for react-query package
echo "=== Checking react-query eslint.config.js ==="
cat packages/react-query/eslint.config.jsRepository: TanStack/query
Length of output: 936
🏁 Script executed:
#!/bin/bash
# Let's look at the actual rule definitions in the plugin
python3 << 'EOF'
import re
import json
# Read the eslint-plugin-react-hooks main file
with open('node_modules/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.production.js', 'r') as f:
content = f.read()
# Search for rule names
rule_pattern = r"'(rules-of-hooks|exhaustive-deps|refs)'|'react-hooks/([\w-]+)'"
matches = re.findall(rule_pattern, content)
print("Rules found in eslint-plugin-react-hooks:")
# Also look for the object keys in rules object
rules_section = re.search(r'rules\s*:\s*{([^}]+)}', content)
if rules_section:
rules_content = rules_section.group(1)
# Find quoted strings that look like rule names
rule_names = re.findall(r"'([^']+)':", rules_content)
unique_rules = sorted(set(rule_names))
for rule in unique_rules:
print(f" - {rule}")
EOFRepository: TanStack/query
Length of output: 144
Remove invalid ESLint disable directives for non-existent react-hooks/refs rule
The react-hooks/refs rule does not exist in eslint-plugin-react-hooks. Only react-hooks/rules-of-hooks and react-hooks/exhaustive-deps are official rules. These disable directives are invalid and should be removed:
- Line 97:
// eslint-disable-next-line react-hooks/refs - Lines 103–107:
/* eslint-disable react-hooks/refs *///* eslint-enable react-hooks/refs */ - Lines 136–141:
/* eslint-disable react-hooks/refs *///* eslint-enable react-hooks/refs */
Also in packages/react-query/src/useBaseQuery.ts:60
🧰 Tools
🪛 ESLint
[error] 97-97: Definition for rule 'react-hooks/refs' was not found.
(react-hooks/refs)
[error] 103-103: Definition for rule 'react-hooks/refs' was not found.
(react-hooks/refs)
[error] 107-107: Definition for rule 'react-hooks/refs' was not found.
(react-hooks/refs)
🤖 Prompt for AI Agents
In `@packages/react-query/src/HydrationBoundary.tsx` around lines 94 - 108, Remove
the invalid ESLint disable directives referencing the non-existent rule
"react-hooks/refs" in HydrationBoundary.tsx and useBaseQuery.ts: delete the
inline comment before the hydrate call and the block comments around the loop
that adds to hydratingQueriesRef.current (the comments around the for...of loop
that references dehydratedQuery.queryHash) and any identical directives in
useBaseQuery (around the related refs usage); leave the hydrate(...) call and
the hydratingQueriesRef.current.add(...) logic unchanged, ensuring no other
eslint-disable comments are added.
|
|
||
| // Check if this query is pending hydration (using mutable ref) | ||
| // eslint-disable-next-line react-hooks/refs | ||
| if (hydratingQueriesRef.current.has(defaultedOptions.queryHash)) { | ||
| defaultedOptions._isHydrating = true | ||
| } |
There was a problem hiding this comment.
Remove the invalid ESLint disable comment.
The ESLint directive react-hooks/refs references a non-existent rule (as confirmed by static analysis). Additionally, accessing a ref's .current property outside of hooks like useMemo/useCallback does not violate any React hooks rules, so this comment serves no purpose.
🔧 Suggested fix
// Check if this query is pending hydration (using mutable ref)
- // eslint-disable-next-line react-hooks/refs
if (hydratingQueriesRef.current.has(defaultedOptions.queryHash)) {
defaultedOptions._isHydrating = true
}📝 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.
| // Check if this query is pending hydration (using mutable ref) | |
| // eslint-disable-next-line react-hooks/refs | |
| if (hydratingQueriesRef.current.has(defaultedOptions.queryHash)) { | |
| defaultedOptions._isHydrating = true | |
| } | |
| // Check if this query is pending hydration (using mutable ref) | |
| if (hydratingQueriesRef.current.has(defaultedOptions.queryHash)) { | |
| defaultedOptions._isHydrating = true | |
| } |
🧰 Tools
🪛 ESLint
[error] 60-60: Definition for rule 'react-hooks/refs' was not found.
(react-hooks/refs)
🤖 Prompt for AI Agents
In `@packages/react-query/src/useBaseQuery.ts` around lines 58 - 63, Remove the
invalid ESLint disable comment above the hydratingQueriesRef access: delete the
line "// eslint-disable-next-line react-hooks/refs" so the code simply checks
hydratingQueriesRef.current.has(defaultedOptions.queryHash) and sets
defaultedOptions._isHydrating = true; ensure no other eslint-disable for
non-existent rules remain near the hydratingQueriesRef usage.
3 participants
🎯 Changes
Fixes #9610
When using
HydrationBoundarywith existing queries in the cache, an unnecessary refetch was triggered during hydration even though fresh data was about to be hydrated from the server.Root Cause
useQuery'sonSubscriberuns beforeHydrationBoundary'suseEffectcompletes hydrationSolution
pendingHydrationQueriesWeakSet to track queries pending hydrationuseMemo(before children render)queryObserver.onSubscribeif query is pending hydrationrefetchOnMount: 'always'still triggers refetch (user explicitly wants fresh data)useEffectafter hydration completesFiles Changed
packages/query-core/src/hydration.ts- ExportpendingHydrationQueriesWeakSetpackages/query-core/src/index.ts- Re-exportpendingHydrationQueriespackages/query-core/src/queryObserver.ts- Check pending hydration inonSubscribepackages/react-query/src/HydrationBoundary.tsx- Mark/clear pending hydration queries with cleanuppackages/react-query/src/__tests__/HydrationBoundary.test.tsx- Add 14 test casesdocs/framework/react/guides/ssr.md- Document new behaviordocs/framework/react/reference/hydration.md- Document new behavior✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
New Features
useIsHydratinghook andIsHydratingProviderto track hydration state.Bug Fixes
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.