inline css#7253
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📥 CommitsReviewing files that changed from the base of the PR and between 661df155c05ed8d896d0f86ae65845793459a066 and 0b7a745. 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds SSR inline-CSS support end-to-end: build-time CSS collection and rebasing, manifest shape/extensions and SSR dehydration to emit inline style placeholders, runtime head/Asset rendering changes across adapters, build/tooling defines, rsbuild/vite integration, a new e2e example and Playwright tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser/Client
participant Server as SSR Server
participant Builder as Manifest Builder
participant CSS as CSS Processor
Browser->>Server: GET /
Server->>Builder: compute start manifest for matched routes
Builder->>Builder: collect referenced stylesheet hrefs
Builder->>CSS: fetch/rebase CSS content for hrefs
CSS-->>Builder: return rebased CSS strings
Builder->>Server: attach inlineCss map and replace link assets with style placeholders
Server-->>Browser: HTML with inline <style data-tsr-inline-css> and serialized manifest
Browser->>Browser: hydrate using inline styles (read style[data-tsr-inline-css] if placeholder)
Browser->>Browser: client navigation (no external stylesheet link fetches)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
View your CI Pipeline Execution ↗ for commit 0b7a745
☁️ Nx Cloud last updated this comment at |
🚀 Changeset Version Preview6 package(s) bumped directly, 17 bumped as dependents. 🟩 Patch bumps
|
schiller-manuel
force-pushed
the
inline-css
branch
2 times, most recently
from
76c747b to
17e5fa8
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
e2e/react-start/css-inline/src/components/Shell.tsx (1)
6-8: Avoidanyfor the hydration marker onwindow.Use a typed global
Windowaugmentation instead of(window as any)to keep strict TS safety.♻️ Suggested change
+declare global { + interface Window { + __CSS_INLINE_E2E_HYDRATED__?: boolean + } +} + export function Shell() { useEffect(() => { - ;(window as any).__CSS_INLINE_E2E_HYDRATED__ = true + window.__CSS_INLINE_E2E_HYDRATED__ = true }, [])As per coding guidelines,
**/*.{ts,tsx}: Use TypeScript strict mode with extensive type safety.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/react-start/css-inline/src/components/Shell.tsx` around lines 6 - 8, The code currently uses a cast (window as any) to set the __CSS_INLINE_E2E_HYDRATED__ flag inside useEffect; instead add a Window global augmentation declaring __CSS_INLINE_E2E_HYDRATED__?: boolean (e.g., in a nearby module or global.d.ts) and then replace (window as any).__CSS_INLINE_E2E_HYDRATED__ = true with a typed assignment window.__CSS_INLINE_E2E_HYDRATED__ = true inside the useEffect so TypeScript strict mode is preserved and no any cast is used.packages/react-router/src/headContentUtils.tsx (1)
330-366: Consider extracting shared asset mapping logic.The asset-to-tag transformation logic (lines 330-366) is nearly identical to the server-side version (lines 107-143). While this duplication is understandable given the different contexts (server function vs. React hook selector), extracting a shared helper could reduce maintenance burden.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react-router/src/headContentUtils.tsx` around lines 330 - 366, The asset-to-tag mapping logic is duplicated; extract a shared helper (e.g., mapAssetToRouterManagedTag(manifest, assetCrossOrigin, nonce, asset)) that encapsulates the switch on asset.tag and uses isInlinableStylesheet, getAssetCrossOrigin, nonce, and preserves inlineCss/children and attrs, then replace the .flatMap inline function here and the near-identical server-side mapper with calls to that helper so both contexts reuse the same transformation logic.packages/start-plugin-core/src/start-manifest-plugin/inlineCss.ts (1)
71-80: Consider adding type annotation for the import rule parameter.The
rule: anyparameter could benefit from a more specific type if lightningcss exports the rule type, though this is acceptable given the visitor pattern constraints.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/start-plugin-core/src/start-manifest-plugin/inlineCss.ts` around lines 71 - 80, The import(rule: any) visitor parameter is typed too loosely; replace the any with the specific LightningCSS rule type (e.g., the import/URL rule type exported by lightningcss) by importing the appropriate type and annotating the parameter (for example, import type { ImportRule } from 'lightningcss' and change the signature to import(rule: ImportRule)). If LightningCSS doesn't export a single exact type for this visitor, use the closest exported union/type that covers import rules or add a narrow local type alias for the fields you access (value.url) instead of any.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@e2e/react-start/css-inline/src/routes/__root.tsx`:
- Around line 3-4: The import order is incorrect: place the type import before
regular imports. Move the React type import (ReactNode) so the line "import type
{ ReactNode } from 'react'" appears before the component import "import { Shell
} from '~/components/Shell'"; ensure using the "import type" form for ReactNode
and maintain existing imports otherwise.
In `@e2e/react-start/css-inline/tests/css-inline.spec.ts`:
- Around line 13-20: The current getInlineCssTexts function filters style tags
by 'background', which can hide extra injected <style> tags; change
getInlineCssTexts to return the textContent of all <style> elements (no .filter)
so count assertions on total style tags are accurate, and perform any
'background' content checks separately where getInlineCssTexts is used (retain
the string check in the tests but not in the function). Update references to
getInlineCssTexts in the spec so assertions that expect a single inline style
tag (the count checks around the previous Line 120 and 131) use the raw list
length and any content assertions use a separate contains check.
- Around line 1-2: Replace the loose any types used in this test file with
concrete Playwright/DOM types: import and use Page for the Playwright page
object, Locator for query results created via page.locator, and
ElementHandle<HTMLElement> (or ElementHandle<Element>) for handles returned by
page.$ / page.evaluateHandle; for shadow roots or DOM nodes use the appropriate
DOM types (ShadowRoot | HTMLElement) or cast ElementHandle to
ElementHandle<HTMLElement>. Update each variable currently typed as any (the
ones flagged in the review) to the corresponding type (Page, Locator,
ElementHandle<HTMLElement>, etc.) and add an import line like import type {
Page, Locator, ElementHandle } from '@playwright/test' so the file remains in
strict TypeScript mode.
In `@packages/react-router/src/Asset.tsx`:
- Around line 46-58: The component in Asset.tsx destructures { tag, attrs,
children, nonce } but references asset.inlineCss which is undefined and causes a
ReferenceError; fix by either accepting the full asset prop (e.g., function
signature that receives asset and then read asset.inlineCss) or add inlineCss to
the destructured props (include inlineCss in the destructuring alongside tag,
attrs, children, nonce) and update all usages accordingly (ensure InlineCssStyle
branch reads the correct inlineCss variable and keep
TSS_INLINE_CSS_ENABLED/isServer checks intact).
In `@packages/router-core/tests/ssr-server-manifest.test.ts`:
- Around line 1-9: Reorder the imports at the top of the test so they follow
ESLint import/order: place built-in modules before external packages and test
framework imports before local project imports — specifically move
runInNewContext (node:vm) above createMemoryHistory (`@tanstack/history`), and
move describe/expect/test (vitest) before importing BaseRootRoute/BaseRoute and
attachRouterServerSsrUtils from ../src; ensure Manifest and DehydratedRouter
type imports and createTestRouter remain after external/test imports.
---
Nitpick comments:
In `@e2e/react-start/css-inline/src/components/Shell.tsx`:
- Around line 6-8: The code currently uses a cast (window as any) to set the
__CSS_INLINE_E2E_HYDRATED__ flag inside useEffect; instead add a Window global
augmentation declaring __CSS_INLINE_E2E_HYDRATED__?: boolean (e.g., in a nearby
module or global.d.ts) and then replace (window as
any).__CSS_INLINE_E2E_HYDRATED__ = true with a typed assignment
window.__CSS_INLINE_E2E_HYDRATED__ = true inside the useEffect so TypeScript
strict mode is preserved and no any cast is used.
In `@packages/react-router/src/headContentUtils.tsx`:
- Around line 330-366: The asset-to-tag mapping logic is duplicated; extract a
shared helper (e.g., mapAssetToRouterManagedTag(manifest, assetCrossOrigin,
nonce, asset)) that encapsulates the switch on asset.tag and uses
isInlinableStylesheet, getAssetCrossOrigin, nonce, and preserves
inlineCss/children and attrs, then replace the .flatMap inline function here and
the near-identical server-side mapper with calls to that helper so both contexts
reuse the same transformation logic.
In `@packages/start-plugin-core/src/start-manifest-plugin/inlineCss.ts`:
- Around line 71-80: The import(rule: any) visitor parameter is typed too
loosely; replace the any with the specific LightningCSS rule type (e.g., the
import/URL rule type exported by lightningcss) by importing the appropriate type
and annotating the parameter (for example, import type { ImportRule } from
'lightningcss' and change the signature to import(rule: ImportRule)). If
LightningCSS doesn't export a single exact type for this visitor, use the
closest exported union/type that covers import rules or add a narrow local type
alias for the fields you access (value.url) instead of any.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5f57d9ac-65d7-41b9-94e3-e33c37a46571
📥 Commits
Reviewing files that changed from the base of the PR and between f3cd565 and 8c265b2c4314cc825286a3299ec66c5febe827b1.
⛔ Files ignored due to path filters (2)
e2e/react-start/css-inline/src/styles/inline-dot.svgis excluded by!**/*.svgpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (49)
.changeset/smooth-cobras-inline.mde2e/react-start/css-inline/package.jsone2e/react-start/css-inline/playwright.config.tse2e/react-start/css-inline/rsbuild.config.tse2e/react-start/css-inline/server.jse2e/react-start/css-inline/src/components/NestedPanel.tsxe2e/react-start/css-inline/src/components/Shell.tsxe2e/react-start/css-inline/src/routeTree.gen.tse2e/react-start/css-inline/src/router.tsxe2e/react-start/css-inline/src/routes/__root.tsxe2e/react-start/css-inline/src/routes/app/dashboard/details.tsxe2e/react-start/css-inline/src/routes/app/dashboard/index.tsxe2e/react-start/css-inline/src/routes/app/dashboard/route.tsxe2e/react-start/css-inline/src/routes/app/route.tsxe2e/react-start/css-inline/src/routes/index.tsxe2e/react-start/css-inline/src/styles/app-layout.module.csse2e/react-start/css-inline/src/styles/dashboard-details.module.csse2e/react-start/css-inline/src/styles/dashboard-index.module.csse2e/react-start/css-inline/src/styles/dashboard-layout.module.csse2e/react-start/css-inline/src/styles/nested-panel.module.csse2e/react-start/css-inline/src/styles/shell.module.csse2e/react-start/css-inline/tests/css-inline.spec.tse2e/react-start/css-inline/tsconfig.jsone2e/react-start/css-inline/vite.config.tspackages/react-router/src/Asset.tsxpackages/react-router/src/headContentUtils.tsxpackages/router-core/src/index.tspackages/router-core/src/manifest.tspackages/router-core/src/ssr/ssr-server.tspackages/router-core/tests/ssr-server-manifest.test.tspackages/solid-router/src/Asset.tsxpackages/solid-router/src/headContentUtils.tsxpackages/start-plugin-core/package.jsonpackages/start-plugin-core/src/rsbuild/normalized-client-build.tspackages/start-plugin-core/src/rsbuild/plugin.tspackages/start-plugin-core/src/rsbuild/virtual-modules.tspackages/start-plugin-core/src/schema.tspackages/start-plugin-core/src/start-manifest-plugin/inlineCss.tspackages/start-plugin-core/src/start-manifest-plugin/manifestBuilder.tspackages/start-plugin-core/src/types.tspackages/start-plugin-core/src/vite/planning.tspackages/start-plugin-core/src/vite/plugin.tspackages/start-plugin-core/src/vite/start-manifest-plugin/normalized-client-build.tspackages/start-plugin-core/src/vite/start-manifest-plugin/plugin.tspackages/start-plugin-core/tests/start-manifest-plugin/manifestBuilder.test.tspackages/start-server-core/src/router-manifest.tspackages/start-server-core/src/transformAssetUrls.tspackages/vue-router/src/Asset.tsxpackages/vue-router/src/headContentUtils.tsx
| import { Shell } from '~/components/Shell' | ||
| import type { ReactNode } from 'react' |
There was a problem hiding this comment.
Fix import order: type imports should come before regular imports.
Per ESLint, the react type import on line 4 should precede the component import on line 3.
🔧 Proposed fix
import { HeadContent, Scripts, createRootRoute } from '@tanstack/react-router'
+import type { ReactNode } from 'react'
import { Shell } from '~/components/Shell'
-import type { ReactNode } from 'react'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { Shell } from '~/components/Shell' | |
| import type { ReactNode } from 'react' | |
| import { HeadContent, Scripts, createRootRoute } from '@tanstack/react-router' | |
| import type { ReactNode } from 'react' | |
| import { Shell } from '~/components/Shell' |
🧰 Tools
🪛 ESLint
[error] 4-4: react type import should occur before import of ~/components/Shell
(import/order)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@e2e/react-start/css-inline/src/routes/__root.tsx` around lines 3 - 4, The
import order is incorrect: place the type import before regular imports. Move
the React type import (ReactNode) so the line "import type { ReactNode } from
'react'" appears before the component import "import { Shell } from
'~/components/Shell'"; ensure using the "import type" form for ReactNode and
maintain existing imports otherwise.
| import { expect } from '@playwright/test' | ||
| import { test } from '@tanstack/router-e2e-utils' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify strict TS config and explicit `any` usage in this test file.
fd -a '^tsconfig.*\.json$'
rg -n --iglob 'tsconfig*.json' '"strict"\s*:\s*true|"noImplicitAny"\s*:\s*true'
rg -n --type=ts '\bany\b' e2e/react-start/css-inline/tests/css-inline.spec.tsRepository: TanStack/router
Length of output: 42868
🏁 Script executed:
cat -n e2e/react-start/css-inline/tests/css-inline.spec.tsRepository: TanStack/router
Length of output: 5364
Replace any with concrete Playwright/DOM types.
Lines 7, 13, 23, and 25 bypass type safety with any. The *.ts files in this repo maintain strict TypeScript mode and should use proper types.
Typing update
-import { expect } from '@playwright/test'
+import { expect, type Page } from '@playwright/test'
import { test } from '@tanstack/router-e2e-utils'
@@
-async function getStyle(page: any, testId: string, property: string) {
+async function getStyle(page: Page, testId: string, property: string) {
@@
-async function getInlineCssTexts(page: any) {
+async function getStyleTagTexts(page: Page) {
@@
-async function waitForHydration(page: any) {
+declare global {
+ interface Window {
+ __CSS_INLINE_E2E_HYDRATED__?: boolean
+ }
+}
+
+async function waitForHydration(page: Page) {
await page.waitForFunction(() => {
- return (window as any).__CSS_INLINE_E2E_HYDRATED__ === true
+ return window.__CSS_INLINE_E2E_HYDRATED__ === true
})
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@e2e/react-start/css-inline/tests/css-inline.spec.ts` around lines 1 - 2,
Replace the loose any types used in this test file with concrete Playwright/DOM
types: import and use Page for the Playwright page object, Locator for query
results created via page.locator, and ElementHandle<HTMLElement> (or
ElementHandle<Element>) for handles returned by page.$ / page.evaluateHandle;
for shadow roots or DOM nodes use the appropriate DOM types (ShadowRoot |
HTMLElement) or cast ElementHandle to ElementHandle<HTMLElement>. Update each
variable currently typed as any (the ones flagged in the review) to the
corresponding type (Page, Locator, ElementHandle<HTMLElement>, etc.) and add an
import line like import type { Page, Locator, ElementHandle } from
'@playwright/test' so the file remains in strict TypeScript mode.
| async function getInlineCssTexts(page: any) { | ||
| return page | ||
| .locator('style') | ||
| .evaluateAll((elements: Array<HTMLStyleElement>) => | ||
| elements | ||
| .map((element) => element.textContent ?? '') | ||
| .filter((text) => text.includes('background')), | ||
| ) |
There was a problem hiding this comment.
Avoid filtering <style> tags before asserting count.
Line 19 filters by 'background', so the count checks at Line 120 and Line 131 can still pass even if extra <style> tags are injected. That weakens the “single inline style tag” regression check.
🔧 Suggested fix
-async function getInlineCssTexts(page: any) {
+async function getStyleTagTexts(page: Page) {
return page
.locator('style')
.evaluateAll((elements: Array<HTMLStyleElement>) =>
- elements
- .map((element) => element.textContent ?? '')
- .filter((text) => text.includes('background')),
+ elements.map((element) => element.textContent ?? ''),
)
}
@@
- const inlineStyleText = (await getInlineCssTexts(page))[0]
+ const inlineStyleText = (await getStyleTagTexts(page))[0]
expect(inlineStyleText?.length).toBeGreaterThan(100)
@@
- await expect.poll(() => getInlineCssTexts(page)).toHaveLength(1)
+ await expect.poll(async () => (await getStyleTagTexts(page)).length).toBe(1)
await expect
- .poll(() => getInlineCssTexts(page))
- .toEqual([expect.stringContaining('background')])
+ .poll(async () => (await getStyleTagTexts(page))[0] ?? '')
+ .toContain('background')
@@
- await expect.poll(() => getInlineCssTexts(page)).toHaveLength(1)
+ await expect.poll(async () => (await getStyleTagTexts(page)).length).toBe(1)
await expect
- .poll(() => getInlineCssTexts(page))
- .toEqual([expect.stringContaining('background')])
+ .poll(async () => (await getStyleTagTexts(page))[0] ?? '')
+ .toContain('background')Also applies to: 120-124, 131-134
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@e2e/react-start/css-inline/tests/css-inline.spec.ts` around lines 13 - 20,
The current getInlineCssTexts function filters style tags by 'background', which
can hide extra injected <style> tags; change getInlineCssTexts to return the
textContent of all <style> elements (no .filter) so count assertions on total
style tags are accurate, and perform any 'background' content checks separately
where getInlineCssTexts is used (retain the string check in the tests but not in
the function). Update references to getInlineCssTexts in the spec so assertions
that expect a single inline style tag (the count checks around the previous Line
120 and 131) use the raw list length and any content assertions use a separate
contains check.
| import { createMemoryHistory } from '@tanstack/history' | ||
| import { runInNewContext } from 'node:vm' | ||
| import { BaseRootRoute, BaseRoute } from '../src' | ||
| import { attachRouterServerSsrUtils } from '../src/ssr/ssr-server' | ||
| import { GLOBAL_TSR } from '../src/ssr/constants' | ||
| import { createTestRouter } from './routerTestUtils' | ||
| import { describe, expect, test } from 'vitest' | ||
| import type { Manifest } from '../src/manifest' | ||
| import type { DehydratedRouter } from '../src/ssr/types' |
There was a problem hiding this comment.
Fix import order to satisfy ESLint.
Static analysis flagged import order violations. Per ESLint import/order rule, node:vm should come before @tanstack/history and vitest should come before ../src.
Proposed fix for import order
+import { runInNewContext } from 'node:vm'
import { createMemoryHistory } from '@tanstack/history'
-import { runInNewContext } from 'node:vm'
+import { describe, expect, test } from 'vitest'
import { BaseRootRoute, BaseRoute } from '../src'
import { attachRouterServerSsrUtils } from '../src/ssr/ssr-server'
import { GLOBAL_TSR } from '../src/ssr/constants'
import { createTestRouter } from './routerTestUtils'
-import { describe, expect, test } from 'vitest'
import type { Manifest } from '../src/manifest'
import type { DehydratedRouter } from '../src/ssr/types'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { createMemoryHistory } from '@tanstack/history' | |
| import { runInNewContext } from 'node:vm' | |
| import { BaseRootRoute, BaseRoute } from '../src' | |
| import { attachRouterServerSsrUtils } from '../src/ssr/ssr-server' | |
| import { GLOBAL_TSR } from '../src/ssr/constants' | |
| import { createTestRouter } from './routerTestUtils' | |
| import { describe, expect, test } from 'vitest' | |
| import type { Manifest } from '../src/manifest' | |
| import type { DehydratedRouter } from '../src/ssr/types' | |
| import { runInNewContext } from 'node:vm' | |
| import { createMemoryHistory } from '@tanstack/history' | |
| import { describe, expect, test } from 'vitest' | |
| import { BaseRootRoute, BaseRoute } from '../src' | |
| import { attachRouterServerSsrUtils } from '../src/ssr/ssr-server' | |
| import { GLOBAL_TSR } from '../src/ssr/constants' | |
| import { createTestRouter } from './routerTestUtils' | |
| import type { Manifest } from '../src/manifest' | |
| import type { DehydratedRouter } from '../src/ssr/types' |
🧰 Tools
🪛 ESLint
[error] 2-2: node:vm import should occur before import of @tanstack/history
(import/order)
[error] 7-7: vitest import should occur before import of ../src
(import/order)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/router-core/tests/ssr-server-manifest.test.ts` around lines 1 - 9,
Reorder the imports at the top of the test so they follow ESLint import/order:
place built-in modules before external packages and test framework imports
before local project imports — specifically move runInNewContext (node:vm) above
createMemoryHistory (`@tanstack/history`), and move describe/expect/test (vitest)
before importing BaseRootRoute/BaseRoute and attachRouterServerSsrUtils from
../src; ensure Manifest and DehydratedRouter type imports and createTestRouter
remain after external/test imports.
Bundle Size Benchmarks
Trend sparkline is historical gzip bytes ending with this PR measurement; lower is better. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/react-router/src/Asset.tsx (1)
77-78: Consider stricter typing forattrsparameter.Using
Record<string, any>reduces type safety. Consider usingRouterManagedTag['attrs']or a more specific type for consistency with the rest of the codebase.function InlineCssStyle({ attrs, children, nonce, }: { - attrs?: Record<string, any> + attrs?: RouterManagedTag['attrs'] children?: RouterManagedTag['children'] nonce?: string }) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react-router/src/Asset.tsx` around lines 77 - 78, The attrs parameter is currently typed as Record<string, any>, weakening type safety; update the parameter type to use the more specific RouterManagedTag['attrs'] (or another precise attrs type used elsewhere) in the Asset component/function signature so it aligns with the rest of the codebase and preserves attribute typings; locate the attrs declaration in Asset.tsx (the function/component parameter named attrs) and replace its type accordingly and run type checks to ensure compatibility.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/react-router/src/Asset.tsx`:
- Around line 88-92: The hydration code in InlineCssStyle that queries
document.querySelector(`style[${INLINE_CSS_HYDRATION_ATTR}]`) will always return
the first matching style so multiple inline CSS assets collide; modify the
component so each InlineCssStyle renders and queries using a unique attribute
value (e.g., include a content hash, route id or asset path in the attribute
name/value) instead of the shared INLINE_CSS_HYDRATION_ATTR, update the renderer
to emit that unique attribute and replace the querySelector call with a
querySelector that selects the specific unique attribute for that instance (use
the same unique token when marking server HTML and when hydrating in the
InlineCssStyle logic).
In `@packages/solid-router/src/Asset.tsx`:
- Around line 51-53: The hydration lookup currently uses document.querySelector
with INLINE_CSS_HYDRATION_ATTR which always returns the first inline style node;
change the logic in Asset.tsx where the ternary reads that query so it instead
uses document.querySelectorAll(`style[${INLINE_CSS_HYDRATION_ATTR}]`) and
consumes the nodes in order, picking the next unclaimed node for each Asset
instance (marking a node as claimed by removing the attribute or adding a
data-claimed flag) so each placeholder asset hydrates from its corresponding
inline style element rather than all using the first one.
---
Nitpick comments:
In `@packages/react-router/src/Asset.tsx`:
- Around line 77-78: The attrs parameter is currently typed as Record<string,
any>, weakening type safety; update the parameter type to use the more specific
RouterManagedTag['attrs'] (or another precise attrs type used elsewhere) in the
Asset component/function signature so it aligns with the rest of the codebase
and preserves attribute typings; locate the attrs declaration in Asset.tsx (the
function/component parameter named attrs) and replace its type accordingly and
run type checks to ensure compatibility.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e5611b24-12c2-4a80-b9e8-dc994e721099
📥 Commits
Reviewing files that changed from the base of the PR and between 8c265b2c4314cc825286a3299ec66c5febe827b1 and 17e5fa881619c99a9c8ba441ca80ad19b778ec8b.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
packages/react-router/src/Asset.tsxpackages/solid-router/src/Asset.tsx
| return ( | ||
| document.querySelector<HTMLStyleElement>( | ||
| `style[${INLINE_CSS_HYDRATION_ATTR}]`, | ||
| )?.textContent ?? undefined | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if multiple inline CSS assets can be emitted
rg -n 'inlineCss' --type ts -C 5 | head -80Repository: TanStack/router
Length of output: 6282
🏁 Script executed:
# Read the Asset.tsx file to understand the context around lines 88-92
cat -n packages/react-router/src/Asset.tsx | head -120Repository: TanStack/router
Length of output: 3752
🏁 Script executed:
# Search for INLINE_CSS_HYDRATION_ATTR definition and usage
rg -n 'INLINE_CSS_HYDRATION_ATTR' packages/react-router --type ts -B 2 -A 2Repository: TanStack/router
Length of output: 1131
🏁 Script executed:
# Check how inline CSS assets are rendered in react-router
rg -n 'inlineCss' packages/react-router --type ts -B 5 -A 5Repository: TanStack/router
Length of output: 2448
🏁 Script executed:
# Look at headContentUtils to see full context of how inline CSS assets are created
cat -n packages/react-router/src/headContentUtils.tsx | sed -n '125,145p'Repository: TanStack/router
Length of output: 617
🏁 Script executed:
# Search for where inline CSS assets are created or marked
rg -n 'inlineCss.*true' packages/react-router --type ts -B 3 -A 3Repository: TanStack/router
Length of output: 1154
🏁 Script executed:
# Check asset consolidation logic - see if assets are deduplicated or merged
rg -n 'const.*assets|return.*assets' packages/react-router/src/headContentUtils.tsx -A 2 -B 1Repository: TanStack/router
Length of output: 324
🏁 Script executed:
# Look at more context around asset assembly to understand the flow
cat -n packages/react-router/src/headContentUtils.tsx | sed -n '300,375p'Repository: TanStack/router
Length of output: 2609
🏁 Script executed:
# Check if there's any filtering or deduplication of inline CSS assets
rg -n 'filter|reduce|unique|deduplicate|consolidate' packages/react-router/src/headContentUtils.tsx --type tsRepository: TanStack/router
Length of output: 634
🏁 Script executed:
# Search for how manifest routes get their inline CSS assets
rg -n 'manifest.*routes|routes\[' packages/react-router/src/headContentUtils.tsx -B 2 -A 2Repository: TanStack/router
Length of output: 915
🏁 Script executed:
# Search for where inlineCss flag is set on assets in the build/manifest generation
rg -n 'inlineCss' packages/start-plugin-core packages/start-server-core --type ts -B 2 -A 2 | head -100Repository: TanStack/router
Length of output: 8917
🏁 Script executed:
# Check if there's any consolidation or merging of inline CSS assets from multiple routes
rg -n 'inlineCss|consolidate|merge' packages/react-router/src --type tsRepository: TanStack/router
Length of output: 801
🏁 Script executed:
# Look at buildInlineCssManifestData to understand how inline CSS assets are assigned
rg -n 'buildInlineCssManifestData' packages/start-plugin-core/src/start-manifest-plugin --type ts -A 30 | head -60Repository: TanStack/router
Length of output: 6275
🏁 Script executed:
# Check if assets are per-route or consolidated
rg -n 'route.assets\|manifest.routes' packages/start-plugin-core/src/start-manifest-plugin/manifestBuilder.ts -B 2 -A 2Repository: TanStack/router
Length of output: 41
🏁 Script executed:
# Look for where asset.inlineCss is set
rg -n 'asset\.inlineCss|inlineCss.*true' packages/start-plugin-core/src/start-manifest-plugin --type ts -B 5 -A 5Repository: TanStack/router
Length of output: 41
🏁 Script executed:
# Check how assets are created/marked for routes
cat -n packages/start-plugin-core/src/start-manifest-plugin/manifestBuilder.ts | sed -n '369,450p'Repository: TanStack/router
Length of output: 3001
🏁 Script executed:
# Check how appendUniqueAssets works (mentioned in test imports)
rg -n 'appendUniqueAssets' packages/start-plugin-core/src --type ts -A 20 | head -60Repository: TanStack/router
Length of output: 6018
🏁 Script executed:
# Look for where asset.inlineCss flag gets set during route manifest building
cat -n packages/start-plugin-core/src/start-manifest-plugin/manifestBuilder.ts | sed -n '450,550p'Repository: TanStack/router
Length of output: 3397
🏁 Script executed:
# Check how styles are actually rendered during SSR - look for where InlineCssStyle gets rendered
rg -n 'InlineCssStyle' packages/react-router/src --type ts -B 3 -A 3Repository: TanStack/router
Length of output: 938
🏁 Script executed:
# Verify if multiple style assets can be rendered for a single route or multiple active routes
cat -n packages/react-router/src/headContentUtils.tsx | sed -n '305,375p'Repository: TanStack/router
Length of output: 2495
Multiple inline CSS assets can coexist and querySelector returns only the first match during hydration.
When multiple routes with inline CSS assets are active simultaneously, each InlineCssStyle component renders with the same data-tsr-inline-css attribute. During client hydration, all components call querySelector to retrieve content, but since they all have an identical selector, every component retrieves content from the first matching element, causing data loss for subsequent assets.
Consider using a unique identifier (e.g., content hash, route ID, or asset source path) to distinguish between multiple inline CSS style elements during hydration.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/react-router/src/Asset.tsx` around lines 88 - 92, The hydration code
in InlineCssStyle that queries
document.querySelector(`style[${INLINE_CSS_HYDRATION_ATTR}]`) will always return
the first matching style so multiple inline CSS assets collide; modify the
component so each InlineCssStyle renders and queries using a unique attribute
value (e.g., include a content hash, route id or asset path in the attribute
name/value) instead of the shared INLINE_CSS_HYDRATION_ATTR, update the renderer
to emit that unique attribute and replace the querySelector call with a
querySelector that selects the specific unique attribute for that instance (use
the same unique token when marking server HTML and when hydrating in the
InlineCssStyle logic).
| : (document.querySelector<HTMLStyleElement>( | ||
| `style[${INLINE_CSS_HYDRATION_ATTR}]`, | ||
| )?.textContent ?? '') |
There was a problem hiding this comment.
Hydration lookup currently always reads the first inline style tag.
Line 51 uses querySelector, so multiple placeholder assets can all reuse the same CSS text from the first style[data-tsr-inline-css]. That can mis-hydrate when more than one inline style asset is present.
💡 Suggested fix (consume unclaimed inline style nodes in order)
const INLINE_CSS_HYDRATION_ATTR = 'data-tsr-inline-css'
+const consumedInlineCssStyles = new WeakSet<HTMLStyleElement>()
function InlineCssStyle({
attrs,
children,
}: {
attrs?: Record<string, any>
children?: RouterManagedTag['children']
}) {
const isInlineCssPlaceholder = children === undefined
const html = isInlineCssPlaceholder
? typeof document === 'undefined'
? ''
- : (document.querySelector<HTMLStyleElement>(
- `style[${INLINE_CSS_HYDRATION_ATTR}]`,
- )?.textContent ?? '')
+ : (() => {
+ const nextStyle = Array.from(
+ document.querySelectorAll<HTMLStyleElement>(
+ `style[${INLINE_CSS_HYDRATION_ATTR}]`,
+ ),
+ ).find((el) => !consumedInlineCssStyles.has(el))
+ if (nextStyle) consumedInlineCssStyles.add(nextStyle)
+ return nextStyle?.textContent ?? ''
+ })()
: (children ?? '')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/solid-router/src/Asset.tsx` around lines 51 - 53, The hydration
lookup currently uses document.querySelector with INLINE_CSS_HYDRATION_ATTR
which always returns the first inline style node; change the logic in Asset.tsx
where the ternary reads that query so it instead uses
document.querySelectorAll(`style[${INLINE_CSS_HYDRATION_ATTR}]`) and consumes
the nodes in order, picking the next unclaimed node for each Asset instance
(marking a node as claimed by removing the attribute or adding a data-claimed
flag) so each placeholder asset hydrates from its corresponding inline style
element rather than all using the first one.
![nx-cloud[bot]](https://avatars.githubusercontent.com/in/80458?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
schiller-manuel
force-pushed
the
inline-css
branch
from
11163a1 to
d09eb59
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
packages/solid-router/src/Asset.tsx (1)
44-51:⚠️ Potential issue | 🟠 MajorHydration lookup always reads the first inline style tag.
When multiple inline CSS assets are present,
querySelectorreturns only the first matchingstyle[data-tsr-inline-css]element. All placeholder assets will hydrate with the same CSS content, causing incorrect styling for subsequent assets.Consider consuming matched elements in order (e.g., via a
WeakSetto track claimed nodes, or by removing the attribute after reading).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/solid-router/src/Asset.tsx` around lines 44 - 51, The current inline-CSS hydration always uses the first matching node; change the lookup in the isInlineCssPlaceholder branch to consume matched nodes in order: call document.querySelectorAll<HTMLStyleElement>(`style[${INLINE_CSS_HYDRATION_ATTR}]`), locate the first element not yet claimed (use a module-scoped WeakSet like claimedInlineCssNodes or remove the hydration attribute from the element once read), read its .textContent and mark it claimed, and assign that string to html; reference symbols: isInlineCssPlaceholder, html, INLINE_CSS_HYDRATION_ATTR, and children to locate and replace the existing querySelector logic.packages/react-router/src/Asset.tsx (1)
85-89:⚠️ Potential issue | 🟠 MajorHydration collision issue persists with multiple inline CSS assets.
The
querySelectorat lines 86-88 returns only the firststyle[data-tsr-inline-css]element. When multiple inline CSS assets are rendered, all will hydrate with the same CSS content.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react-router/src/Asset.tsx` around lines 85 - 89, The current single-element query with document.querySelector using INLINE_CSS_HYDRATION_ATTR only grabs the first inline CSS style and causes hydration collisions; replace it with document.querySelectorAll<HTMLStyleElement>(`style[${INLINE_CSS_HYDRATION_ATTR}]`) in the same expression (in Asset.tsx where INLINE_CSS_HYDRATION_ATTR is used) then collect each element's textContent, filter out null/empty values and concatenate them (e.g., join with a newline) and return the concatenated string (or undefined if none) so all inline CSS assets are included during hydration.packages/vue-router/src/Asset.tsx (1)
189-195:⚠️ Potential issue | 🟠 MajorSame hydration collision issue as other framework implementations.
The
querySelectorcall at lines 192-195 will return only the first matchingstyle[data-tsr-inline-css]element, causing all placeholder assets to hydrate with identical CSS content when multiple inline CSS assets exist.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vue-router/src/Asset.tsx` around lines 189 - 195, The hydration currently uses document.querySelector which always returns the first matching style and causes collisions; change the hydratedInlineCss logic to use document.querySelectorAll( `style[${INLINE_CSS_HYDRATION_ATTR}]` ) and select the correct style node for this placeholder (instead of the first). Locate the inline placeholder identification in this component (isInlineCssPlaceholder / hydratedInlineCss) and either match by a per-asset identifier (e.g., a data attribute on the placeholder that corresponds to a data attribute on the style tag) or, if no identifier exists, determine this component's ordinal among inline placeholders and pick the same-indexed element from the NodeList to read textContent. Ensure you reference INLINE_CSS_HYDRATION_ATTR and preserve the existing undefined fallback when no matching element is found.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@e2e/react-start/rsc/tests/rsc-deferred-component-flash.spec.ts`:
- Around line 154-156: Replace the unsafe casts "window as any" used inside the
page.evaluate callbacks (where you set/read "__navigation_marker__") by
augmenting the global Window interface with a typed property; add a declaration
like Window { __navigation_marker__?: string } in this test file (or a nearby
.d.ts) and then use window.__navigation_marker__ directly in the evaluate
callbacks instead of "(window as any).__navigation_marker__".
In `@packages/router-plugin/src/core/router-composed-plugin.ts`:
- Around line 31-42: The plugin object named
'tanstack:router-inline-css-defaults' only provides a vite.config hook; add
bundler-specific hooks for webpack, rspack, and esbuild to set the same default
defines so TSS_INLINE_CSS_ENABLED is always set to "false" (i.e., set
'process.env.TSS_INLINE_CSS_ENABLED' and
'import.meta.env.TSS_INLINE_CSS_ENABLED' to JSON.stringify('false')); follow the
pattern used in router-generator-plugin.ts for implementing unplugin targets
(add webpack, rspack, esbuild keys alongside vite with their respective
config/transform hooks that return the same define map).
---
Duplicate comments:
In `@packages/react-router/src/Asset.tsx`:
- Around line 85-89: The current single-element query with
document.querySelector using INLINE_CSS_HYDRATION_ATTR only grabs the first
inline CSS style and causes hydration collisions; replace it with
document.querySelectorAll<HTMLStyleElement>(`style[${INLINE_CSS_HYDRATION_ATTR}]`)
in the same expression (in Asset.tsx where INLINE_CSS_HYDRATION_ATTR is used)
then collect each element's textContent, filter out null/empty values and
concatenate them (e.g., join with a newline) and return the concatenated string
(or undefined if none) so all inline CSS assets are included during hydration.
In `@packages/solid-router/src/Asset.tsx`:
- Around line 44-51: The current inline-CSS hydration always uses the first
matching node; change the lookup in the isInlineCssPlaceholder branch to consume
matched nodes in order: call
document.querySelectorAll<HTMLStyleElement>(`style[${INLINE_CSS_HYDRATION_ATTR}]`),
locate the first element not yet claimed (use a module-scoped WeakSet like
claimedInlineCssNodes or remove the hydration attribute from the element once
read), read its .textContent and mark it claimed, and assign that string to
html; reference symbols: isInlineCssPlaceholder, html,
INLINE_CSS_HYDRATION_ATTR, and children to locate and replace the existing
querySelector logic.
In `@packages/vue-router/src/Asset.tsx`:
- Around line 189-195: The hydration currently uses document.querySelector which
always returns the first matching style and causes collisions; change the
hydratedInlineCss logic to use document.querySelectorAll(
`style[${INLINE_CSS_HYDRATION_ATTR}]` ) and select the correct style node for
this placeholder (instead of the first). Locate the inline placeholder
identification in this component (isInlineCssPlaceholder / hydratedInlineCss)
and either match by a per-asset identifier (e.g., a data attribute on the
placeholder that corresponds to a data attribute on the style tag) or, if no
identifier exists, determine this component's ordinal among inline placeholders
and pick the same-indexed element from the NodeList to read textContent. Ensure
you reference INLINE_CSS_HYDRATION_ATTR and preserve the existing undefined
fallback when no matching element is found.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3c6bf7a3-9123-4882-aad1-03f2a24e28fc
📥 Commits
Reviewing files that changed from the base of the PR and between 17e5fa881619c99a9c8ba441ca80ad19b778ec8b and f22eddd121e26b431d2a8d812fadf293650e3385.
📒 Files selected for processing (7)
e2e/react-start/rsc/tests/rsc-deferred-component-flash.spec.tspackages/react-router/src/Asset.tsxpackages/router-plugin/src/core/router-composed-plugin.tspackages/solid-router/src/Asset.tsxpackages/start-plugin-core/src/rsbuild/plugin.tspackages/start-plugin-core/src/vite/planning.tspackages/vue-router/src/Asset.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/start-plugin-core/src/rsbuild/plugin.ts
| await page.evaluate(() => { | ||
| ;(window as any).__navigation_marker__ = 'client-side-navigation' | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify remaining unsafe marker accesses and any-casts in TS test files
rg -nP --type=ts '__navigation_marker__|window\s+as\s+any'Repository: TanStack/router
Length of output: 4756
Replace window as any casts with typed Window interface declaration.
Lines 155 and 163 use window as any which violates the TypeScript strict-mode requirement for this file. Add a global Window interface augmentation to provide type-safe access to __navigation_marker__:
Recommended refactor
+declare global {
+ interface Window {
+ __navigation_marker__?: 'client-side-navigation'
+ }
+}
+
test.describe('RSC Deferred Component Flash Prevention', () => {
@@
await page.evaluate(() => {
- ;(window as any).__navigation_marker__ = 'client-side-navigation'
+ window.__navigation_marker__ = 'client-side-navigation'
})
@@
expect(
- await page.evaluate(() => (window as any).__navigation_marker__),
+ await page.evaluate(() => window.__navigation_marker__),
).toBe('client-side-navigation')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@e2e/react-start/rsc/tests/rsc-deferred-component-flash.spec.ts` around lines
154 - 156, Replace the unsafe casts "window as any" used inside the
page.evaluate callbacks (where you set/read "__navigation_marker__") by
augmenting the global Window interface with a typed property; add a declaration
like Window { __navigation_marker__?: string } in this test file (or a nearby
.d.ts) and then use window.__navigation_marker__ directly in the evaluate
callbacks instead of "(window as any).__navigation_marker__".
There was a problem hiding this comment.
Nx Cloud has identified a flaky task in your failed CI:
🔂 Since the failure was identified as flaky, we triggered a CI rerun by adding an empty commit to this branch.
🎓 Learn more about Self-Healing CI on nx.dev
schiller-manuel
force-pushed
the
inline-css
branch
from
baef0e7 to
0186b35
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/start-server-core/src/transformAssetUrls.ts (1)
467-485:⚠️ Potential issue | 🟡 MinorAvoid sharing
inlineCssreference in the returned manifest.Line 484 returns
source.manifest.inlineCssdirectly. If this is object-shaped, mutations can leak into the cached base manifest despite the function’s immutability contract.Proposed fix
- return { inlineCss: source.manifest.inlineCss, routes } + return { + inlineCss: structuredClone(source.manifest.inlineCss), + routes, + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/start-server-core/src/transformAssetUrls.ts` around lines 467 - 485, The function buildManifestWithClientEntry currently returns source.manifest.inlineCss by reference which can allow external mutations to affect the cached base manifest; change the return so inlineCss is returned as a defensive copy (e.g., shallow-clone the object/array when present) instead of the original reference. Locate buildManifestWithClientEntry and replace the direct inlineCss passthrough (source.manifest.inlineCss) with a cloned value (e.g., source.manifest.inlineCss ? { ...source.manifest.inlineCss } or an appropriate shallow copy for arrays) to preserve immutability of the base manifest.
♻️ Duplicate comments (6)
e2e/react-start/css-inline/src/routes/__root.tsx (1)
2-4:⚠️ Potential issue | 🟡 MinorMove the React type import before the local value import.
Line 4 should be placed before Line 3 to satisfy
import/order.🔧 Proposed fix
import { HeadContent, Scripts, createRootRoute } from '@tanstack/react-router' -import { Shell } from '~/components/Shell' import type { ReactNode } from 'react' +import { Shell } from '~/components/Shell'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/react-start/css-inline/src/routes/__root.tsx` around lines 2 - 4, Reorder the imports to satisfy import/order by moving the type import for ReactNode before the local value import; specifically place "import type { ReactNode } from 'react'" above "import { Shell } from '~/components/Shell'" so the React type import appears with external/module imports (HeadContent, Scripts, createRootRoute) and before the local Shell value import.packages/router-core/tests/ssr-server-manifest.test.ts (1)
1-9:⚠️ Potential issue | 🟡 MinorFix import order to satisfy ESLint.
Static analysis flagged import order violations. The
node:vmimport should occur before@tanstack/history(builtins before externals), andvitestshould occur before../srcimports (test framework before local).Proposed fix for import order
+import { runInNewContext } from 'node:vm' import { createMemoryHistory } from '@tanstack/history' -import { runInNewContext } from 'node:vm' +import { describe, expect, test } from 'vitest' import { BaseRootRoute, BaseRoute } from '../src' import { attachRouterServerSsrUtils } from '../src/ssr/ssr-server' import { GLOBAL_TSR } from '../src/ssr/constants' import { createTestRouter } from './routerTestUtils' -import { describe, expect, test } from 'vitest' import type { Manifest } from '../src/manifest' import type { DehydratedRouter } from '../src/ssr/types'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/router-core/tests/ssr-server-manifest.test.ts` around lines 1 - 9, Reorder the imports to satisfy ESLint import-order rules: move the builtin import runInNewContext from 'node:vm' above the external import createMemoryHistory from '@tanstack/history', and move the test framework imports (describe, expect, test from 'vitest') so they appear before the local project imports (BaseRootRoute, BaseRoute, attachRouterServerSsrUtils, GLOBAL_TSR, createTestRouter, Manifest, DehydratedRouter) — adjust the top of packages/router-core/tests/ssr-server-manifest.test.ts so builtins come first, externals next, then test framework, then local imports, keeping the same symbols (createMemoryHistory, runInNewContext, describe/expect/test, BaseRootRoute/BaseRoute, attachRouterServerSsrUtils, GLOBAL_TSR, createTestRouter, Manifest, DehydratedRouter).e2e/react-start/css-inline/tests/css-inline.spec.ts (2)
13-20:⚠️ Potential issue | 🟠 MajorDo not filter style tags before count assertions.
Line 19 filters to CSS containing
'background', so the “single style tag” checks at Lines 120 and 131 can still pass even if extra<style>tags are present.Also applies to: 120-124, 131-134
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/react-start/css-inline/tests/css-inline.spec.ts` around lines 13 - 20, getInlineCssTexts currently filters style tag texts to only those containing 'background', which masks extra <style> tags and lets count assertions pass incorrectly; change getInlineCssTexts (the function using page.locator('style').evaluateAll) to return all style.textContent values without the .filter step, and if any tests need only background-containing CSS add a new helper (e.g., getInlineCssWithBackground) that applies the filter so count assertions use the unfiltered getInlineCssTexts.
7-27:⚠️ Potential issue | 🟡 MinorReplace
anywith concrete Playwright/DOM types.Lines 7, 13, 23, and 25 bypass strict typing and make this test less safe to refactor.
Type-safe update
-import { expect } from '@playwright/test' +import { expect, type Page } from '@playwright/test' @@ -async function getStyle(page: any, testId: string, property: string) { +async function getStyle(page: Page, testId: string, property: string) { @@ -async function getInlineCssTexts(page: any) { +async function getInlineCssTexts(page: Page) { @@ -async function waitForHydration(page: any) { +declare global { + interface Window { + __CSS_INLINE_E2E_HYDRATED__?: boolean + } +} + +async function waitForHydration(page: Page) { await page.waitForFunction(() => { - return (window as any).__CSS_INLINE_E2E_HYDRATED__ === true + return window.__CSS_INLINE_E2E_HYDRATED__ === true }) }As per coding guidelines,
**/*.{ts,tsx}: Use TypeScript strict mode with extensive type safety.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/react-start/css-inline/tests/css-inline.spec.ts` around lines 7 - 27, Replace the loose any types in getStyle, getInlineCssTexts and waitForHydration with concrete Playwright/DOM types: change the page parameter to Playwright's Page type (import from '@playwright/test') and use Locator/HTMLStyleElement/HTMLElement types for locator/evaluate callbacks (e.g., evaluateAll callback should accept Array<HTMLStyleElement> and getStyle's element parameter should be Element or HTMLElement), keep prop/property as string, and tighten waitForHydration's window access by typing window as Window & { __CSS_INLINE_E2E_HYDRATED__?: boolean } so no any remains in getStyle, getInlineCssTexts, or waitForHydration.packages/react-router/src/Asset.tsx (1)
86-88:⚠️ Potential issue | 🟠 MajorHydration currently always reads the first inline style element.
Line 86 uses
querySelector, so multiple inline CSS placeholders can all resolve to the firststyle[data-tsr-inline-css], causing mis-hydration.Proposed fix (ordered consumption)
const INLINE_CSS_HYDRATION_ATTR = 'data-tsr-inline-css' +const consumedInlineCssStyles = new WeakSet<HTMLStyleElement>() @@ - return ( - document.querySelector<HTMLStyleElement>( - `style[${INLINE_CSS_HYDRATION_ATTR}]`, - )?.textContent ?? undefined - ) + const nextStyle = Array.from( + document.querySelectorAll<HTMLStyleElement>( + `style[${INLINE_CSS_HYDRATION_ATTR}]`, + ), + ).find((el) => !consumedInlineCssStyles.has(el)) + if (nextStyle) consumedInlineCssStyles.add(nextStyle) + return nextStyle?.textContent ?? undefined🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react-router/src/Asset.tsx` around lines 86 - 88, The hydration code currently calls document.querySelector with INLINE_CSS_HYDRATION_ATTR which always returns the first matching inline style, causing multiple Asset instances to read the same style; change the logic in Asset.tsx to call document.querySelectorAll(`style[${INLINE_CSS_HYDRATION_ATTR}]`), convert to an array, and consume the elements in order (e.g., use elements.shift() for each Asset instance or index into the array using an explicit assetIndex) so each Asset reads and removes/consumes the next matching style node instead of always the first.packages/solid-router/src/Asset.tsx (1)
48-50:⚠️ Potential issue | 🟠 MajorHydration lookup can map multiple placeholders to the same style node.
Line 48 uses
querySelector, which always returns the firststyle[data-tsr-inline-css]. If multiple inline CSS placeholders exist, later instances can hydrate with the wrong CSS text.Proposed fix (consume unclaimed style nodes in order)
const INLINE_CSS_HYDRATION_ATTR = 'data-tsr-inline-css' +const consumedInlineCssStyles = new WeakSet<HTMLStyleElement>() @@ - : (document.querySelector<HTMLStyleElement>( - `style[${INLINE_CSS_HYDRATION_ATTR}]`, - )?.textContent ?? '') + : (() => { + const nextStyle = Array.from( + document.querySelectorAll<HTMLStyleElement>( + `style[${INLINE_CSS_HYDRATION_ATTR}]`, + ), + ).find((el) => !consumedInlineCssStyles.has(el)) + if (nextStyle) consumedInlineCssStyles.add(nextStyle) + return nextStyle?.textContent ?? '' + })()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/solid-router/src/Asset.tsx` around lines 48 - 50, The hydration lookup currently uses document.querySelector(`style[${INLINE_CSS_HYDRATION_ATTR}]`) which always returns the first matching style node and can wrongly hydrate later placeholders; change this to document.querySelectorAll(`style[${INLINE_CSS_HYDRATION_ATTR}]`) and consume nodes in order by selecting the next unused node for each hydration (e.g., track a per-document index or mark a node as claimed by removing/changing the attribute after using it) so each placeholder gets the corresponding style text; update the code path that reads .textContent (the lookup using INLINE_CSS_HYDRATION_ATTR) to use that next-node logic instead of a single querySelector.
🧹 Nitpick comments (3)
packages/start-server-core/src/router-manifest.ts (1)
64-90: Typemanifestat declaration instead of casting at return.With the new
inlineCssfield (Line 65), the cast at Line 90 can hide shape drift. Prefer declaration-site typing so TS validates the full object immediately.Proposed refactor
- const manifest = { + const manifest: StartManifestWithClientEntry['manifest'] = { inlineCss: startManifest.inlineCss, routes: Object.fromEntries( Object.entries(startManifest.routes).flatMap(([k, v]) => { @@ return { - manifest: manifest as StartManifestWithClientEntry['manifest'], + manifest, clientEntry: startManifest.clientEntry, injectedHeadScripts, }As per coding guidelines:
**/*.{ts,tsx}: Use TypeScript strict mode with extensive type safety.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/start-server-core/src/router-manifest.ts` around lines 64 - 90, Declare the manifest with its proper type at declaration instead of casting at return: change the declaration of const manifest so it is typed as StartManifestWithClientEntry['manifest'] (or a suitable intermediate type) and then build inlineCss and routes (using Object.fromEntries/entries mapping) to satisfy that type; ensure the routes entry mapper (processing startManifest.routes, v.preloads, v.assets) returns the typed shape so TypeScript validates the full object at creation rather than relying on the cast in the return.packages/start-plugin-core/src/start-manifest-plugin/inlineCss.ts (1)
88-93: Avoid coercing unsupported asset source objects to CSS text.
String(source)can silently produce invalid inline CSS (e.g.[object Object]). Prefer returningundefinedfor unsupported shapes so callers can skip safely.Safer normalization
export function getCssAssetSource(source: unknown) { if (typeof source === 'string') return source if (source instanceof Uint8Array) return Buffer.from(source).toString('utf8') - if (source == null) return undefined - return String(source) + return undefined }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/start-plugin-core/src/start-manifest-plugin/inlineCss.ts` around lines 88 - 93, The getCssAssetSource function currently falls back to String(source) which can coerce objects into invalid CSS; change it to only accept valid text-like inputs (string or Uint8Array) and return undefined for any other shape. Update getCssAssetSource to: if source is string return it; if source is Uint8Array return Buffer.from(...).toString('utf8'); if source == null return undefined; otherwise return undefined (do not call String(source)). This ensures callers can safely skip unsupported asset shapes.packages/vue-router/src/Asset.tsx (1)
209-209: Avoidanyreturn type onAsset.Line 209 drops type safety for a core render path. Prefer
Vue.VNode | null(or a precise Vue node union) instead ofany.Type-safe signature
-export function Asset(asset: RouterManagedTag): any { +export function Asset(asset: RouterManagedTag): Vue.VNode | null {As per coding guidelines,
**/*.{ts,tsx}: Use TypeScript strict mode with extensive type safety.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vue-router/src/Asset.tsx` at line 209, The Asset function currently returns any; change its signature to return a precise Vue node type (e.g., Vue.VNode | null or the appropriate JSX/TSX vnode union) by replacing the return type on the Asset(asset: RouterManagedTag) declaration and importing the required Vue types (or JSX namespace) so the compiler recognizes VNode; also update any internal early-return points to explicitly return null instead of undefined and ensure the JSX return expressions conform to the chosen VNode/JSX type so the function no longer uses any.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/router-core/src/manifest.ts`:
- Around line 81-82: The rel token comparison is case-sensitive; convert the rel
string to lowercase before splitting/checking so ASCII case-insensitive matches
succeed: when computing relTokens from rel (the variable used in this module,
near the check using relTokens.includes('stylesheet')), normalize rel to
lowercase (e.g., via toLowerCase()) before splitting so tokens like "StyleSheet"
or "STYLESHEET" will match 'stylesheet'.
In `@packages/start-plugin-core/src/start-manifest-plugin/inlineCss.ts`:
- Around line 64-82: Replace the untyped parameter in the visitor Rule.import
callback: change the signature from "rule: any" to use LightningCSS's ImportRule
type (import it from the lightningcss AST types), update the callback to accept
"rule: ImportRule", and ensure any property access (rule.value.url) remains
valid; locate the visitor object (visitor -> Rule -> import) where
rebaseCssUrl(rule.value.url, options.cssHref) is called and update imports
accordingly so the file compiles under strict TypeScript.
In `@packages/vue-router/src/Asset.tsx`:
- Around line 192-194: The selector in Asset.tsx that uses
`style[${INLINE_CSS_HYDRATION_ATTR}]` is fragile; update the placeholder
rendering and lookup so each instance is uniquely identifiable (e.g., add an
instance index or data-id attribute when rendering the placeholder/CSS asset and
use that attribute in the querySelector instead of the generic
`INLINE_CSS_HYDRATION_ATTR`), so the placeholder branch (where `children ===
undefined`) locates its own style tag reliably; adjust the code paths that
set/read the attribute (search for INLINE_CSS_HYDRATION_ATTR, the placeholder
logic, and the CSS asset render in this file) to propagate and consume the
instance-specific identifier (or use `nth-of-type()` keyed by that index) so
lookups are deterministic.
---
Outside diff comments:
In `@packages/start-server-core/src/transformAssetUrls.ts`:
- Around line 467-485: The function buildManifestWithClientEntry currently
returns source.manifest.inlineCss by reference which can allow external
mutations to affect the cached base manifest; change the return so inlineCss is
returned as a defensive copy (e.g., shallow-clone the object/array when present)
instead of the original reference. Locate buildManifestWithClientEntry and
replace the direct inlineCss passthrough (source.manifest.inlineCss) with a
cloned value (e.g., source.manifest.inlineCss ? { ...source.manifest.inlineCss }
or an appropriate shallow copy for arrays) to preserve immutability of the base
manifest.
---
Duplicate comments:
In `@e2e/react-start/css-inline/src/routes/__root.tsx`:
- Around line 2-4: Reorder the imports to satisfy import/order by moving the
type import for ReactNode before the local value import; specifically place
"import type { ReactNode } from 'react'" above "import { Shell } from
'~/components/Shell'" so the React type import appears with external/module
imports (HeadContent, Scripts, createRootRoute) and before the local Shell value
import.
In `@e2e/react-start/css-inline/tests/css-inline.spec.ts`:
- Around line 13-20: getInlineCssTexts currently filters style tag texts to only
those containing 'background', which masks extra <style> tags and lets count
assertions pass incorrectly; change getInlineCssTexts (the function using
page.locator('style').evaluateAll) to return all style.textContent values
without the .filter step, and if any tests need only background-containing CSS
add a new helper (e.g., getInlineCssWithBackground) that applies the filter so
count assertions use the unfiltered getInlineCssTexts.
- Around line 7-27: Replace the loose any types in getStyle, getInlineCssTexts
and waitForHydration with concrete Playwright/DOM types: change the page
parameter to Playwright's Page type (import from '@playwright/test') and use
Locator/HTMLStyleElement/HTMLElement types for locator/evaluate callbacks (e.g.,
evaluateAll callback should accept Array<HTMLStyleElement> and getStyle's
element parameter should be Element or HTMLElement), keep prop/property as
string, and tighten waitForHydration's window access by typing window as Window
& { __CSS_INLINE_E2E_HYDRATED__?: boolean } so no any remains in getStyle,
getInlineCssTexts, or waitForHydration.
In `@packages/react-router/src/Asset.tsx`:
- Around line 86-88: The hydration code currently calls document.querySelector
with INLINE_CSS_HYDRATION_ATTR which always returns the first matching inline
style, causing multiple Asset instances to read the same style; change the logic
in Asset.tsx to call
document.querySelectorAll(`style[${INLINE_CSS_HYDRATION_ATTR}]`), convert to an
array, and consume the elements in order (e.g., use elements.shift() for each
Asset instance or index into the array using an explicit assetIndex) so each
Asset reads and removes/consumes the next matching style node instead of always
the first.
In `@packages/router-core/tests/ssr-server-manifest.test.ts`:
- Around line 1-9: Reorder the imports to satisfy ESLint import-order rules:
move the builtin import runInNewContext from 'node:vm' above the external import
createMemoryHistory from '@tanstack/history', and move the test framework
imports (describe, expect, test from 'vitest') so they appear before the local
project imports (BaseRootRoute, BaseRoute, attachRouterServerSsrUtils,
GLOBAL_TSR, createTestRouter, Manifest, DehydratedRouter) — adjust the top of
packages/router-core/tests/ssr-server-manifest.test.ts so builtins come first,
externals next, then test framework, then local imports, keeping the same
symbols (createMemoryHistory, runInNewContext, describe/expect/test,
BaseRootRoute/BaseRoute, attachRouterServerSsrUtils, GLOBAL_TSR,
createTestRouter, Manifest, DehydratedRouter).
In `@packages/solid-router/src/Asset.tsx`:
- Around line 48-50: The hydration lookup currently uses
document.querySelector(`style[${INLINE_CSS_HYDRATION_ATTR}]`) which always
returns the first matching style node and can wrongly hydrate later
placeholders; change this to
document.querySelectorAll(`style[${INLINE_CSS_HYDRATION_ATTR}]`) and consume
nodes in order by selecting the next unused node for each hydration (e.g., track
a per-document index or mark a node as claimed by removing/changing the
attribute after using it) so each placeholder gets the corresponding style text;
update the code path that reads .textContent (the lookup using
INLINE_CSS_HYDRATION_ATTR) to use that next-node logic instead of a single
querySelector.
---
Nitpick comments:
In `@packages/start-plugin-core/src/start-manifest-plugin/inlineCss.ts`:
- Around line 88-93: The getCssAssetSource function currently falls back to
String(source) which can coerce objects into invalid CSS; change it to only
accept valid text-like inputs (string or Uint8Array) and return undefined for
any other shape. Update getCssAssetSource to: if source is string return it; if
source is Uint8Array return Buffer.from(...).toString('utf8'); if source == null
return undefined; otherwise return undefined (do not call String(source)). This
ensures callers can safely skip unsupported asset shapes.
In `@packages/start-server-core/src/router-manifest.ts`:
- Around line 64-90: Declare the manifest with its proper type at declaration
instead of casting at return: change the declaration of const manifest so it is
typed as StartManifestWithClientEntry['manifest'] (or a suitable intermediate
type) and then build inlineCss and routes (using Object.fromEntries/entries
mapping) to satisfy that type; ensure the routes entry mapper (processing
startManifest.routes, v.preloads, v.assets) returns the typed shape so
TypeScript validates the full object at creation rather than relying on the cast
in the return.
In `@packages/vue-router/src/Asset.tsx`:
- Line 209: The Asset function currently returns any; change its signature to
return a precise Vue node type (e.g., Vue.VNode | null or the appropriate
JSX/TSX vnode union) by replacing the return type on the Asset(asset:
RouterManagedTag) declaration and importing the required Vue types (or JSX
namespace) so the compiler recognizes VNode; also update any internal
early-return points to explicitly return null instead of undefined and ensure
the JSX return expressions conform to the chosen VNode/JSX type so the function
no longer uses any.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a1480be5-2371-41db-9123-67b5489decd2
📥 Commits
Reviewing files that changed from the base of the PR and between f22eddd121e26b431d2a8d812fadf293650e3385 and 661df155c05ed8d896d0f86ae65845793459a066.
⛔ Files ignored due to path filters (2)
e2e/react-start/css-inline/src/styles/inline-dot.svgis excluded by!**/*.svgpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (52)
.changeset/smooth-cobras-inline.mde2e/react-start/css-inline/package.jsone2e/react-start/css-inline/playwright.config.tse2e/react-start/css-inline/rsbuild.config.tse2e/react-start/css-inline/server.jse2e/react-start/css-inline/src/components/NestedPanel.tsxe2e/react-start/css-inline/src/components/Shell.tsxe2e/react-start/css-inline/src/routeTree.gen.tse2e/react-start/css-inline/src/router.tsxe2e/react-start/css-inline/src/routes/__root.tsxe2e/react-start/css-inline/src/routes/app/dashboard/details.tsxe2e/react-start/css-inline/src/routes/app/dashboard/index.tsxe2e/react-start/css-inline/src/routes/app/dashboard/route.tsxe2e/react-start/css-inline/src/routes/app/route.tsxe2e/react-start/css-inline/src/routes/index.tsxe2e/react-start/css-inline/src/styles/app-layout.module.csse2e/react-start/css-inline/src/styles/dashboard-details.module.csse2e/react-start/css-inline/src/styles/dashboard-index.module.csse2e/react-start/css-inline/src/styles/dashboard-layout.module.csse2e/react-start/css-inline/src/styles/nested-panel.module.csse2e/react-start/css-inline/src/styles/shell.module.csse2e/react-start/css-inline/tests/css-inline.spec.tse2e/react-start/css-inline/tsconfig.jsone2e/react-start/css-inline/vite.config.tse2e/react-start/hmr/tests/app.spec.tse2e/react-start/rsc/tests/rsc-deferred-component-flash.spec.tspackages/react-router/src/Asset.tsxpackages/react-router/src/headContentUtils.tsxpackages/router-core/src/index.tspackages/router-core/src/manifest.tspackages/router-core/src/ssr/ssr-server.tspackages/router-core/tests/ssr-server-manifest.test.tspackages/router-plugin/src/core/router-composed-plugin.tspackages/solid-router/src/Asset.tsxpackages/solid-router/src/headContentUtils.tsxpackages/start-plugin-core/package.jsonpackages/start-plugin-core/src/rsbuild/normalized-client-build.tspackages/start-plugin-core/src/rsbuild/plugin.tspackages/start-plugin-core/src/rsbuild/virtual-modules.tspackages/start-plugin-core/src/schema.tspackages/start-plugin-core/src/start-manifest-plugin/inlineCss.tspackages/start-plugin-core/src/start-manifest-plugin/manifestBuilder.tspackages/start-plugin-core/src/types.tspackages/start-plugin-core/src/vite/planning.tspackages/start-plugin-core/src/vite/plugin.tspackages/start-plugin-core/src/vite/start-manifest-plugin/normalized-client-build.tspackages/start-plugin-core/src/vite/start-manifest-plugin/plugin.tspackages/start-plugin-core/tests/start-manifest-plugin/manifestBuilder.test.tspackages/start-server-core/src/router-manifest.tspackages/start-server-core/src/transformAssetUrls.tspackages/vue-router/src/Asset.tsxpackages/vue-router/src/headContentUtils.tsx
✅ Files skipped from review due to trivial changes (18)
- e2e/react-start/css-inline/src/styles/dashboard-details.module.css
- packages/start-plugin-core/package.json
- e2e/react-start/css-inline/src/styles/dashboard-layout.module.css
- .changeset/smooth-cobras-inline.md
- e2e/react-start/css-inline/src/styles/dashboard-index.module.css
- e2e/react-start/css-inline/src/components/NestedPanel.tsx
- e2e/react-start/css-inline/src/styles/shell.module.css
- e2e/react-start/css-inline/src/styles/app-layout.module.css
- e2e/react-start/css-inline/src/router.tsx
- e2e/react-start/css-inline/src/routes/app/dashboard/route.tsx
- e2e/react-start/css-inline/tsconfig.json
- e2e/react-start/css-inline/rsbuild.config.ts
- e2e/react-start/css-inline/src/styles/nested-panel.module.css
- e2e/react-start/css-inline/package.json
- e2e/react-start/css-inline/vite.config.ts
- e2e/react-start/css-inline/src/routeTree.gen.ts
- packages/solid-router/src/headContentUtils.tsx
- packages/start-plugin-core/tests/start-manifest-plugin/manifestBuilder.test.ts
🚧 Files skipped from review as they are similar to previous changes (14)
- packages/start-plugin-core/src/schema.ts
- packages/start-plugin-core/src/types.ts
- packages/router-plugin/src/core/router-composed-plugin.ts
- e2e/react-start/css-inline/src/routes/app/route.tsx
- packages/start-plugin-core/src/vite/start-manifest-plugin/plugin.ts
- e2e/react-start/css-inline/src/components/Shell.tsx
- packages/router-core/src/index.ts
- e2e/react-start/rsc/tests/rsc-deferred-component-flash.spec.ts
- e2e/react-start/css-inline/src/routes/index.tsx
- e2e/react-start/css-inline/server.js
- e2e/react-start/css-inline/src/routes/app/dashboard/index.tsx
- packages/start-plugin-core/src/start-manifest-plugin/manifestBuilder.ts
- packages/router-core/src/ssr/ssr-server.ts
- packages/start-plugin-core/src/rsbuild/virtual-modules.ts
| const relTokens = typeof rel === 'string' ? rel.split(/\s+/) : [] | ||
| if (!relTokens.includes('stylesheet')) return undefined |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "manifest.ts" -path "*/router-core/*" | head -20Repository: TanStack/router
Length of output: 98
🏁 Script executed:
find . -name "manifest.ts" | grep -i routerRepository: TanStack/router
Length of output: 98
🏁 Script executed:
fd -i manifest.tsRepository: TanStack/router
Length of output: 146
🏁 Script executed:
git ls-files | grep manifest.tsRepository: TanStack/router
Length of output: 279
🏁 Script executed:
ls -la packages/router-core/src/ 2>/dev/null | head -20 || echo "Directory not found, listing packages"Repository: TanStack/router
Length of output: 1258
🏁 Script executed:
ls -la packages/ 2>/dev/null | head -20 || echo "Packages directory not found"Repository: TanStack/router
Length of output: 1290
🏁 Script executed:
pwd && git ls-files | head -30Repository: TanStack/router
Length of output: 806
🌐 Web query:
HTML link rel attribute case sensitivity RFC spec
💡 Result:
The HTML link rel attribute is case-insensitive according to the HTML Standard (WHATWG). Keywords in the rel attribute are always ASCII case-insensitive and must be compared as such. For example, rel="stylesheet" is equivalent to rel="STYLESHEET". RFC 8288 (Web Linking), which defines the HTTP Link header (related to HTML link rel), specifies that registered relation types are compared character by character in a case-insensitive fashion. It recommends serializing registered types in lowercase. No RFC defines HTML parsing rules; HTML follows the WHATWG HTML Living Standard as the authoritative specification. MDN confirms rel values are case-insensitive.
Citations:
- 1: https://datatracker.ietf.org/doc/html/rfc8288
- 2: https://w3.org/TR/html5/links.html
- 3: https://html.spec.whatwg.org/C/links.html
- 4: https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Attributes/rel
- 5: https://html.spec.whatwg.org/dev/links.html
🏁 Script executed:
git ls-files | grep -E '\.(ts|tsx|js)$' | head -50Repository: TanStack/router
Length of output: 3233
🏁 Script executed:
find . -type f -name "*.ts" | grep -i manifest | head -20Repository: TanStack/router
Length of output: 1278
🏁 Script executed:
cat -n packages/router-core/src/manifest.ts | sed -n '70,95p'Repository: TanStack/router
Length of output: 949
Convert rel token to lowercase before checking for stylesheet match.
Line 82 checks only the lowercase string 'stylesheet'. Per the HTML specification (WHATWG), link relation tokens are ASCII case-insensitive. A rel attribute with value "StyleSheet" or "STYLESHEET" would fail to match.
Proposed fix
- const relTokens = typeof rel === 'string' ? rel.split(/\s+/) : []
+ const relTokens =
+ typeof rel === 'string'
+ ? rel
+ .toLowerCase()
+ .split(/\s+/)
+ .filter(Boolean)
+ : []📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const relTokens = typeof rel === 'string' ? rel.split(/\s+/) : [] | |
| if (!relTokens.includes('stylesheet')) return undefined | |
| const relTokens = | |
| typeof rel === 'string' | |
| ? rel | |
| .toLowerCase() | |
| .split(/\s+/) | |
| .filter(Boolean) | |
| : [] | |
| if (!relTokens.includes('stylesheet')) return undefined |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/router-core/src/manifest.ts` around lines 81 - 82, The rel token
comparison is case-sensitive; convert the rel string to lowercase before
splitting/checking so ASCII case-insensitive matches succeed: when computing
relTokens from rel (the variable used in this module, near the check using
relTokens.includes('stylesheet')), normalize rel to lowercase (e.g., via
toLowerCase()) before splitting so tokens like "StyleSheet" or "STYLESHEET" will
match 'stylesheet'.
| visitor: { | ||
| Url(url) { | ||
| return { | ||
| ...url, | ||
| url: rebaseCssUrl(url.url, options.cssHref), | ||
| } | ||
| }, | ||
| Rule: { | ||
| import(rule: any) { | ||
| return { | ||
| ...rule, | ||
| value: { | ||
| ...rule.value, | ||
| url: rebaseCssUrl(rule.value.url, options.cssHref), | ||
| }, | ||
| } | ||
| }, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "inlineCss.ts" -type fRepository: TanStack/router
Length of output: 127
🏁 Script executed:
cat -n ./packages/start-plugin-core/src/start-manifest-plugin/inlineCss.tsRepository: TanStack/router
Length of output: 2866
🏁 Script executed:
find . -name "tsconfig.json" -type f | head -5Repository: TanStack/router
Length of output: 266
🏁 Script executed:
find ./packages -name "tsconfig.json" -type fRepository: TanStack/router
Length of output: 1896
🏁 Script executed:
cat ./packages/start-plugin-core/tsconfig.jsonRepository: TanStack/router
Length of output: 307
🏁 Script executed:
cat ./tsconfig.jsonRepository: TanStack/router
Length of output: 847
🏁 Script executed:
cd ./packages/start-plugin-core && cat package.json | grep -A 5 -B 5 lightningcssRepository: TanStack/router
Length of output: 395
🌐 Web query:
lightningcss visitor transform Rule import types TypeScript
💡 Result:
LightningCSS provides full TypeScript support through built-in type definitions in the node/index.d.ts and node/ast.d.ts files included in the npm package. No separate @types/lightningcss package is needed. To use visitor transforms with Rule types in TypeScript: 1. Install LightningCSS: npm install --save-dev lightningcss 2. Import types and functions: import { transform } from 'lightningcss'; import type { Visitor, RuleVisitor, RuleVisitors, ImportRule, StyleRule /* other Rule types / } from 'lightningcss'; 3. Define a visitor for Rule, using either a general RuleVisitor or specific RuleVisitors: const visitor: Visitor = { Rule: { // For specific rule types like @import import: ((rule: ImportRule) => { // Transform logic, e.g., return [] to remove, or modified rule console.log(Removing import: ${rule.value.url}); return undefined; // removes the rule }) as RuleVisitor, // For style rules style: ((rule: StyleRule) => { // e.g., modify declarations return rule; }) as RuleVisitor, // Custom at-rules custom: { mixin: (rule) => { / ... */ } } } }; 4. Pass to transform: const result = transform({ filename: 'input.css', code: Buffer.from(cssCode), visitor }); The TypeScript definitions document all AST node types (e.g., ImportRule, StyleRule) and visitor interfaces. RuleVisitors supports keys like 'import', 'style', 'media', 'custom', etc. Visitors can return the modified rule, an array of rules, or void/undefined to remove. Use composeVisitors to combine multiple visitors. Official docs recommend checking TypeScript definitions for full visitor functions list: https://lightningcss.dev/transforms.html
Citations:
- 1: https://lightningcss.dev/transforms.html
- 2: https://tessl.io/registry/tessl/npm-lightningcss/1.30.0/files/docs/visitors.md
- 3: https://github.com/parcel-bundler/lightningcss/blob/master/node/index.d.ts
- 4: https://lightningcss.dev/docs.html
- 5: https://www.npmjs.com/package/lightningcss
🏁 Script executed:
cd ./packages/start-plugin-core && npm list lightningcss 2>/dev/null | head -5Repository: TanStack/router
Length of output: 154
🌐 Web query:
lightningcss ImportRule type definition TypeScript
💡 Result:
The LightningCSS ImportRule TypeScript type definition is part of the library's AST (Abstract Syntax Tree) types, exported from 'lightningcss/ast'. ImportRule is referenced in the Rule union type as: type Rule<D = Declaration, M = MediaQuery> = ... | { type: "import"; value: ImportRule; } ... The full interface definition for ImportRule is available in the official TypeScript declarations at https://github.com/parcel-bundler/lightningcss/blob/master/node/ast.d.ts. To use it: import type { ImportRule, MediaQuery } from "lightningcss/ast"; Example: ImportRule represents an @import rule with its URL, optional media queries, supports conditions, and location. The Rust equivalent shows ImportRule<'i> as a variant in CssRule enum, confirming it holds the parsed @import URL, media, and supports. Official docs recommend checking TypeScript definitions for complete AST documentation: https://lightningcss.dev/transforms.html ("See the TypeScript definitions for full documentation of all values.")
Citations:
- 1: https://github.com/parcel-bundler/lightningcss/blob/master/node/index.d.ts
- 2: https://github.com/parcel-bundler/lightningcss/blob/master/node/ast.d.ts
- 3: https://cdn.jsdelivr.net/npm/lightningcss@1.31.1/node/ast.d.ts
- 4: https://docs.rs/lightningcss/latest/lightningcss/rules/enum.CssRule.html
- 5: https://lightningcss.dev/bundling.html
- 6: https://lightningcss.dev/transforms.html
- 7: https://lightningcss.dev/docs.html
Remove any type annotation from the Rule.import visitor callback.
Line 72 uses rule: any, which violates the project's strict TypeScript mode requirement. The lightningcss library provides proper types for this callback; use the typed ImportRule instead.
Suggested typed rewrite
- const result = transform({
+ const visitor = {
+ Url(url) {
+ return {
+ ...url,
+ url: rebaseCssUrl(url.url, options.cssHref),
+ }
+ },
+ Rule: {
+ import(rule) {
+ return {
+ ...rule,
+ value: {
+ ...rule.value,
+ url: rebaseCssUrl(rule.value.url, options.cssHref),
+ },
+ }
+ },
+ },
+ } satisfies NonNullable<Parameters<typeof transform>[0]['visitor']>
+
+ const result = transform({
filename: options.cssHref,
code: Buffer.from(css),
minify: true,
- visitor: {
- Url(url) {
- return {
- ...url,
- url: rebaseCssUrl(url.url, options.cssHref),
- }
- },
- Rule: {
- import(rule: any) {
- return {
- ...rule,
- value: {
- ...rule.value,
- url: rebaseCssUrl(rule.value.url, options.cssHref),
- },
- }
- },
- },
- },
+ visitor,
})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/start-plugin-core/src/start-manifest-plugin/inlineCss.ts` around
lines 64 - 82, Replace the untyped parameter in the visitor Rule.import
callback: change the signature from "rule: any" to use LightningCSS's ImportRule
type (import it from the lightningcss AST types), update the callback to accept
"rule: ImportRule", and ensure any property access (rule.value.url) remains
valid; locate the visitor object (visitor -> Rule -> import) where
rebaseCssUrl(rule.value.url, options.cssHref) is called and update imports
accordingly so the file compiles under strict TypeScript.
| ? (document.querySelector<HTMLStyleElement>( | ||
| `style[${INLINE_CSS_HYDRATION_ATTR}]`, | ||
| )?.textContent ?? '') |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/vue-router/src/Asset.tsx | head -210 | tail -30Repository: TanStack/router
Length of output: 1080
🏁 Script executed:
# Search for InlineCssStyle class definition and INLINE_CSS_HYDRATION_ATTR
rg -n "class InlineCssStyle|INLINE_CSS_HYDRATION_ATTR" packages/vue-router/src/Asset.tsxRepository: TanStack/router
Length of output: 217
🏁 Script executed:
# Get full context of the file to understand structure
wc -l packages/vue-router/src/Asset.tsxRepository: TanStack/router
Length of output: 97
🏁 Script executed:
# View the full component definition including its name
sed -n '170,210p' packages/vue-router/src/Asset.tsxRepository: TanStack/router
Length of output: 1056
🏁 Script executed:
# Search for how this component is used/exported
rg -n "defineComponent|export.*InlineCSS" packages/vue-router/src/Asset.tsxRepository: TanStack/router
Length of output: 187
🏁 Script executed:
# Look for where this component or Asset function is used in the codebase
rg -n "InlineCssStyle|InlineCSS" --max-count 20Repository: TanStack/router
Length of output: 1079
🏁 Script executed:
# Check createInlineCssStyleAsset implementation in manifest.ts
sed -n '95,110p' packages/router-core/src/manifest.tsRepository: TanStack/router
Length of output: 418
🏁 Script executed:
# Look at ssr-server.ts to see how multiple CSS assets are handled
rg -n "createInlineCssStyleAsset" packages/router-core/src/ssr/ssr-server.ts -A 3 -B 3Repository: TanStack/router
Length of output: 515
🏁 Script executed:
# Check if there are multiple CSS entries per page or request
rg -n "createInlineCssStyleAsset|inlineCss" packages/router-core/src/ssr/ssr-server.ts | head -30Repository: TanStack/router
Length of output: 705
🏁 Script executed:
# Check how requestAssets is rendered and if multiple InlineCssStyle components can exist
rg -n "requestAssets" packages/vue-router/src/Asset.tsx -B 5 -A 5Repository: TanStack/router
Length of output: 41
🏁 Script executed:
# Look at the Asset function that uses InlineCssStyle
sed -n '209,235p' packages/vue-router/src/Asset.tsxRepository: TanStack/router
Length of output: 786
🏁 Script executed:
# Search for how many style tags might be rendered in SSR context
rg -n "inlineCss|InlineCssStyle" packages/router-core/src/ssr/ssr-server.ts -B 2 -A 2 | head -50Repository: TanStack/router
Length of output: 1506
🏁 Script executed:
# View createInlineCssPlaceholderAsset implementation
sed -n '100,120p' packages/router-core/src/manifest.tsRepository: TanStack/router
Length of output: 297
🏁 Script executed:
# Search for usage of createInlineCssPlaceholderAsset
rg -n "createInlineCssPlaceholderAsset" packages/router-core/src/ssr/ssr-server.ts -B 3 -A 3Repository: TanStack/router
Length of output: 531
🏁 Script executed:
# Check the full stripInlinedStylesheetAssets function to understand replacement logic
sed -n '243,280p' packages/router-core/src/ssr/ssr-server.tsRepository: TanStack/router
Length of output: 951
🏁 Script executed:
# Check if multiple CSS assets could be created per request
rg -n "getInlineCssAssetForMatches" packages/router-core/src/ssr/ssr-server.ts -B 5 -A 10Repository: TanStack/router
Length of output: 1472
🏁 Script executed:
# Look at the full SSR rendering flow to see how assets are ordered
sed -n '285,320p' packages/router-core/src/ssr/ssr-server.tsRepository: TanStack/router
Length of output: 1170
🏁 Script executed:
# Check if there's any case where multiple style tags with inlineCss=true can be created
rg -n "inlineCss.*true" packages/router-core/src/ssr/ssr-server.tsRepository: TanStack/router
Length of output: 41
🏁 Script executed:
# Verify: Is createInlineCssStyleAsset called multiple times per request?
rg -n "createInlineCssStyleAsset" packages/router-core/src/ssr/ssr-server.tsRepository: TanStack/router
Length of output: 166
🏁 Script executed:
# Check if getRequestAssets could provide multiple inlineCss style assets
rg -n "getRequestAssets" packages/router-core/src/ -A 5 -B 5 | head -50Repository: TanStack/router
Length of output: 1617
🏁 Script executed:
# Look for any SPA or dynamic loading patterns that might create multiple CSS assets
rg -n "inlineCss|getInlineCss" packages/router-core/src/ssr/ssr-server.ts | head -20Repository: TanStack/router
Length of output: 1130
🏁 Script executed:
# Check if there's any actual scenario where multiple InlineCssStyle components render
# with the same data-tsr-inline-css attribute and query for each other's content
rg -n "data-allow-mismatch|suppressHydrationWarning" packages/vue-router/src/Asset.tsx -B 2 -A 2Repository: TanStack/router
Length of output: 752
🏁 Script executed:
# Verify the complete hydration flow - check if placeholder actually queries or if actual CSS queries
sed -n '188,206p' packages/vue-router/src/Asset.tsxRepository: TanStack/router
Length of output: 663
🏁 Script executed:
# Check if TypeScript strict mode would catch any type issues with the querySelector usage
sed -n '192,194p' packages/vue-router/src/Asset.tsxRepository: TanStack/router
Length of output: 195
The selector strategy is fragile and non-unique, but current code flow mitigates the risk.
Lines 192-194 use a non-unique selector (style[data-tsr-inline-css]) that would return the first match across multiple tags. However, the actual risk is limited because only placeholder instances (where children === undefined) call querySelector. The actual CSS asset uses props.children directly without querying. Since the standard flow creates at most one placeholder and one CSS asset per request, the placeholder correctly finds itself.
To improve robustness, the selector should be instance-specific rather than attribute-based. Consider using nth-of-type() or an index-based data attribute so each instance can reliably find its own content.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/vue-router/src/Asset.tsx` around lines 192 - 194, The selector in
Asset.tsx that uses `style[${INLINE_CSS_HYDRATION_ATTR}]` is fragile; update the
placeholder rendering and lookup so each instance is uniquely identifiable
(e.g., add an instance index or data-id attribute when rendering the
placeholder/CSS asset and use that attribute in the querySelector instead of the
generic `INLINE_CSS_HYDRATION_ATTR`), so the placeholder branch (where `children
=== undefined`) locates its own style tag reliably; adjust the code paths that
set/read the attribute (search for INLINE_CSS_HYDRATION_ATTR, the placeholder
logic, and the CSS asset render in this file) to propagate and consume the
instance-specific identifier (or use `nth-of-type()` keyed by that index) so
lookups are deterministic.
661df15 to
0b7a745
Compare
1 participant
Summary by CodeRabbit
New Features
Tests
Chores