isHydrationRequest not cleared when hydration navigation is aborted#14871
Open
nowells wants to merge 2 commits intoremix-run:mainfrom
Open
isHydrationRequest not cleared when hydration navigation is aborted#14871nowells wants to merge 2 commits intoremix-run:mainfrom
isHydrationRequest not cleared when hydration navigation is aborted#14871nowells wants to merge 2 commits intoremix-run:mainfrom
Conversation
…, causing `serverLoader()` to return stale SSR data`
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
isHydrationRequestnot cleared when hydration navigation is aborted, causingserverLoader()to return stale SSR dataWhat version of React Router are you using?
7.9.3
Steps to Reproduce
Failing integration test: See
integration/bug-report-test.tsin this PR's branch.The test:
/searchroute withclientLoader.hydrate = truethat reads?q=from the URLclientLoaderdoes async work before callingserverLoader()(simulating real-world patterns like cache seeding). AfirstCallflag ensures only the hydration invocation is slow — the subsequent PUSH invocation runs immediately./search?q=initial, then clicks a link to/search?q=updatedbefore hydration completes"updated"— but it shows"initial"(stale SSR data)To run:
pnpm test:integration bug-report --project chromiumExpected Behavior
When the hydration POP navigation is aborted by a new PUSH navigation,
serverLoader()in the PUSH navigation's loaders should fetch fresh data from the server.Actual Behavior
serverLoader()returns the original SSRinitialData(for the wrong URL) because theisHydrationRequestclosure variable was never cleared.Root Cause Analysis
In
createClientRoutes(lib/dom/ssr/routes.tsx), each route withhydrate = truethat matched the initial SSR URL gets a closure variableisHydrationRequest = true. The route'sdataRoute.loaderuses this flag to decide whetherserverLoader()returnsinitialData(cached SSR data) or callsfetchServerLoader()(network request). The flag is cleared in afinallyblock after the loader executes.The problem: when the hydration POP navigation is aborted (via
pendingNavigationController.abort()instartNavigation), loaders that haven't completed yet never reach theirfinallyblock, soisHydrationRequeststaystrue. When the new PUSH navigation calls these loaders (second invocation, same closure),serverLoader()seesisHydrationRequest === trueand returnsinitialData— the SSR data for the original URL params, not the navigation target.Key detail: React Router runs matched route loaders in parallel. During hydration, a
clientLoaderthat just doesawait serverLoader()completes almost instantly (sinceserverLoader()returnsinitialDatasynchronously), clearingisHydrationRequestbefore any abort can happen. The bug only manifests when theclientLoaderdoes async work before callingserverLoader()(e.g., cache operations, analytics, initialization) — keeping the loader pending long enough for the hydration POP to be aborted whileisHydrationRequestis stilltrue.Timeline:
Evidence from instrumented build (in our production app):
Suggested Fix
When the hydration navigation is aborted, clear
isHydrationRequestfor all routes — not just the ones whose loaders completed. Options:On abort, iterate routes and clear the flag:
When
pendingNavigationController.abort()fires for aninitialHydrationnavigation, setisHydrationRequest = falsefor all hydrating routes.Check URL in the loader:
In
dataRoute.loader, compare the request URL toinitialState.location— if they differ, clearisHydrationRequestbefore callingserverLoader().Use router state instead of closure:
Replace the
isHydrationRequestclosure with a check againstrouter.state.initializedor similar, which is always up-to-date.Option 1 is the most direct fix. Option 3 is the cleanest long-term.
The above diff allows this failing test to pass, and I believe addresses the intent and fixes the bug