fix(react-router): honor preventScriptHoist during hydration - #8036
fix(react-router): honor preventScriptHoist during hydration#8036grayashh wants to merge 4 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Included review availability: Your plan includes up to 10 reviews per rolling hour; 9 remain after this review. 📝 WalkthroughWalkthrough
ChangesScript hydration
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The change keeps scripts with preventScriptHoist in place during hydration and adds coverage for the mismatch scenario; no actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
43df437 to
fde43c4
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Fixes #7775
Problem
When
<Scripts />is mounted inside an element (or directly in<body>), every load produces a hydration mismatch once the Start manifest injects asrcscript such as the client entry. In production builds it surfaces as minified React error #418.preventScriptHoistis only honored in the server branch ofScriptinAsset.tsx. SSR attachesonLoad: noopScriptHandler, so React leaves the async src script in place in the body. The client!hydratedbranch ignores the flag and renders<script {...attrs} suppressHydrationWarning />. Withasync, a stringsrc, and no event handler, react-dom treats that as a hoistable resource and hoists it into<head>during hydration, so the server-rendered body script node is never claimed and hydration fails.suppressHydrationWarningdoesn't help because it only covers attribute and text diffs, not structural ones.Fix
Apply the same guard to the client
!hydratedbranch: whenpreventScriptHoistis set, attachonLoad={noopScriptHandler}so the hydration render matches the server output and the script stays in place.Test
Added a test that renders a router with a manifest src script to string on the server, then hydrates the same markup with
hydrateRootand assertsonRecoverableErroris never called. Without the fix it fails with "Hydration failed because the server rendered HTML didn't match the client", the exact error from the issue.Follow-up to my analysis comment on #7775. We had been running this change as a pnpm patch in two production TanStack Start apps: hydration mismatches went from one per load to zero, and the surviving body script is cleaned up by React after hydration with no double execution.
Summary by CodeRabbit