fix(react-router): preserve inline head scripts in React tree after hydration#7107
fix(react-router): preserve inline head scripts in React tree after hydration#7107mixelburg wants to merge 2 commits intoTanStack:mainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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)
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 |
Bundle Size Benchmarks
Trend sparkline is historical gzip bytes ending with this PR measurement; lower is better. |
|
please add a regression test to the e2e test projects (we already have some tests for the scripts) |
|
View your CI Pipeline Execution ↗ for commit 97fadb4
☁️ Nx Cloud last updated this comment at |
![nx-cloud[bot]](https://avatars.githubusercontent.com/in/80458?s=60&v=4){kind=small}
There was a problem hiding this comment.
Nx Cloud is proposing a fix for your failed CI:
We narrowed the inline-script React-tree-preservation guard to only activate for components that went through the SSR pre-hydration phase, preventing the regression on client-side navigation where useHydrated() always returns true for newly mounted components. Without this fix, client navigation caused React to render an inert dangerouslySetInnerHTML script that the useEffect dedup check mistook for an already-executed script, so the script was never actually run.
Tip
✅ We verified this fix by re-running @tanstack/react-router:test:unit, tanstack-react-start-e2e-basic-spa:test:e2e.
Suggested Fix changes
diff --git a/packages/react-router/src/Asset.tsx b/packages/react-router/src/Asset.tsx
index f4016ae240..fa29d5dfc0 100644
--- a/packages/react-router/src/Asset.tsx
+++ b/packages/react-router/src/Asset.tsx
@@ -51,6 +51,11 @@ function Script({
}) {
const router = useRouter()
const hydrated = useHydrated()
+ // Track whether this component instance went through the !hydrated (pre-hydration) phase.
+ // true → component was SSR-rendered; the inline script was already executed by the browser.
+ // false → component was mounted fresh on the client (client-side navigation); useEffect must
+ // inject an executable script because dangerouslySetInnerHTML never executes scripts.
+ const wentThroughNonHydratedPhase = React.useRef(!hydrated)
const dataScript =
typeof attrs?.type === 'string' &&
attrs.type !== '' &&
@@ -215,11 +220,20 @@ function Script({
}
}
- // For inline scripts (children, no src), keep the element in the React tree after
- // hydration so React doesn't unmount the SSR-rendered script from the DOM.
+ // For inline scripts (children, no src) that went through SSR hydration, keep the element
+ // in the React tree so React doesn't unmount the SSR-rendered script from the DOM.
// The useEffect above detects the existing element via textContent match and skips
// re-injection, so the script won't execute a second time.
- if (!attrs?.src && typeof children === 'string') {
+ //
+ // For client-side navigation (wentThroughNonHydratedPhase === false), skip this path and
+ // fall through to return null — the useEffect handles imperative injection in that case.
+ // (dangerouslySetInnerHTML does not execute scripts, so we must not render an inert element
+ // that would fool the existingScript dedup check into returning early without executing.)
+ if (
+ !attrs?.src &&
+ typeof children === 'string' &&
+ wentThroughNonHydratedPhase.current
+ ) {
return (
<script
{...attrs}
Because this branch comes from a fork, it is not possible for us to apply fixes directly, but you can apply the changes locally using the available options below.
Apply changes locally with:
npx nx-cloud apply-locally Mdan-wqFq
Apply fix locally with your editor ↗ View interactive diff ↗
🎓 Learn more about Self-Healing CI on nx.dev
…dratedPhase ref Narrows the dangerouslySetInnerHTML React-tree-preservation guard to only activate for components that went through the SSR pre-hydration phase. On client-side navigation useHydrated() immediately returns true, so wentThroughNonHydratedPhase.current is false and we fall through to return null — the useEffect handles imperative injection instead. Without this fix, client navigation caused React to render an inert dangerouslySetInnerHTML script that the useEffect dedup check mistook for an already-executed script, so the script was never actually run. Fixes the 3 failing tests in Scripts.test.tsx.
2 participants
Fixes #7104
Problem
When a route's
head()function returnsscripts[]entries withchildren(inline script content) but nosrc, those script tags are rendered correctly on the server and during initial hydration, but disappear from the DOM after hydration completes.Root cause in
Asset.tsx— theScriptcomponent:<script dangerouslySetInnerHTML={{ __html: children }} />✓!hydrated): renders the same element to match server HTML ✓return null— React unmounts the script from the DOM ✗useEffectfires after React commits the removal →querySelectorAll('script:not([src])')no longer finds the script → creates a new one and appends it → script re-executes (e.g.gtagconfig double-fires)Both
solid-routerandvue-routeralready keep inline scripts rendered after hydration — onlyreact-routerhad this gap.Fix
After the
!hydratedblock, add an early-return that keeps inlinechildrenscripts in the React tree:With this change:
useEffectstill runs, but theexistingScriptcheck finds the element and returns early — so the script is not re-executedsolid-routerandvue-routerScripts with
srcare unaffected (still managed imperatively viauseEffect).Summary by CodeRabbit