Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 43 additions & 5 deletions packages/react-router/src/Asset.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,7 @@ export function Asset({
}: RouterManagedTag & { nonce?: string }): React.ReactElement | null {
switch (tag) {
case 'title':
return (
<title {...attrs} suppressHydrationWarning>
{children}
</title>
)
return <Title attrs={attrs}>{children}</Title>
case 'meta':
return <meta {...attrs} suppressHydrationWarning />
case 'link':
Expand All @@ -40,6 +36,48 @@ export function Asset({
}
}

// Track if we've taken control of the title
let titleControlled = false
Comment on lines +39 to +40
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Module-level state causes lifecycle and isolation issues.

The titleControlled flag persists for the module's lifetime and is shared across all Title instances, which breaks in several scenarios:

  1. Testing: The flag isn't reset between test runs, causing test pollution.
  2. HMR: During development hot reloads, the flag remains true, preventing cleanup logic from running.
  3. Multiple routers: If the app uses nested routers or multiple router instances, they incorrectly share this flag.
  4. Microfrontends: Different apps on the same page will interfere with each other.

Consider one of these alternatives:

Option 1: Track in Router context (preferred if router has instance state):

// In Router initialization
this.titleControlled = false

// In Title component
const router = useRouter()
if (!router.titleControlled) {
  // cleanup logic
  router.titleControlled = true
}

Option 2: Use a ref per component (if only one Title should ever be active):

-let titleControlled = false
-
 function Title({ attrs, children }: { attrs?: Record<string, any>; children?: string }) {
   const router = useRouter()
+  const hasCleanedUp = React.useRef(false)

   React.useEffect(() => {
     if (typeof children === 'string') {
-      if (!titleControlled) {
+      if (!hasCleanedUp.current) {
         const existingTitles = Array.from(document.querySelectorAll('title'))
         existingTitles.forEach(titleEl => titleEl.remove())
-        titleControlled = true
+        hasCleanedUp.current = true
       }
       document.title = children
     }
   }, [children])

Note: Option 2 still has issues if multiple Title components mount simultaneously, but it's better isolated. Option 1 is the robust solution.

πŸ€– Prompt for AI Agents
In packages/react-router/src/Asset.tsx around lines 39-40, the module-level let
titleControlled = false creates shared state across all Title instances causing
test/HMR/multi-router/microfrontend issues; change it to instance-scoped state
by removing the module variable and tracking title control on the router
instance (preferred) β€” add a titleControlled boolean to the Router
initialization and read/update it from the Title component via the router
context (use router.titleControlled to decide/perform cleanup and set it true),
or if router changes are not possible, replace the module variable with a
per-component ref so each Title manages its own control state; ensure tests and
HMR no longer rely on module-level persistence.


function Title({
attrs,
children,
}: {
attrs?: Record<string, any>
children?: string
}) {
const router = useRouter()

React.useEffect(() => {
if (typeof children === 'string') {
// On the first title update, clean up any existing titles
if (!titleControlled) {
// Remove all existing title tags - router will now manage the title
const existingTitles = Array.from(document.querySelectorAll('title'))
existingTitles.forEach(titleEl => {
titleEl.remove()
})
titleControlled = true
}

// Set document.title directly - no DOM title tags needed on client
document.title = children
}
}, [children])

if (!router.isServer) {
// On client, don't render title tag - we manage document.title directly
return null
}

// On server, render title tag normally for SSR
return (
<title {...attrs} suppressHydrationWarning>
{children}
</title>
)
}

function Script({
attrs,
children,
Expand Down