Skip to content

Conversation

@ioslh
Copy link

@ioslh ioslh commented Nov 20, 2025

PR Description:

🐛 Problem & User Need

The Original Need

I want to display meaningful page titles immediately during route loading, before head() executes. Currently, there's a title gap because:

  1. Route matches → empty/stale title period begins
  2. beforeLoad executes → still no title update
  3. loader executes → still no title update
  4. head() executes → title finally updates

This creates poor UX with blank or stale titles during navigation.

Current Workaround & Issue

Based on existing APIs, the only way to set immediate titles is in beforeLoad:

export const Route = createFileRoute('/posts/$postId')({
  beforeLoad: ({ params }) => {
    // Only way to set immediate title with current API
    document.title = `Loading Post ${params.postId}...`
  },
  head: ({ loaderData }) => ({
    meta: [{ title: loaderData.title }], // Final title after loading
  }),
})

This causes duplicate title tags:

<!-- After beforeLoad sets document.title -->
<head>
  <title>Loading Post 123...</title>  <!-- Set by beforeLoad -->
</head>

<!-- After head() executes and Asset renders -->
<head>
  <title>Loading Post 123...</title>  <!-- Original from document.title -->
  <title>My Amazing Post</title>      <!-- New one from head() -->
</head>

The result is invalid HTML with multiple <title> elements, causing SEO and browser issues.

✅ Solution

This PR enables the workaround mentioned above by fixing the duplicate title issue. Now developers can safely use beforeLoad to set immediate titles without worrying about invalid HTML.

The fix modifies title handling in Asset.tsx to:

  1. Clean up conflicts: On first router-managed title update, automatically remove any existing <title> tags
  2. Use document.title API on client: Directly set document.title instead of rendering additional <title> elements
  3. Maintain SSR compatibility: Continue rendering <title> tags normally on the server for SEO

Result: Clean HTML

<!-- After this fix, only one title exists -->
<head>
  <title>My Amazing Post</title>  <!-- Only the final title -->
</head>

Technical Implementation

Client-side behavior:

  • First title update removes all existing <title> DOM elements
  • Subsequent updates only use document.title API
  • No additional <title> tags are rendered

Server-side behavior:

  • Normal <title> tag rendering for SSR/SEO
  • No changes to existing SSR behavior

🔧 Implementation Details

File Changed: packages/react-router/src/Asset.tsx

Before:

case 'title':
  return (
    <title {...attrs} suppressHydrationWarning>
      {children}
    </title>
  )

After:

case 'title':
  return <Title attrs={attrs}>{children}</Title>

// New Title component that handles deduplication
function Title({ attrs, children }) {
  const router = useRouter()

  React.useEffect(() => {
    if (typeof children === 'string') {
      // Clean up existing titles on first update
      if (!titleControlled) {
        document.querySelectorAll('title').forEach(el => el.remove())
        titleControlled = true
      }
      // Use document.title API directly
      document.title = children
    }
  }, [children])

  // Client: no DOM rendering, Server: normal SSR
  return !router.isServer ? null : <title {...attrs}>{children}</title>
}

📝 Usage & Developer Experience

Before This Fix

Developers faced a dilemma:

  • Use only head() → good HTML, but title gaps during loading
  • Use beforeLoad + head() → immediate titles, but invalid HTML with duplicates

After This Fix

Developers can now safely use the beforeLoad pattern without HTML validity concerns:

export const Route = createFileRoute('/posts/$postId')({
  beforeLoad: ({ params }) => {
    // ✅ Now safe to use - no more duplicate title tags!
    if (typeof window !== 'undefined') {
      document.title = `Loading Post ${params.postId}...`
    }
  },
  head: ({ loaderData }) => ({
    meta: [{ title: loaderData.title }], // Will cleanly replace the loading title
  }),
})

Summary: This PR fixes duplicate title tags by implementing clean title management that automatically handles conflicts while maintaining full backward compatibility and SSR support.

Summary by CodeRabbit

  • Refactor
    • Optimized how page titles are managed, improving client-side performance by updating the document title more efficiently while maintaining proper server-side behavior.

✏️ Tip: You can customize this high-level summary in your review settings.

This fix addresses invalid HTML caused by multiple <title> elements when
users manually set titles before router head management kicks in.

Changes:
- Modify Asset.tsx to use document.title API on client side
- Automatically remove conflicting title tags on first update
- Maintain normal SSR behavior for SEO compatibility
- No API changes - fully backward compatible

Fixes duplicate title tags while keeping the developer experience simple.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 20, 2025

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: prevent duplicate title tags in document head' directly and clearly describes the main problem being solved by this changeset: preventing duplicate title tags from appearing in the document head when titles are set both via document.title (in beforeLoad) and via head(). This is the core issue the PR addresses.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
packages/react-router/src/Asset.tsx (1)

42-48: Consider stronger typing for attrs.

The attrs parameter is typed as Record<string, any>, which is permissive. Since it's only used in the server-side render (line 75) and title elements support a limited set of attributes, consider using a more specific type:

interface TitleAttrs {
  id?: string
  className?: string
  lang?: string
  dir?: 'ltr' | 'rtl' | 'auto'
  // Add other valid HTML title attributes as needed
}

function Title({
  attrs,
  children,
}: {
  attrs?: TitleAttrs
  children?: string
}) {

This provides better type safety and autocomplete for consumers.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f139f1e and 1ffc1a9.

📒 Files selected for processing (1)
  • packages/react-router/src/Asset.tsx (2 hunks)
🔇 Additional comments (3)
packages/react-router/src/Asset.tsx (3)

19-19: LGTM: Clean delegation to Title component.

The refactor to delegate title rendering to a dedicated component is a good separation of concerns and aligns with the PR objective to prevent duplicate title tags.


68-78: SSR/CSR split is correctly implemented.

Verification confirms the implementation is sound:

  • Server-side: Renders <title> with suppressHydrationWarning (line 75)
  • Client-side: Returns null (line 70) to prevent rendering the title tag
  • Hydration handling: useEffect (lines 51-66) runs after hydration completes and manages document.title directly, removing any existing title tags exactly once via the titleControlled flag
  • Pattern consistency: Matches the Script component and other Asset components throughout the codebase

The intentional server-to-client mismatch (title tag → null) is properly suppressed and handled by the useEffect, so no hydration warnings will occur and no flicker will happen during the SSR → hydration → client transition.


51-66: Design decisions confirmed: title persistence, attrs handling, and race condition guards are intentional.

The code's behavior is by design:

  1. No cleanup function is correct. The title persists after unmount because routes change without destroying the component—the next route's title update resets it. This is the intended pattern for client-side routing.

  2. attrs only apply to server render. On the client, document.title is a string property, not a DOM element, so attrs like lang, dir, id don't apply. The server renders <title {...attrs}> for SSR, and the client manages document.title directly. This differs from the Script component (which uses attrs on both client and server), but that's because scripts are DOM elements on both sides.

  3. titleControlled guards against duplicate removals. The module-level flag prevents the cleanup code (lines 55-59) from running more than once globally. React serializes effects within a render, and across navigation, the flag persists, so only the first Title component removes existing titles. This is safe in practice.

No issues found. The implementation matches the intended design pattern for title management in a client-side router.

Comment on lines +39 to +40
// Track if we've taken control of the title
let titleControlled = false
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant