refactor(react-router): useTags performance #6521
Conversation
📝 WalkthroughWalkthroughSeparates and reorders head tag collection into per-type aggregation (title, ld+json, meta, links, preloads, styles, head scripts) with backward-iteration deduplication and nonce propagation; updates RouteMatchExtensions typings to allow optional children on script/style elements; removes the exported Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx affected --targets=test:eslint,test:unit,tes... |
❌ Failed | 12m 11s | View ↗ |
nx run-many --target=build --exclude=examples/*... |
✅ Succeeded | 1m 1s | View ↗ |
☁️ Nx Cloud last updated this comment at 2026-01-26 10:04:22 UTC
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/react-router/src/headContentUtils.tsx (1)
66-81: Empty key causes unintended deduplication of meta tags withoutnameorproperty.When a meta tag lacks both
nameandpropertyattributes (e.g.,<meta charset="utf-8">or<meta http-equiv="..."/>), the key defaults to''. This causes all such meta tags to deduplicate against each other, keeping only the first one encountered.Additionally, spreading
nonceinto meta tag attrs (line 78) is unusual—nonce is typically only meaningful for<script>and<style>elements, not<meta>.🐛 Proposed fix for empty key deduplication
} else { // Deduplicate by name or property attribute const key = m.name ?? m.property ?? '' - if (key) { - if (seenMeta.has(key)) continue - seenMeta.add(key) - } + // Only deduplicate if we have a meaningful key + if (key !== '') { + if (seenMeta.has(key)) continue + seenMeta.add(key) + } metaTags.push({ tag: 'meta', attrs: { ...m, - nonce, }, }) }
🤖 Fix all issues with AI agents
In `@packages/react-router/src/headContentUtils.tsx`:
- Around line 231-237: The dedupe key generation inside the loop over
matchScripts (in headContentUtils.tsx) can produce an empty string when both
script.attrs.src and script.children are missing/empty, causing unintended
deduplication; update the loop in the block that builds key (where { children,
...attrs } = script and key = attrs.src ?? String(children ?? '')) to first
derive a normalizedKey (trim strings) and then skip processing (continue) if
normalizedKey is empty — or alternatively treat scripts with no meaningful key
as non-dedupe candidates by not checking/adding them to the seen set; adjust the
logic around seen.has(key)/seen.add(key) accordingly so only non-empty keys are
deduplicated.
- Around line 265-275: The function uniqBy is dead code after the refactor and
should be removed; delete the exported function uniqBy from headContentUtils.tsx
and any related export references, and ensure there are no lingering
imports/usages (e.g., check useTags and other helpers in this file) so the file
builds cleanly; if this was part of a public API intentionally kept, instead
align its implementation with the other routers by replacing it with an
arr.filter-based implementation, but otherwise remove the uniqBy declaration and
its export to eliminate the unused code.
| for (const script of matchScripts) { | ||
| if (!script) continue | ||
| // Deduplicate by src (external) or children (inline) before creating the object | ||
| const { children, ...attrs } = script | ||
| const key = attrs.src ?? String(children ?? '') | ||
| if (seen.has(key)) continue | ||
| seen.add(key) |
There was a problem hiding this comment.
Empty deduplication key when both src and children are empty.
If a script has neither src nor children (or both are empty), the key becomes '', causing all such scripts to deduplicate against each other. Consider skipping deduplication for scripts without a meaningful key, or skip invalid scripts entirely.
🔧 Suggested fix
const { children, ...attrs } = script
const key = attrs.src ?? String(children ?? '')
+ // Only deduplicate if we have a meaningful key
+ if (key === '') continue // Skip scripts with no src or content
if (seen.has(key)) continue
seen.add(key)🤖 Prompt for AI Agents
In `@packages/react-router/src/headContentUtils.tsx` around lines 231 - 237, The
dedupe key generation inside the loop over matchScripts (in
headContentUtils.tsx) can produce an empty string when both script.attrs.src and
script.children are missing/empty, causing unintended deduplication; update the
loop in the block that builds key (where { children, ...attrs } = script and key
= attrs.src ?? String(children ?? '')) to first derive a normalizedKey (trim
strings) and then skip processing (continue) if normalizedKey is empty — or
alternatively treat scripts with no meaningful key as non-dedupe candidates by
not checking/adding them to the seen set; adjust the logic around
seen.has(key)/seen.add(key) accordingly so only non-empty keys are deduplicated.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/react-router/src/headContentUtils.tsx (1)
67-93: Fix meta deduplication forcharset/httpEquiv/itemPropand align CSP nonce checks.Line 68 dedupes only by
name/property,content, andmedia. Meta tags that rely oncharset,httpEquiv, oritemPropwill all collapse into the same key and get dropped. Also, the CSP nonce check (seenMeta.has('csp-nonce')) doesn’t match the composite key format, so duplicates can slip in.🔧 Suggested fix
- const key = `${m.name ?? m.property}\0${m.content}\0${m.media}` - if (key) { - if (seenMeta.has(key)) continue - seenMeta.add(key) - } + const dedupeId = + m.name ?? + m.property ?? + (m.httpEquiv ? `http-equiv:${m.httpEquiv}` : undefined) ?? + (m.charset ? `charset:${m.charset}` : undefined) ?? + m.itemProp + const key = dedupeId + ? `${dedupeId}\0${m.content ?? ''}\0${m.media ?? ''}` + : null + if (key) { + if (seenMeta.has(key)) continue + seenMeta.add(key) + } @@ - if (nonce && !seenMeta.has('csp-nonce')) { + const cspKey = nonce ? `csp-nonce\0${nonce}\0` : null + if (nonce && (!cspKey || !seenMeta.has(cspKey))) { metaTags.push({ tag: 'meta', attrs: { property: 'csp-nonce', content: nonce, }, }) + if (cspKey) seenMeta.add(cspKey) }
♻️ Duplicate comments (1)
packages/react-router/src/headContentUtils.tsx (1)
232-246: Avoid deduping scripts with no meaningful key.If both
srcandchildrenare empty, the key becomes a generic string (e.g.,"undefined\0undefined\0undefined"), causing unrelated scripts to dedupe against each other. This was previously flagged and still applies.🔧 Suggested fix
const { children, ...attrs } = script - const key = `${attrs.src}\0${attrs.type}\0${children}` - if (seen.has(key)) continue - seen.add(key) + const hasKey = attrs.src || children + if (hasKey) { + const key = `${attrs.src ?? ''}\0${attrs.type ?? ''}\0${children ?? ''}` + if (seen.has(key)) continue + seen.add(key) + } result.push({
not human review yet
About 2x performance on
useTagsbefore
after
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.