-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
fix(react-query/HydrationBoundary): prevent unnecessary refetch during hydration #9968
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix(react-query/HydrationBoundary): prevent unnecessary refetch during hydration #9968
Conversation
🦋 Changeset detectedLatest commit: c4ad620 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 adds hydration-aware tracking and logic so queries hydrated by HydrationBoundary skip unnecessary refetches on mount when data is fresh; it introduces a hydrating-queries context, marks queries as _isHydrating, and updates observer subscribe logic and hydration flow to prevent double-fetches. Changes
Sequence Diagram(s)sequenceDiagram
actor App
participant HB as HydrationBoundary
participant IsH as IsHydratingProvider
participant Hook as useBaseQuery / useQueries
participant QO as QueryObserver
participant Cache as Query Cache
App->>HB: render with dehydrated state
HB->>HB: identify new vs existing queries
HB->>IsH: populate hydratingQueriesRef (pending hashes)
HB->>Cache: hydrate new queries
Hook->>IsH: check hydratingQueriesRef for queryHash
Hook->>Hook: set defaultedOptions._isHydrating = true (if pending)
Hook->>QO: create observer with _isHydrating flag
QO->>QO: onSubscribe -> resolve refetchOnMount
QO->>QO: compute shouldSkipFetchForHydration(_isHydrating, stale)
alt Hydrating & Data Fresh & not 'always'
QO->>Hook: skip fetch, return hydrated data
else Stale or refetchOnMount='always'
QO->>Cache: perform refetch
end
HB->>IsH: clear pending hashes after hydration
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 |
|
View your CI Pipeline Execution ↗ for commit c4ad620
☁️ Nx Cloud last updated this comment at |
86745a3 to
8626d42
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #9968 +/- ##
==========================================
+ Coverage 36.86% 42.81% +5.95%
==========================================
Files 245 173 -72
Lines 11102 8284 -2818
Branches 2079 1698 -381
==========================================
- Hits 4093 3547 -546
+ Misses 6484 4347 -2137
+ Partials 525 390 -135 🚀 New features to boost your workflow:
|
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 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.
🎯 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
IsHydratingContext(React Context) to track queries pending hydrationuseMemo(before children render)queryObserver.#executeFetchif query is pending hydration and data is freshrefetchOnMount: 'always'still triggers refetch (user explicitly wants fresh data)staleTime) triggers refetch (cached markup scenario)useEffectafter hydration completesWhy React Context instead of global state?
QueryClientinstances (micro-frontends, testing)HydrationBoundarycomponentsFiles Changed
packages/query-core/src/queryObserver.ts- Check pending hydration and stale state in#executeFetchpackages/query-core/src/types.ts- Add_isHydratingoption typepackages/react-query/src/HydrationBoundary.tsx- Mark/clear pending hydration queries with Contextpackages/react-query/src/IsHydratingProvider.ts- New Context provider for hydration statepackages/react-query/src/useBaseQuery.ts- Set_isHydratingflag from Contextpackages/react-query/src/useQueries.ts- Set_isHydratingflag from Contextpackages/react-query/src/index.ts- ExportuseIsHydratingandIsHydratingProviderpackages/react-query/src/__tests__/HydrationBoundary.test.tsx- Add 26 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
Release Notes
New Features
useIsHydratinghook to track pending hydration status.Bug Fixes
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.