fix: cleanly separate dev styles from prod runtime #6356
Conversation
📝 WalkthroughWalkthroughRefactors head-tag generation into new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant Handler as StartHandler
participant Manifest as RouterManifest
Client->>Server: HTTP request
Server->>Handler: route resolution -> matchedRoutes
Handler->>Manifest: getStartManifest(matchedRoutes)
alt TSS_DEV_SERVER == true
Manifest->>Manifest: compute matched route IDs
Manifest->>Manifest: build dev-styles URL
Manifest->>Handler: include dev-styles link in manifest
else
Manifest->>Handler: return manifest without dev-styles
end
Handler->>Server: render SSR HTML (includes manifest assets)
Server->>Client: SSR HTML served
Client->>Client: hydrate
Client->>Client: HeadContent.dev detects hydration complete
Client->>Client: remove dev-styles link elements from DOM
Client->>Client: filter dev-styles from future head rendering
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 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 |
|
View your CI Pipeline Execution ↗ for commit 5b75bfc
☁️ Nx Cloud last updated this comment at |
![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/vue-router/src/HeadContent.tsx (1)
12-22: ReplaceJSON.stringify(tag)keys with stable identifiers; nonce and other volatile SSR fields cause key instability across renders.Using
JSON.stringify(tag)as a key includes volatile fields likenonce(fromrouter.options.ssr?.noncein Scripts.tsx lines 30, 51, 74, 81) that change per request. This breaks Vue's key stability and causes unnecessary remounting and potential hydration mismatches.Suggested fix:
- Extract stable identifiers from tag attributes (
href,src,name,property)- Explicitly exclude volatile fields like
nonce- Fall back to a compact serialization for tags without stable fields
Proposed fix (omit `nonce` and keep keys small/stable)
export const HeadContent = Vue.defineComponent({ name: 'HeadContent', setup() { const tags = useTags() return () => { return tags().map((tag) => Vue.h(Asset, { ...tag, - key: `tsr-meta-${JSON.stringify(tag)}`, + key: `tsr-head-${tag.tag}-${stableTagKey(tag)}`, }), ) } }, }) + +function stableTagKey(tag: Record<string, any>) { + const attrs = tag?.attrs ? { ...tag.attrs } : undefined + if (attrs && 'nonce' in attrs) delete attrs.nonce + // Prefer common identity fields; fall back to a compact serialization. + return ( + attrs?.href ?? + attrs?.src ?? + attrs?.name ?? + attrs?.property ?? + (tag.children ? String(tag.children).slice(0, 64) : JSON.stringify({ ...tag, attrs })) + ) +}
🤖 Fix all issues with AI agents
In @packages/react-router/src/headContentUtils.tsx:
- Around line 164-169: The style tag objects are returning a top-level nonce
property which mismatches RouterManagedTag and other tag shapes; update the
mapping in the style tag creation (the map over ({ children, ...attrs })) to
place nonce inside attrs (e.g., add nonce into attrs before returning) so the
returned object is { tag: 'style', attrs, children } with attrs.nonce set,
matching meta/link/script tag structure and the RouterManagedTag type.
🧹 Nitpick comments (7)
packages/react-router/src/headContentUtils.tsx (2)
62-68: Nonce on meta tags is unconventional.Meta tags typically don't require or use CSP nonces. The nonce is usually only needed for
scriptandstyleelements. Addingnonceto meta tag attributes may be unnecessary and could clutter the HTML output.Consider removing nonce from meta tags
resultMeta.push({ tag: 'meta', attrs: { ...m, - nonce, }, })
128-129: Type assertion forstructuralSharingindicates API typing gap.The
as anyassertion onstructuralSharing: truesuggests a mismatch between the expected type and the actual usage. This pattern is repeated across multipleuseRouterStatecalls (lines 128, 154, 170, 188).Consider opening an issue to fix the typing in
useRouterStateoptions to properly acceptbooleanforstructuralSharing, eliminating the need foras any.packages/react-router/src/HeadContent.dev.tsx (1)
34-46: Consider more efficient key generation.The current approach using
JSON.stringify(tag)for keys works but could be inefficient. Consider using a hash function or constructing a simpler key from tag properties (e.g.,${tag.tag}-${tag.attrs?.name || tag.attrs?.property || ''}).However, given that head tags are typically small in number, the current implementation is acceptable.
♻️ Example of more efficient key generation
- <Asset {...tag} key={`tsr-meta-${JSON.stringify(tag)}`} nonce={nonce} /> + <Asset {...tag} key={`tsr-meta-${tag.tag}-${tag.attrs?.name || tag.attrs?.property || tag.attrs?.rel || ''}-${tag.children || ''}`} nonce={nonce} />packages/solid-router/src/headContentUtils.tsx (4)
14-18: Tighten typing: avoidmatch.meta!+filter(Boolean)+ later!indexing (strict TS + runtime safety).Proposed refactor (keeps behavior, removes non-null assertions)
const routeMeta = useRouterState({ select: (state) => { - return state.matches.map((match) => match.meta!).filter(Boolean) + return state.matches.map((match) => match.meta ?? []).filter((m) => m.length) }, })Also applies to: 24-27
92-123:links: considerflatMap+ type-narrowing instead offilter(Boolean).flat(1)(cleaner strict TS).
150-183:styles/headScripts: theas Array<RouterManagedTag>casts likely hide mismatches; prefersatisfiesat construction sites.
199-209:uniqBy: add an explicit return type for stricter TS signal (and easier public API reading).Proposed tweak
-export function uniqBy<T>(arr: Array<T>, fn: (item: T) => string) { +export function uniqBy<T>(arr: Array<T>, fn: (item: T) => string): Array<T> { const seen = new Set<string>() return arr.filter((item) => { const key = fn(item) if (seen.has(key)) { return false } seen.add(key) return true }) }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
packages/react-router/package.jsonpackages/react-router/src/HeadContent.dev.tsxpackages/react-router/src/HeadContent.tsxpackages/react-router/src/headContentUtils.tsxpackages/react-router/src/index.dev.tsxpackages/react-router/src/index.tsxpackages/react-router/tests/Scripts.test.tsxpackages/react-router/vite.config.tspackages/solid-router/package.jsonpackages/solid-router/src/HeadContent.dev.tsxpackages/solid-router/src/HeadContent.tsxpackages/solid-router/src/headContentUtils.tsxpackages/solid-router/src/index.dev.tsxpackages/solid-router/src/index.tsxpackages/solid-router/src/ssr/RouterServer.tsxpackages/solid-router/vite.config.tspackages/start-server-core/src/createStartHandler.tspackages/start-server-core/src/router-manifest.tspackages/vue-router/package.jsonpackages/vue-router/src/HeadContent.dev.tsxpackages/vue-router/src/HeadContent.tsxpackages/vue-router/src/headContentUtils.tsxpackages/vue-router/src/index.dev.tsxpackages/vue-router/src/index.tsxpackages/vue-router/src/ssr/RouterServer.tsxpackages/vue-router/vite.config.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript strict mode with extensive type safety for all code
Files:
packages/vue-router/src/ssr/RouterServer.tsxpackages/vue-router/src/index.tsxpackages/vue-router/src/HeadContent.dev.tsxpackages/react-router/tests/Scripts.test.tsxpackages/react-router/src/index.tsxpackages/react-router/src/HeadContent.dev.tsxpackages/solid-router/src/HeadContent.dev.tsxpackages/solid-router/src/headContentUtils.tsxpackages/solid-router/src/index.dev.tsxpackages/start-server-core/src/router-manifest.tspackages/vue-router/vite.config.tspackages/vue-router/src/headContentUtils.tsxpackages/vue-router/src/index.dev.tsxpackages/react-router/src/index.dev.tsxpackages/vue-router/src/HeadContent.tsxpackages/react-router/src/headContentUtils.tsxpackages/solid-router/src/ssr/RouterServer.tsxpackages/react-router/vite.config.tspackages/solid-router/vite.config.tspackages/react-router/src/HeadContent.tsxpackages/start-server-core/src/createStartHandler.tspackages/solid-router/src/HeadContent.tsxpackages/solid-router/src/index.tsx
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Implement ESLint rules for router best practices using the ESLint plugin router
Files:
packages/vue-router/src/ssr/RouterServer.tsxpackages/vue-router/src/index.tsxpackages/vue-router/src/HeadContent.dev.tsxpackages/react-router/tests/Scripts.test.tsxpackages/react-router/src/index.tsxpackages/react-router/src/HeadContent.dev.tsxpackages/solid-router/src/HeadContent.dev.tsxpackages/solid-router/src/headContentUtils.tsxpackages/solid-router/src/index.dev.tsxpackages/start-server-core/src/router-manifest.tspackages/vue-router/vite.config.tspackages/vue-router/src/headContentUtils.tsxpackages/vue-router/src/index.dev.tsxpackages/react-router/src/index.dev.tsxpackages/vue-router/src/HeadContent.tsxpackages/react-router/src/headContentUtils.tsxpackages/solid-router/src/ssr/RouterServer.tsxpackages/react-router/vite.config.tspackages/solid-router/vite.config.tspackages/react-router/src/HeadContent.tsxpackages/start-server-core/src/createStartHandler.tspackages/solid-router/src/HeadContent.tsxpackages/solid-router/src/index.tsx
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Use workspace protocol
workspace:*for internal dependencies in package.json files
Files:
packages/solid-router/package.jsonpackages/vue-router/package.jsonpackages/react-router/package.json
🧠 Learnings (10)
📚 Learning: 2025-11-02T16:16:24.898Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5732
File: packages/start-client-core/src/client/hydrateStart.ts:6-9
Timestamp: 2025-11-02T16:16:24.898Z
Learning: In packages/start-client-core/src/client/hydrateStart.ts, the `import/no-duplicates` ESLint disable is necessary for imports from `#tanstack-router-entry` and `#tanstack-start-entry` because both aliases resolve to the same placeholder file (`fake-start-entry.js`) in package.json during static analysis, even though they resolve to different files at runtime.
Applied to files:
packages/solid-router/package.jsonpackages/react-router/tests/Scripts.test.tsxpackages/vue-router/package.jsonpackages/react-router/package.jsonpackages/solid-router/src/index.dev.tsxpackages/start-server-core/src/router-manifest.tspackages/vue-router/vite.config.tspackages/vue-router/src/index.dev.tsxpackages/react-router/src/index.dev.tsxpackages/solid-router/src/ssr/RouterServer.tsxpackages/react-router/vite.config.tspackages/solid-router/vite.config.tspackages/start-server-core/src/createStartHandler.tspackages/solid-router/src/index.tsx
📚 Learning: 2025-12-06T15:03:07.223Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T15:03:07.223Z
Learning: Applies to **/*.{ts,tsx} : Use TypeScript strict mode with extensive type safety for all code
Applied to files:
packages/solid-router/package.json
📚 Learning: 2025-10-08T08:11:47.088Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5402
File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21
Timestamp: 2025-10-08T08:11:47.088Z
Learning: Test snapshot files in the router-generator tests directory (e.g., files matching the pattern `packages/router-generator/tests/generator/**/routeTree*.snapshot.ts` or `routeTree*.snapshot.js`) should not be modified or have issues flagged, as they are fixtures used to verify the generator's output and are intentionally preserved as-is.
Applied to files:
packages/react-router/tests/Scripts.test.tsxpackages/start-server-core/src/router-manifest.tspackages/start-server-core/src/createStartHandler.ts
📚 Learning: 2025-12-21T12:52:35.231Z
Learnt from: Sheraff
Repo: TanStack/router PR: 6171
File: packages/router-core/src/new-process-route-tree.ts:898-898
Timestamp: 2025-12-21T12:52:35.231Z
Learning: In `packages/router-core/src/new-process-route-tree.ts`, the matching logic intentionally allows paths without trailing slashes to match index routes with trailing slashes (e.g., `/a` can match `/a/` route), but not vice-versa (e.g., `/a/` cannot match `/a` layout route). This is implemented via the condition `!pathIsIndex || node.kind === SEGMENT_TYPE_INDEX` and is a deliberate design decision to provide better UX by being permissive with missing trailing slashes.
Applied to files:
packages/solid-router/src/index.dev.tsxpackages/start-server-core/src/router-manifest.tspackages/vue-router/src/index.dev.tsxpackages/start-server-core/src/createStartHandler.ts
📚 Learning: 2025-12-17T02:17:55.086Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 6120
File: packages/router-generator/src/generator.ts:654-657
Timestamp: 2025-12-17T02:17:55.086Z
Learning: In `packages/router-generator/src/generator.ts`, pathless_layout routes must receive a `path` property when they have a `cleanedPath`, even though they are non-path routes. This is necessary because child routes inherit the path from their parent, and without this property, child routes would not have the correct full path at runtime.
Applied to files:
packages/solid-router/src/index.dev.tsxpackages/start-server-core/src/router-manifest.tspackages/solid-router/src/ssr/RouterServer.tsxpackages/start-server-core/src/createStartHandler.ts
📚 Learning: 2025-10-01T18:30:26.591Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 5330
File: packages/router-core/src/router.ts:2231-2245
Timestamp: 2025-10-01T18:30:26.591Z
Learning: In `packages/router-core/src/router.ts`, the `resolveRedirect` method intentionally strips the router's origin from redirect URLs when they match (e.g., `https://foo.com/bar` → `/bar` for same-origin redirects) while preserving the full URL for cross-origin redirects. This logic should not be removed or simplified to use `location.publicHref` directly.
Applied to files:
packages/solid-router/src/index.dev.tsxpackages/start-server-core/src/router-manifest.tspackages/solid-router/src/ssr/RouterServer.tsxpackages/solid-router/src/index.tsx
📚 Learning: 2025-10-01T18:31:35.420Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 5330
File: e2e/react-start/custom-basepath/src/routeTree.gen.ts:58-61
Timestamp: 2025-10-01T18:31:35.420Z
Learning: Do not review files named `routeTree.gen.ts` in TanStack Router repositories, as these are autogenerated files that should not be manually modified.
Applied to files:
packages/start-server-core/src/router-manifest.tspackages/start-server-core/src/createStartHandler.ts
📚 Learning: 2025-12-06T15:03:07.223Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T15:03:07.223Z
Learning: Applies to **/*.{js,ts,tsx} : Implement ESLint rules for router best practices using the ESLint plugin router
Applied to files:
packages/start-server-core/src/router-manifest.tspackages/solid-router/src/ssr/RouterServer.tsxpackages/react-router/vite.config.ts
📚 Learning: 2025-12-06T15:03:07.223Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T15:03:07.223Z
Learning: Separate framework-agnostic core logic from React/Solid bindings
Applied to files:
packages/solid-router/src/ssr/RouterServer.tsx
📚 Learning: 2025-12-24T22:47:44.320Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 6211
File: e2e/react-start/i18n-paraglide/src/server.ts:6-6
Timestamp: 2025-12-24T22:47:44.320Z
Learning: In TanStack Router projects using `inlang/paraglide-js`, the callback passed to `paraglideMiddleware` should use `() => handler.fetch(req)` (referencing the outer `req`) instead of `({ request }) => handler.fetch(request)`. This is intentional because the router needs the untouched URL to perform its own rewrite logic with `deLocalizeUrl`/`localizeUrl`. The middleware's processed request would delocalize the URL and interfere with the router's rewrite handling.
Applied to files:
packages/start-server-core/src/createStartHandler.ts
🧬 Code graph analysis (3)
packages/start-server-core/src/router-manifest.ts (1)
packages/virtual-file-routes/src/api.ts (2)
route(66-84)rootRoute(10-19)
packages/react-router/src/headContentUtils.tsx (3)
packages/react-router/src/index.tsx (1)
RouterManagedTag(78-78)packages/vue-router/src/headContentUtils.tsx (1)
uniqBy(166-176)packages/router-core/src/router.ts (1)
state(1104-1106)
packages/start-server-core/src/createStartHandler.ts (1)
packages/router-core/src/route.ts (1)
AnyRoute(778-797)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test
- GitHub Check: Preview
🔇 Additional comments (39)
packages/start-server-core/src/router-manifest.ts (4)
4-5: LGTM: Good use of pre-computed constant.Pre-computing the basepath from the environment variable avoids repeated lookups and improves runtime performance.
12-18: LGTM: Well-documented API change.The optional
matchedRoutesparameter is properly typed withReadonlyArray, and the documentation clearly explains its dev-mode purpose.
1-2: All imported utilities (buildDevStylesUrl,AnyRoute,RouterManagedTag) are properly exported from@tanstack/router-coreand are available for use in this package.
27-38: Dev-styles cleanup is properly implemented across all frameworks.The
data-tanstack-router-dev-stylesattribute is correctly handled in all three dev implementations:
- React, Solid, and Vue each filter out dev styles after hydration
- All include fallback cleanup effects that remove orphaned dev-style link elements from the DOM when hydration completes
- Consistent pattern across frameworks ensures reliable cleanup in all scenarios
packages/start-server-core/src/createStartHandler.ts (5)
85-95: LGTM: Dev-mode caching disabled for route-specific styles.The conditional caching strategy is appropriate:
- Dev mode: Fresh manifest on each request to include route-specific dev styles
- Production: Cached manifest (dev styles not needed)
The performance impact of disabling caching in dev mode is acceptable since it only affects development builds.
321-324: LGTM: Signature properly extends executeRouter.The optional
matchedRoutesparameter maintains backward compatibility while enabling dev-styles injection.
340-340: LGTM: Correctly propagates matchedRoutes.The
matchedRoutesparameter is properly passed togetManifest, completing the data flow for dev-styles injection.
488-491: LGTM: Type definition matches implementation.The
executeRouterparameter type is correctly updated to include the optionalmatchedRoutesparameter, consistent with the function implementation.
555-558: LGTM: Completes the dev-styles data flow.The comment clearly explains the purpose, and
matchedRoutescorrectly flows fromrouter.getMatchedRoutes(pathname)through toexecuteRouter, enabling route-scoped dev-styles injection.packages/solid-router/vite.config.ts (1)
39-44: LGTM! Development entry point added consistently.The addition of
'./src/index.dev.tsx'to the entry array aligns with the PR's objective to separate dev-specific code from production builds. This change is consistent with the parallel updates in React and Vue router packages.packages/vue-router/package.json (1)
46-46: LGTM! Development export properly configured.The addition of the
developmentconditional export follows Node.js module resolution conventions correctly. WhenNODE_ENVor conditions include "development", bundlers will resolve to the dev build, enabling development-specific features like dev-style cleanup.packages/react-router/package.json (1)
53-53: LGTM! Development exports configured for both module formats.The addition of
developmentconditional exports for both ESM and CJS formats ensures that development builds are available regardless of the consuming project's module system. This follows Node.js conventions and properly supports the PR's goal of separating dev and production runtimes.Also applies to: 58-58
packages/solid-router/package.json (1)
51-51: LGTM! Development exports cover all distribution formats.The development conditional exports are properly configured across all three distribution formats:
solidcondition for JSX source (preserving reactivity)importfor ESM consumersrequirefor CJS consumersThis comprehensive coverage ensures dev-specific behavior works correctly regardless of how the package is consumed.
Also applies to: 56-56, 61-61
packages/solid-router/src/HeadContent.dev.tsx (2)
33-44: Well-structured hydration-aware filtering.The memo correctly implements the filtering strategy:
- Pre-hydration: all tags (including dev styles) render to match server HTML
- Post-hydration: dev styles are filtered out from the reactive tree
The combination of the DOM cleanup effect (lines 24-30) and this filtered rendering ensures dev styles are removed both from Solid's reactive tree and from any orphaned DOM elements. This dual approach handles both normal cases and hydration mismatches effectively.
7-7: DEV_STYLES_ATTR constant is consistent across the codebase.The
DEV_STYLES_ATTRconstant at line 7 ('data-tanstack-router-dev-styles') matches the attribute name used in server-side manifest generation, and is used consistently in cleanup logic and tag filtering across all framework implementations (React, Vue, Solid).packages/react-router/src/HeadContent.tsx (1)
11-21: LGTM! Clean separation of concerns.The refactored
HeadContentcomponent is now a thin renderer that delegates tag construction touseTags. The implementation is correct and follows good separation principles.One minor observation: using
JSON.stringify(tag)for React keys works but is computed on every render. SinceuseTagsalready deduplicates tags, this should be fine for typical head content sizes.packages/react-router/src/headContentUtils.tsx (1)
205-215: LGTM! Consistent utility implementation.The
uniqByfunction matches the Vue implementation exactly, ensuring consistent behavior across framework variants.packages/vue-router/src/ssr/RouterServer.tsx (1)
3-3: LGTM! Import path updated correctly.The import source change from
../HeadContentto../headContentUtilsaligns with the refactoring to centralize tag generation utilities.packages/vue-router/src/index.tsx (1)
342-342: LGTM! Public API export added correctly.The
useTagshook is now exported from the package's public API, enabling consumers to access head tag data programmatically if needed.packages/solid-router/src/ssr/RouterServer.tsx (1)
10-10: LGTM! Import path updated correctly.The import source change from
../HeadContentto../headContentUtilsaligns with the refactoring to centralize tag generation utilities across all framework variants (React, Vue, Solid).packages/vue-router/vite.config.ts (1)
27-32: LGTM! Development entry point added.The new
./src/index.dev.tsxentry point enables separate bundling of development-specific code (like dev styles cleanup), aligning with the PR's objective to cleanly separate dev styles from production runtime.packages/react-router/src/index.tsx (1)
346-346: LGTM! Public API export added correctly.The
useTagshook is now exported from the React router package's public API, enabling advanced consumers to access head tag data programmatically. This mirrors the same change in Vue router for cross-framework consistency.packages/react-router/vite.config.ts (1)
22-27: LGTM! Clean addition of development entry point.The addition of
./src/index.dev.tsxto the Vite entry points aligns well with the PR's objective to separate dev styles from production runtime, and mirrors the pattern used in solid-router and vue-router packages.packages/react-router/src/index.dev.tsx (1)
1-6: LGTM! Clean re-export pattern for development builds.The development entry point cleanly overrides
HeadContentwhile preserving all other exports. The comments clearly document the purpose, making this easy to understand and maintain.packages/solid-router/src/index.dev.tsx (1)
1-6: LGTM! Consistent dev entry point pattern across frameworks.This follows the same clean re-export pattern as the React Router variant, ensuring consistent developer experience across TanStack Router's framework implementations.
packages/react-router/tests/Scripts.test.tsx (1)
220-220: LGTM! Test correctly reflects dev-style separation.The updated test expectation correctly validates that dev stylesheet link tags are omitted from SSR/production output, aligning with the PR's objective to cleanly separate development styles from production runtime.
packages/vue-router/src/HeadContent.dev.tsx (1)
1-42: LGTM! Well-structured dev-time cleanup with defensive fallback.The component correctly:
- Renders all tags (including dev styles) during SSR and initial hydration
- Filters dev styles from the virtual DOM after hydration via the
hydratedcheck- Provides fallback DOM cleanup in
onMountedas a safety net for edge casesThe optional chaining on line 31 ensures type safety, and the pattern is consistent with the production HeadContent implementation referenced in the code snippets.
packages/vue-router/src/index.dev.tsx (1)
1-6: LGTM! Clean dev/prod separation pattern.The development entry point correctly re-exports all production exports and overrides HeadContent with the dev variant. This pattern aligns with the broader refactoring across React and Solid routers.
packages/react-router/src/HeadContent.dev.tsx (3)
1-7: LGTM! Clean imports and constant definition.The imports are well-organized and the
DEV_STYLES_ATTRconstant provides a single source of truth for the dev styles attribute name.
9-17: LGTM! Well-documented dev variant.The JSDoc clearly explains the development-specific behavior, including dev styles filtering and hydration cleanup.
18-32: LGTM! Correct hydration cleanup logic.The useEffect correctly waits for hydration to complete before removing orphaned dev styles link elements from the DOM. The guard condition prevents premature execution.
packages/solid-router/src/index.tsx (1)
349-350: LGTM! Clean export reorganization.The relocation of
useTagsfromHeadContentto the dedicatedheadContentUtilsmodule improves modularity. Since both exports remain available from the package root, this is a non-breaking refactoring that aligns with the changes in React and Vue routers.packages/vue-router/src/headContentUtils.tsx (4)
1-7: LGTM! Clean imports.All necessary dependencies are imported correctly.
8-74: LGTM! Robust meta tag building with proper deduplication.The meta building logic correctly:
- Implements last-wins semantics for titles by processing routes in reverse
- Handles JSON-LD with proper HTML escaping and error handling
- Deduplicates meta tags by
nameorpropertyattributeThe try-catch for invalid JSON-LD objects prevents runtime errors from malformed data.
76-149: LGTM! Comprehensive tag collection from multiple sources.The code correctly collects and transforms:
- Route-defined links
- Manifest-based modulepreload links
- Head scripts with proper attribute/children separation
- Manifest assets filtered to link tags
The
satisfies RouterManagedTagannotation ensures type safety.
151-176: LGTM! Correct Vue Composition API pattern.The function correctly returns a computed function that:
- Accesses
.valueon Vue refs- Combines tags from all sources in the correct order
- Deduplicates using
uniqBywith JSON.stringifyThe
uniqByimplementation efficiently uses aSetfor O(n) deduplication.Note: The function return pattern is appropriate for Vue's composition API, where the calling code will invoke this function to get the current tag list.
packages/solid-router/src/HeadContent.tsx (1)
1-20: LGTM! Excellent refactoring that improves separation of concerns.The simplified HeadContent implementation correctly:
- Delegates tag generation to the centralized
useTagsutility- Uses Solid's
Forcomponent to efficiently render tags- Maintains the MetaProvider wrapper for proper portal behavior
This refactoring reduces code duplication and makes the head content logic reusable across production and dev builds.
packages/vue-router/src/HeadContent.tsx (1)
4-4: Good simplification: centralizing tag construction inheadContentUtilsis the right direction.packages/solid-router/src/headContentUtils.tsx (1)
38-52: No changes needed; current implementation is correct. TheescapeHtml()function uses unicode escape sequences (\u003cfor<, etc.), which are valid JSON per RFC 7159 and do not modify JSON-LD semantics. When JSON is parsed, these escape sequences are correctly interpreted back to their original characters. This behavior is explicitly tested inpackages/router-core/tests/utils.test.ts(line 696–709) with the test "should handle script tag injection attempt in JSON," confirming that escaped JSON-LD round-trips correctly and prevents XSS injection. The same pattern is used across all router packages (React, Vue, Solid) with no reported issues.
| return () => | ||
| uniqBy( | ||
| [ | ||
| ...meta(), | ||
| ...preloadLinks(), | ||
| ...links(), | ||
| ...styles(), | ||
| ...headScripts(), | ||
| ] as Array<RouterManagedTag>, | ||
| (d) => { | ||
| return JSON.stringify(d) | ||
| }, | ||
| ) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if SSR nonce is expected to be present client-side (to avoid SSR/client tag diffs).
rg -n "ssr\?\?.nonce|ssr\.nonce|csp-nonce" -S packages/solid-router/srcRepository: TanStack/router
Length of output: 236
🏁 Script executed:
# Also check the context of the function to understand the full picture
cat -n packages/solid-router/src/headContentUtils.tsx | head -220 | tail -80Repository: TanStack/router
Length of output: 2367
🏁 Script executed:
# Check if Solid.createMemo is available/imported
rg -n "createMemo|Solid\." packages/solid-router/src/headContentUtils.tsx | head -20Repository: TanStack/router
Length of output: 142
🏁 Script executed:
# Check if uniqBy is being used elsewhere and understand the perf implications
rg -n "uniqBy|JSON\.stringify" packages/solid-router/src/headContentUtils.tsxRepository: TanStack/router
Length of output: 247
Strip nonce from dedup key and memoize the tag deduplication.
Using JSON.stringify(d) directly as the dedup key causes tags to be considered duplicates only when nonce values match. During SSR, the nonce is set from router.options.ssr?.nonce, but on the client it may differ or be removed, causing false duplicates on hydration. Additionally, calling JSON.stringify for every item on every dedup is unnecessary.
Create a stable tagKey function that excludes the nonce before stringifying, and wrap the dedup in Solid.createMemo() to prevent recomputation:
Proposed fix
export const useTags = () => {
const router = useRouter()
const nonce = router.options.ssr?.nonce
+ const tagKey = (d: RouterManagedTag) => {
+ const attrs = d.attrs ? { ...d.attrs } : undefined
+ if (attrs && 'nonce' in attrs) delete (attrs as any).nonce
+ return JSON.stringify({ ...d, attrs })
+ }
// ... meta/links/preloadLinks/styles/headScripts
- return () =>
- uniqBy(
- [
- ...meta(),
- ...preloadLinks(),
- ...links(),
- ...styles(),
- ...headScripts(),
- ] as Array<RouterManagedTag>,
- (d) => {
- return JSON.stringify(d)
- },
- )
+ return Solid.createMemo(() =>
+ uniqBy(
+ [...meta(), ...preloadLinks(), ...links(), ...styles(), ...headScripts()],
+ tagKey,
+ ),
+ )
}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/react-router/src/headContentUtils.tsx (3)
11-18: Avoidmatch.meta!for strict TS; prefer a type guard.
The non-null assertion weakens safety and can hide realundefinedcases (and violates the “strict type safety” guideline).Proposed diff
const routeMeta = useRouterState({ select: (state) => { - return state.matches.map((match) => match.meta!).filter(Boolean) + return state.matches + .map((match) => match.meta) + .filter((m): m is NonNullable<typeof m> => Boolean(m)) }, })
91-129: Removeas anyonstructuralSharing(or fix the option type).
structuralSharing: true as anydefeats strict typing and can hide an invalid option name at compile-time.If
useRouterStatesupports this option, preferstructuralSharing: true. If it doesn’t, either drop it here or extenduseRouterState’s options type to include it (so this file stays type-safe). As per coding guidelines, strict TS should avoidany.Also applies to: 131-156, 157-174, 175-192
193-205: Dedupe viaJSON.stringifyis fragile if tags ever contain non-JSON data.
If anyRouterManagedTagfields can contain non-serializable values (e.g. Symbols/BigInt/circular references/React elements),JSON.stringifycan throw and break rendering. If the type guarantees “JSON-only”, consider documenting/enforcing that at the type boundary; otherwise, consider a safer key (e.g., stable stringify of a normalized subset liketag + attrs + childrenwith sorted keys).
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/react-router/src/headContentUtils.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript strict mode with extensive type safety for all code
Files:
packages/react-router/src/headContentUtils.tsx
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Implement ESLint rules for router best practices using the ESLint plugin router
Files:
packages/react-router/src/headContentUtils.tsx
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T15:03:07.223Z
Learning: Applies to **/*.{js,ts,tsx} : Implement ESLint rules for router best practices using the ESLint plugin router
🧬 Code graph analysis (1)
packages/react-router/src/headContentUtils.tsx (2)
packages/solid-router/src/headContentUtils.tsx (2)
useTags(11-197)uniqBy(199-209)packages/vue-router/src/headContentUtils.tsx (2)
useTags(8-164)uniqBy(166-176)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test
- GitHub Check: Preview
🔇 Additional comments (2)
packages/react-router/src/headContentUtils.tsx (2)
207-217:uniqByimplementation looks good.
Simple, linear-time, and generic.
20-90: Remove this comment; the JSON-LD escaping approach is correct and necessary.The code properly uses
escapeHtml()withdangerouslySetInnerHTML. TheescapeHtmlfunction applies Unicode escapes (e.g.,\u003cfor<,\u003efor>), not HTML entities. SincedangerouslySetInnerHTMLsetsinnerHTMLdirectly without parsing Unicode escape sequences, these are preserved as literal characters in the JSON. When the script executes and the JSON is parsed, the runtime correctly interprets\u003cas<. This prevents XSS by ensuring special characters cannot break out of the script tag while preserving JSON integrity—no double-escaping occurs.Likely an incorrect or invalid review comment.
Summary by CodeRabbit
New Features
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.