-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
fix: properly handle sass in dev styles #6372
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughThis PR adds Sass mixin pages and tests for React/Solid/Vue e2e projects and refactors the dev-server style collection: adds CSS-modules detection/cache, parallelized traversal, CSS extraction, and a pre-phase Changes
Sequence DiagramsequenceDiagram
participant Client
participant DevServer as Dev Server (pre-phase)
participant Vite as Vite Dev Server / Module Graph
participant Cache as CSS Modules Cache
participant Collector as collectDevStyles
Client->>DevServer: GET /@tanstack-start/styles.css?routes=...
DevServer->>DevServer: parse route IDs, resolve entries (TSS_ROUTES_MANIFEST)
DevServer->>Collector: collectDevStyles(entries, cssModulesCache)
Collector->>Vite: transformRequest(module) (parallel)
Vite-->>Cache: transform hook stores CSS module raw output
Collector->>Collector: processEntry → findModuleDeps (traverse importedModules)
loop per dependency
alt CSS Module
Collector->>Cache: read cached CSS by normalized key
Cache-->>Collector: return CSS
else Regular CSS
Collector->>Vite: fetch module code / transformed result
Vite-->>Collector: return CSS code
end
Collector->>Collector: extract/escape CSS, aggregate
end
Collector-->>DevServer: aggregated CSS
DevServer-->>Client: 200 text/css (styles.css)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
|
View your CI Pipeline Execution ↗ for commit 8db8b0e
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @packages/start-plugin-core/src/dev-server-plugin/dev-styles.ts:
- Around line 178-219: findModuleDeps races when spawning parallel branches
because visitNodeAndRecurse/resolveUrlAndVisit test visited non-atomically; fix
by making the visited check-and-add atomic inside findModuleDeps: before pushing
a branch for a depUrl (resolveUrlAndVisit) or a depNode (visitNodeAndRecurse),
do if (!visited.has(key)) { visited.add(key); branches.push(...) } where key is
the depUrl string for deps and the ModuleNode object for importedModules; this
ensures no two branches are scheduled for the same module and preserves the
caller semantics around the visited Set.
🧹 Nitpick comments (7)
e2e/vue-start/css-modules/src/styles/app.scss (1)
1-4: Consider using modern Sass@use/@forwardsyntax.The
@importrule is deprecated in Dart Sass and will eventually be removed. For new files, consider using@useand@forwardfor better encapsulation and namespace control.That said, if this e2e test intentionally validates
@importbehavior for backward compatibility, the current approach is acceptable.💡 Optional: Modern Sass syntax
-// Import the mixin, which should make them available to future imports -@import './center-mixin.scss'; - -@import './mixin-consumer.scss'; +// Forward the mixin for use in other files +@forward './center-mixin.scss'; + +@use './mixin-consumer.scss';Note: This would require updating
mixin-consumer.scssto use@use '../center-mixin.scss' as *;or similar.e2e/solid-start/css-modules/src/styles/mixin-consumer.scss (1)
2-4: Consider updating or removing the stale comment.The comment states this "works for the client build, but not for the dev server styles.css build." Since this PR aims to fix Sass handling in dev styles, this comment may become misleading after the fix is merged. Consider updating it to reflect the fixed behavior or removing it entirely.
Suggested change
.mixin-container { - // Use the center mixin, which is included in app.scss - // This works for the client build, but not for the dev server styles.css build + // Use the center mixin, which is included in app.scss @include center-mixin;e2e/react-start/css-modules/src/styles/mixin-consumer.scss (1)
2-4: Consider updating or removing the stale comment.Same feedback as the Solid Start version: the comment stating this doesn't work for dev server styles.css build may become misleading after this PR is merged. Consider updating or removing it.
Suggested change
.mixin-container { - // Use the center mixin, which is included in app.scss - // This works for the client build, but not for the dev server styles.css build + // Use the center mixin, which is included in app.scss @include center-mixin;e2e/vue-start/css-modules/src/styles/mixin-consumer.scss (1)
2-4: Consider updating the comment to reflect the fix.The comment states this "works for the client build, but not for the dev server styles.css build" — however, this PR is specifically fixing that issue. Once merged, the comment will be misleading since the mixin should work correctly in dev mode SSR.
Consider updating to something like:
-.mixin-container { - // Use the center mixin, which is included in app.scss - // This works for the client build, but not for the dev server styles.css build - @include center-mixin; +.mixin-container { + // Use the center mixin, which is defined in center-mixin.scss and imported via app.scss + @include center-mixin;packages/start-plugin-core/src/dev-server-plugin/plugin.ts (1)
84-95: Consider adding type safety for the global manifest access.The
globalThiscast toanyloses type safety. While the comment correctly notes that only registered routes are used (preventing path injection), the implementation could benefit from stricter typing.💡 Optional: Define the manifest type separately
+interface RouteManifestEntry { + filePath: string + children?: Array<string> +} + +declare global { + // eslint-disable-next-line no-var + var TSS_ROUTES_MANIFEST: Record<string, RouteManifestEntry> | undefined +} - const routesManifest = (globalThis as any).TSS_ROUTES_MANIFEST as - | Record<string, { filePath: string; children?: Array<string> }> - | undefined + const routesManifest = globalThis.TSS_ROUTES_MANIFESTpackages/start-plugin-core/src/dev-server-plugin/dev-styles.ts (2)
103-112: Throwing on missing CSS module cache may cause cascading failures.If a CSS module is not in the cache (e.g., due to a race condition during initial load or a transform error), this throws and aborts the entire style collection. Consider whether a warning with graceful degradation would be more robust for development experience.
💡 Consider graceful degradation
if (dep.file && isCssModulesFile(dep.file)) { const css = cssModulesCache[normalizeCssModuleCacheKey(dep.file)] if (!css) { - throw new Error( - `[tanstack-start] Missing CSS module in cache: ${dep.file}`, - ) + console.warn( + `[tanstack-start] Missing CSS module in cache: ${dep.file}. ` + + `This may happen during initial load - try refreshing.`, + ) + continue } styles.set(dep.url, css) continue }
264-273: CSS extraction may fail on escaped characters not covered.The
extractCssFromCodefunction handles\n,\t,\", and\\, but may miss other escape sequences like\'(single quote) or unicode escapes (\uXXXX). Vite's output format may include these depending on CSS content.💡 Consider using JSON.parse for more robust unescaping
function extractCssFromCode(code: string): string | undefined { const match = VITE_CSS_REGEX.exec(code) if (!match?.[1]) return undefined - return match[1] - .replace(/\\n/g, '\n') - .replace(/\\t/g, '\t') - .replace(/\\"/g, '"') - .replace(/\\\\/g, '\\') + try { + // Use JSON.parse to handle all escape sequences correctly + return JSON.parse(`"${match[1]}"`) + } catch { + // Fall back to manual unescaping if JSON.parse fails + return match[1] + .replace(/\\n/g, '\n') + .replace(/\\t/g, '\t') + .replace(/\\"/g, '"') + .replace(/\\'/g, "'") + .replace(/\\\\/g, '\\') + } }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (24)
e2e/react-start/css-modules/package.jsone2e/react-start/css-modules/src/routes/__root.tsxe2e/react-start/css-modules/src/routes/sass-mixin.tsxe2e/react-start/css-modules/src/styles/app.scsse2e/react-start/css-modules/src/styles/center-mixin.scsse2e/react-start/css-modules/src/styles/mixin-consumer.scsse2e/react-start/css-modules/tests/css.spec.tse2e/react-start/css-modules/vite.config.tse2e/solid-start/css-modules/package.jsone2e/solid-start/css-modules/src/routes/__root.tsxe2e/solid-start/css-modules/src/routes/sass-mixin.tsxe2e/solid-start/css-modules/src/styles/app.scsse2e/solid-start/css-modules/src/styles/center-mixin.scsse2e/solid-start/css-modules/src/styles/mixin-consumer.scsse2e/solid-start/css-modules/tests/css.spec.tse2e/vue-start/css-modules/package.jsone2e/vue-start/css-modules/src/routes/__root.tsxe2e/vue-start/css-modules/src/routes/sass-mixin.tsxe2e/vue-start/css-modules/src/styles/app.scsse2e/vue-start/css-modules/src/styles/center-mixin.scsse2e/vue-start/css-modules/src/styles/mixin-consumer.scsse2e/vue-start/css-modules/tests/css.spec.tspackages/start-plugin-core/src/dev-server-plugin/dev-styles.tspackages/start-plugin-core/src/dev-server-plugin/plugin.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript strict mode with extensive type safety for all code
Files:
e2e/react-start/css-modules/vite.config.tse2e/vue-start/css-modules/src/routes/sass-mixin.tsxe2e/solid-start/css-modules/src/routes/__root.tsxe2e/solid-start/css-modules/tests/css.spec.tse2e/vue-start/css-modules/tests/css.spec.tse2e/react-start/css-modules/tests/css.spec.tse2e/react-start/css-modules/src/routes/__root.tsxpackages/start-plugin-core/src/dev-server-plugin/dev-styles.tse2e/react-start/css-modules/src/routes/sass-mixin.tsxe2e/vue-start/css-modules/src/routes/__root.tsxe2e/solid-start/css-modules/src/routes/sass-mixin.tsxpackages/start-plugin-core/src/dev-server-plugin/plugin.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Implement ESLint rules for router best practices using the ESLint plugin router
Files:
e2e/react-start/css-modules/vite.config.tse2e/vue-start/css-modules/src/routes/sass-mixin.tsxe2e/solid-start/css-modules/src/routes/__root.tsxe2e/solid-start/css-modules/tests/css.spec.tse2e/vue-start/css-modules/tests/css.spec.tse2e/react-start/css-modules/tests/css.spec.tse2e/react-start/css-modules/src/routes/__root.tsxpackages/start-plugin-core/src/dev-server-plugin/dev-styles.tse2e/react-start/css-modules/src/routes/sass-mixin.tsxe2e/vue-start/css-modules/src/routes/__root.tsxe2e/solid-start/css-modules/src/routes/sass-mixin.tsxpackages/start-plugin-core/src/dev-server-plugin/plugin.ts
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Use workspace protocol
workspace:*for internal dependencies in package.json files
Files:
e2e/vue-start/css-modules/package.jsone2e/solid-start/css-modules/package.jsone2e/react-start/css-modules/package.json
🧠 Learnings (8)
📓 Common learnings
Learnt from: nlynzaad
Repo: TanStack/router PR: 6215
File: e2e/react-start/custom-basepath/package.json:13-17
Timestamp: 2025-12-25T13:04:55.492Z
Learning: In the TanStack Router repository, e2e test scripts are specifically designed to run in CI (which uses a Unix environment), so Unix-specific commands (like `rm -rf`, `&` for backgrounding, and direct environment variable assignments without `cross-env`) are acceptable in e2e test npm scripts.
📚 Learning: 2025-10-08T08:11:47.088Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5402
File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21
Timestamp: 2025-10-08T08:11:47.088Z
Learning: Test snapshot files in the router-generator tests directory (e.g., files matching the pattern `packages/router-generator/tests/generator/**/routeTree*.snapshot.ts` or `routeTree*.snapshot.js`) should not be modified or have issues flagged, as they are fixtures used to verify the generator's output and are intentionally preserved as-is.
Applied to files:
e2e/vue-start/css-modules/src/routes/sass-mixin.tsxe2e/react-start/css-modules/src/routes/sass-mixin.tsx
📚 Learning: 2025-12-17T02:17:55.086Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 6120
File: packages/router-generator/src/generator.ts:654-657
Timestamp: 2025-12-17T02:17:55.086Z
Learning: In `packages/router-generator/src/generator.ts`, pathless_layout routes must receive a `path` property when they have a `cleanedPath`, even though they are non-path routes. This is necessary because child routes inherit the path from their parent, and without this property, child routes would not have the correct full path at runtime.
Applied to files:
e2e/vue-start/css-modules/src/routes/sass-mixin.tsxe2e/react-start/css-modules/src/routes/sass-mixin.tsxe2e/solid-start/css-modules/src/routes/sass-mixin.tsx
📚 Learning: 2025-12-06T15:03:07.223Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T15:03:07.223Z
Learning: Use file-based routing in `src/routes/` directories or code-based routing with route definitions
Applied to files:
e2e/vue-start/css-modules/src/routes/sass-mixin.tsxe2e/react-start/css-modules/src/routes/sass-mixin.tsxe2e/solid-start/css-modules/src/routes/sass-mixin.tsx
📚 Learning: 2025-12-25T13:04:55.492Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 6215
File: e2e/react-start/custom-basepath/package.json:13-17
Timestamp: 2025-12-25T13:04:55.492Z
Learning: In the TanStack Router repository, e2e test scripts are specifically designed to run in CI (which uses a Unix environment), so Unix-specific commands (like `rm -rf`, `&` for backgrounding, and direct environment variable assignments without `cross-env`) are acceptable in e2e test npm scripts.
Applied to files:
e2e/vue-start/css-modules/package.jsone2e/solid-start/css-modules/package.json
📚 Learning: 2025-10-01T18:31:35.420Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 5330
File: e2e/react-start/custom-basepath/src/routeTree.gen.ts:58-61
Timestamp: 2025-10-01T18:31:35.420Z
Learning: Do not review files named `routeTree.gen.ts` in TanStack Router repositories, as these are autogenerated files that should not be manually modified.
Applied to files:
e2e/react-start/css-modules/src/routes/sass-mixin.tsx
📚 Learning: 2025-12-06T15:03:07.223Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T15:03:07.223Z
Learning: Applies to **/*.{js,ts,tsx} : Implement ESLint rules for router best practices using the ESLint plugin router
Applied to files:
e2e/react-start/css-modules/src/routes/sass-mixin.tsxe2e/solid-start/css-modules/src/routes/sass-mixin.tsx
📚 Learning: 2025-11-02T16:16:24.898Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5732
File: packages/start-client-core/src/client/hydrateStart.ts:6-9
Timestamp: 2025-11-02T16:16:24.898Z
Learning: In packages/start-client-core/src/client/hydrateStart.ts, the `import/no-duplicates` ESLint disable is necessary for imports from `#tanstack-router-entry` and `#tanstack-start-entry` because both aliases resolve to the same placeholder file (`fake-start-entry.js`) in package.json during static analysis, even though they resolve to different files at runtime.
Applied to files:
packages/start-plugin-core/src/dev-server-plugin/plugin.ts
🧬 Code graph analysis (7)
e2e/vue-start/css-modules/src/routes/sass-mixin.tsx (2)
e2e/react-start/css-modules/src/routes/sass-mixin.tsx (1)
Route(4-6)e2e/vue-start/css-modules/src/routes/__root.tsx (1)
Route(11-19)
e2e/solid-start/css-modules/tests/css.spec.ts (1)
e2e/e2e-utils/src/fixture.ts (1)
test(18-48)
e2e/vue-start/css-modules/tests/css.spec.ts (1)
e2e/e2e-utils/src/fixture.ts (1)
test(18-48)
e2e/react-start/css-modules/tests/css.spec.ts (1)
e2e/e2e-utils/src/fixture.ts (1)
test(18-48)
e2e/react-start/css-modules/src/routes/sass-mixin.tsx (3)
e2e/solid-start/css-modules/src/routes/sass-mixin.tsx (1)
Route(4-6)e2e/vue-start/css-modules/src/routes/sass-mixin.tsx (1)
Route(4-6)packages/router-core/src/ssr/tsrScript.ts (1)
p(16-18)
e2e/solid-start/css-modules/src/routes/sass-mixin.tsx (3)
e2e/react-start/css-modules/src/routes/sass-mixin.tsx (1)
Route(4-6)e2e/solid-start/css-modules/src/routes/__root.tsx (1)
Route(10-18)e2e/vue-start/css-modules/src/routes/sass-mixin.tsx (1)
Route(4-6)
packages/start-plugin-core/src/dev-server-plugin/plugin.ts (1)
packages/start-plugin-core/src/dev-server-plugin/dev-styles.ts (3)
CSS_MODULES_REGEX(13-14)normalizeCssModuleCacheKey(16-19)collectDevStyles(79-145)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test
- GitHub Check: Preview
🔇 Additional comments (30)
e2e/react-start/css-modules/vite.config.ts (1)
25-29: LGTM! Clear documentation of the test scenario.The comment explaining why Nitro is placed before
tanstackStartis helpful for understanding the test intent. The plugin reordering to verify CSS middleware robustness regardless of plugin order is a sensible e2e test strategy.e2e/react-start/css-modules/src/styles/center-mixin.scss (1)
1-5: LGTM - Clean, reusable SCSS mixin.The mixin follows SCSS best practices with a focused, single-responsibility design for flexbox centering. This serves its purpose well for testing Sass mixin functionality in the e2e suite.
e2e/solid-start/css-modules/src/routes/__root.tsx (1)
53-59: LGTM - Navigation link follows established patterns.The new "Sass Mixin" link correctly follows the existing styling conventions and
data-testidnaming pattern (nav-sass-mixin). The kebab-case CSS properties in the style object are idiomatic for Solid.js.e2e/react-start/css-modules/package.json (1)
43-43: LGTM - Sass correctly added as devDependency.The sass package is appropriately placed in devDependencies since it's a build-time preprocessor for the e2e test project.
e2e/react-start/css-modules/src/routes/__root.tsx (1)
51-57: LGTM - Navigation link follows established patterns.The new "Sass Mixin" link correctly follows the existing styling conventions and
data-testidnaming pattern. The camelCase CSS properties (textDecoration) are idiomatic for React's style objects.e2e/vue-start/css-modules/package.json (1)
26-26: Sass devDependency correctly added. The sass package is appropriately placed in devDependencies for this e2e test project, and the version constraint^1.97.2is valid.e2e/vue-start/css-modules/src/styles/center-mixin.scss (1)
1-5: LGTM!Clean and focused mixin definition using standard flexbox centering. The implementation is correct and reusable.
e2e/react-start/css-modules/src/styles/app.scss (1)
1-4: LGTM! Consistent with Vue implementation.The structure mirrors the Vue e2e test's
app.scss, maintaining consistency across frameworks. The same optional consideration for modern@use/@forwardsyntax applies here as noted in the Vue file review.e2e/vue-start/css-modules/src/routes/__root.tsx (1)
53-59: LGTM!The new navigation link follows the established patterns:
- Consistent styling with existing links
- Proper
data-testidfor e2e test targeting- Route path aligns with the new
/sass-mixinroutee2e/solid-start/css-modules/package.json (1)
25-25: LGTM! Sass dependency added correctly.The addition of
sassas a devDependency enables Sass support for the CSS modules e2e tests. The version 1.97.2 is current (published January 6, 2026) with no security advisories. Internal dependencies correctly use the workspace protocol per coding guidelines.e2e/solid-start/css-modules/src/styles/app.scss (1)
1-4: LGTM - Import order is correct for mixin availability.The mixin is imported before its consumer, ensuring proper Sass compilation. Note that
@importis deprecated in Sass in favor of@use/@forward, but for e2e test purposes this is acceptable.e2e/solid-start/css-modules/src/styles/center-mixin.scss (1)
1-5: LGTM - Clean and reusable flexbox centering mixin.The mixin correctly implements standard flexbox centering properties.
e2e/solid-start/css-modules/src/routes/sass-mixin.tsx (2)
1-6: LGTM - Route definition follows TanStack file-based routing conventions.The route setup is consistent with the React and Vue counterparts shown in the relevant code snippets, correctly using
createFileRoutewith the/sass-mixinpath.
8-24: LGTM - Component correctly uses Solid.js conventions.The component properly uses
classinstead ofclassName(Solid.js convention) and includes appropriatedata-testidattributes for e2e testing.e2e/react-start/css-modules/src/routes/sass-mixin.tsx (1)
1-24: LGTM!The route implementation correctly follows TanStack Router's file-based routing conventions. The component structure is clean with appropriate test IDs (
mixin-styled,mixin-content) that align with the e2e test expectations. The SCSS import at the route level properly enables mixin availability during SSR.e2e/vue-start/css-modules/tests/css.spec.ts (1)
118-155: LGTM!The new Sass mixin test is well-structured and consistent with the existing test patterns in the file. It correctly:
- Runs with JavaScript disabled to verify SSR styling
- Uses the
buildUrlhelper for proper base path handling- Verifies all expected mixin styles (flex layout, centering, purple background, padding)
The style assertions align with the values defined in
mixin-consumer.scss.e2e/solid-start/css-modules/tests/css.spec.ts (1)
117-153: LGTM!The Sass mixin test is consistent with the React and Vue implementations, verifying the same style properties. The test correctly validates SSR styling for Solid Start.
e2e/react-start/css-modules/tests/css.spec.ts (1)
117-153: LGTM!The Sass mixin test follows the same pattern as the Vue and Solid implementations, ensuring consistent coverage across all three framework e2e test suites.
e2e/vue-start/css-modules/src/routes/sass-mixin.tsx (2)
4-6: LGTM - Route definition is consistent with project patterns.The route definition follows the same structure as the React equivalent and other routes in the codebase. Function hoisting ensures
SassMixinis available despite being defined after this reference.
8-24: LGTM - Well-structured test component.The component correctly uses Vue JSX conventions (
classinstead ofclassName) and includes appropriatedata-testidattributes for e2e test assertions. The descriptive content clearly documents what the test is validating (Sass mixin availability during dev mode SSR).packages/start-plugin-core/src/dev-server-plugin/plugin.ts (5)
7-11: LGTM!The imports are correctly added for the CSS modules handling utilities needed by this plugin.
24-27: LGTM!Good documentation explaining why the cache is needed - capturing raw CSS content before Vite wraps it in JS for SSR style collection.
35-43: LGTM!The transform hook correctly uses Vite's filter-based approach for CSS modules. Using the regex filter ensures efficient matching, and storing by normalized key handles path variations.
97-104: LGTM!Good defensive check - only calls
collectDevStyleswhen entries exist, avoiding unnecessary work when no routes are matched.
109-117: Good error handling that maintains valid CSS response.The catch block correctly logs the error while still returning a valid CSS response with appropriate headers, preventing MIME type issues in the browser. Including the error message as a CSS comment aids debugging without breaking the response.
packages/start-plugin-core/src/dev-server-plugin/dev-styles.ts (5)
13-19: LGTM!The regex correctly matches CSS module files with various preprocessor extensions and handles query/hash suffixes. The normalization function properly strips query params and hashes while normalizing path separators for cross-platform consistency.
49-67: LGTM!The
resolveDevUrlfunction correctly handles both files within the Vite root (relative paths) and files outside (using/@fsprefix). The path normalization ensures cross-platform compatibility.
241-262: Good error handling for preprocessor partials.The catch block at Lines 257-261 correctly handles the case where Sass/Less partials (files with mixins, variables) can't compile in isolation. The comment clearly explains the expected behavior.
88-93: LGTM!Good use of
Promise.allto process entries in parallel since they are independent. This improves performance when collecting styles from multiple routes.
23-26: The CSS extraction regex is appropriate for Vite's documented dev-mode output format.Vite consistently outputs CSS modules in dev mode using the
const __vite__css = "..."pattern across versions. The regex correctly handles all quote type variations (single, double, and backtick), whitespace patterns, and multi-line CSS. The concern about template literals with expressions does not apply—Vite outputs a simple string constant, not a template literal with expressions. The implementation properly unescapes the captured content.
f564b23 to
8db8b0e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @packages/start-plugin-core/src/dev-server-plugin/dev-styles.ts:
- Line 23: The VITE_CSS_REGEX should use a backreference to ensure the closing
quote matches the opening quote and account for escaped quotes; replace the
pattern with one that captures the quote character and the content separately
(so the CSS is in group 2) and update extractCssFromCode to use match[2] instead
of match[1]; keep the existing .replace(/\\"/g, '"') unmodified so escaped
quotes are handled after extraction.
In @packages/start-plugin-core/src/dev-server-plugin/plugin.ts:
- Around line 35-43: The plugin's transform hook (transform.handler /
CSS_MODULES_REGEX) may run after Vite's core CSS handlers and receive processed
output; add enforce: 'pre' to the plugin object (name:
'tanstack-start-core:dev-server') so the plugin runs in the pre phase and
captures raw CSS module source before Vite's core CSS plugins.
🧹 Nitpick comments (3)
packages/start-plugin-core/src/dev-server-plugin/dev-styles.ts (2)
103-112: Consider graceful handling for missing CSS modules.The hard
throwon line 106-108 will fail the entire style collection if any CSS module is missing from the cache. This could happen if:
- The transform hook hasn't run yet for a newly added CSS module
- A race condition between module graph traversal and transform hook completion
Consider logging a warning and continuing, or returning a placeholder comment:
🔧 Suggested graceful fallback
if (dep.file && isCssModulesFile(dep.file)) { const css = cssModulesCache[normalizeCssModuleCacheKey(dep.file)] if (!css) { - throw new Error( - `[tanstack-start] Missing CSS module in cache: ${dep.file}`, - ) + console.warn( + `[tanstack-start] CSS module not yet in cache (may still be transforming): ${dep.file}`, + ) + continue } styles.set(dep.url, css) continue }
251-260: Consider additional escape sequence handling.The function unescapes
\n,\t,\", and\\, but may be missing:
\'(single quote escape)\r(carriage return, Windows line endings)If Vite can output these escapes, they would remain escaped in the final CSS.
🔧 Suggested enhancement
function extractCssFromCode(code: string): string | undefined { const match = VITE_CSS_REGEX.exec(code) if (!match?.[1]) return undefined return match[1] .replace(/\\n/g, '\n') .replace(/\\t/g, '\t') + .replace(/\\r/g, '\r') .replace(/\\"/g, '"') + .replace(/\\'/g, "'") .replace(/\\\\/g, '\\') }packages/start-plugin-core/src/dev-server-plugin/plugin.ts (1)
84-86: Consider centralizing the TSS_ROUTES_MANIFEST type.The type assertion
as Record<string, { filePath: string; children?: Array<string> }> | undefinedis inline here. If this global is used elsewhere, consider defining and exporting a dedicated type for consistency.// In a shared types file: export interface TSSRoutesManifest { [routeId: string]: { filePath: string; children?: Array<string> } }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/start-plugin-core/src/dev-server-plugin/dev-styles.tspackages/start-plugin-core/src/dev-server-plugin/plugin.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript strict mode with extensive type safety for all code
Files:
packages/start-plugin-core/src/dev-server-plugin/dev-styles.tspackages/start-plugin-core/src/dev-server-plugin/plugin.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Implement ESLint rules for router best practices using the ESLint plugin router
Files:
packages/start-plugin-core/src/dev-server-plugin/dev-styles.tspackages/start-plugin-core/src/dev-server-plugin/plugin.ts
🧠 Learnings (1)
📚 Learning: 2025-11-02T16:16:24.898Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5732
File: packages/start-client-core/src/client/hydrateStart.ts:6-9
Timestamp: 2025-11-02T16:16:24.898Z
Learning: In packages/start-client-core/src/client/hydrateStart.ts, the `import/no-duplicates` ESLint disable is necessary for imports from `#tanstack-router-entry` and `#tanstack-start-entry` because both aliases resolve to the same placeholder file (`fake-start-entry.js`) in package.json during static analysis, even though they resolve to different files at runtime.
Applied to files:
packages/start-plugin-core/src/dev-server-plugin/dev-styles.tspackages/start-plugin-core/src/dev-server-plugin/plugin.ts
🧬 Code graph analysis (1)
packages/start-plugin-core/src/dev-server-plugin/plugin.ts (1)
packages/start-plugin-core/src/dev-server-plugin/dev-styles.ts (3)
CSS_MODULES_REGEX(13-14)normalizeCssModuleCacheKey(16-19)collectDevStyles(79-145)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Preview
- GitHub Check: Test
🔇 Additional comments (8)
packages/start-plugin-core/src/dev-server-plugin/dev-styles.ts (5)
11-19: LGTM! Well-designed CSS modules utilities.The regex pattern correctly handles CSS module extensions with query/hash suffixes, and the cache key normalization properly handles Windows path separators. The non-null assertions are safe since
split()always returns at least one element.
49-67: LGTM! Clean URL resolution logic.The function correctly handles both in-root and out-of-root files, using
/@fsprefix for files outside the project root. The Windows path normalization is properly applied.
150-171: LGTM! Entry processing is well-structured.The function correctly handles the case where a module may not exist yet, and avoids redundant transforms by checking for existing
ssrTransformResult.
200-218: Verify no race condition in parallel visited check.The pattern on lines 205-207 and 215-217 does check-then-add on
visited:if (visited.has(dep)) continue visited.add(dep) branches.push(findModuleDeps(...))Since JavaScript's event loop is single-threaded for synchronous operations, and
Set.has/Set.addare synchronous, this pattern is safe. Theawaiton line 202 could yield, but the subsequent check-and-add block is atomic.The comment on lines 183-184 is a helpful safeguard for future maintainers.
228-249: LGTM! Graceful handling of preprocessor partials.The silent catch on line 244-247 is appropriate and well-documented. Sass/SCSS partials (like
_mixins.scss) cannot be compiled in isolation and will fail; the parent stylesheet will contain the compiled output.packages/start-plugin-core/src/dev-server-plugin/plugin.ts (3)
24-27: LGTM! Well-documented cache initialization.The comment clearly explains why raw CSS content needs to be captured during transform - Vite wraps CSS modules in JS before the style collection runs.
60-71: LGTM! Correct pre-phase middleware registration.The detailed comment explains why pre-phase registration is necessary (before catch-all middlewares), and
endsWithcorrectly handles base path scenarios.
109-117: Error message sanitization looks appropriate for dev mode.Returning the error message as a CSS comment is helpful for debugging in development. Since this is dev-only code (within
configureServer), exposing error details is acceptable.The
no-storecache control is correct for development to ensure fresh styles on each request.
| // URL params that indicate CSS should not be injected (e.g., ?url, ?inline) | ||
| const CSS_SIDE_EFFECT_FREE_PARAMS = ['url', 'inline', 'raw', 'inline-css'] | ||
|
|
||
| const VITE_CSS_REGEX = /const\s+__vite__css\s*=\s*["'`]([\s\S]*?)["'`]/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, let's explore the file structure to locate the file
fd -type f -name "dev-styles.ts" --path "*/dev-server-plugin/*"Repository: TanStack/router
Length of output: 229
🏁 Script executed:
# Also look for the file directly
find . -name "dev-styles.ts" -type fRepository: TanStack/router
Length of output: 123
🏁 Script executed:
# Get the actual content of the file around line 23
cat -n packages/start-plugin-core/src/dev-server-plugin/dev-styles.ts | head -40Repository: TanStack/router
Length of output: 1763
🏁 Script executed:
# Search for how VITE_CSS_REGEX is used in the file
rg -A 5 -B 5 "VITE_CSS_REGEX" packages/start-plugin-core/src/dev-server-plugin/dev-styles.tsRepository: TanStack/router
Length of output: 705
🏁 Script executed:
# Look for the extractCssFromCode function
ast-grep --pattern 'function extractCssFromCode($$$) { $$$ }'Repository: TanStack/router
Length of output: 41
🏁 Script executed:
# Search more broadly for how the regex is used
rg "extractCssFromCode\|match\[" packages/start-plugin-core/src/dev-server-plugin/ -A 3 -B 3Repository: TanStack/router
Length of output: 41
🏁 Script executed:
# Look for test files related to dev-styles
find . -name "*.test.*" -o -name "*.spec.*" | xargs grep -l "VITE_CSS_REGEX\|extractCssFromCode" 2>/dev/nullRepository: TanStack/router
Length of output: 43
🏁 Script executed:
# Search for any tests or examples showing Vite CSS output format
rg "const __vite__css" --type ts --type tsx --type jsRepository: TanStack/router
Length of output: 86
🏁 Script executed:
# Check if there are any comments or documentation in the file
cat -n packages/start-plugin-core/src/dev-server-plugin/dev-styles.ts | head -80Repository: TanStack/router
Length of output: 3228
🏁 Script executed:
# Look for how this function is called and what kind of code is passed to it
rg "extractCssFromCode" packages/start-plugin-core/src/dev-server-plugin/ -B 5 -A 10Repository: TanStack/router
Length of output: 2960
🏁 Script executed:
# Search for test files that might show example Vite transformed code
find . -type f \( -name "*.test.ts" -o -name "*.spec.ts" -o -name "*.test.js" -o -name "*.spec.js" \) | head -20Repository: TanStack/router
Length of output: 1368
🏁 Script executed:
# Look for any example or fixture files showing Vite output
find . -type f -name "*.fixture.*" -o -name "*fixture*" | head -20Repository: TanStack/router
Length of output: 139
🏁 Script executed:
# Search for comments or documentation about the __vite__css pattern
rg "__vite__css" -B 5 -A 5Repository: TanStack/router
Length of output: 1152
🏁 Script executed:
# Check Vite documentation or comments in the codebase about CSS transformation
rg "Vite.*CSS\|__vite__" packages/start-plugin-core/ -B 2 -A 2Repository: TanStack/router
Length of output: 41
🏁 Script executed:
# Search for any references to escaped quotes in CSS context
rg "\\\\\"" packages/start-plugin-core/src/dev-server-plugin/ -B 3 -A 3Repository: TanStack/router
Length of output: 564
🏁 Script executed:
# Look at the entire extractCssFromCode function and understand the flow better
cat -n packages/start-plugin-core/src/dev-server-plugin/dev-styles.ts | sed -n '115,135p'Repository: TanStack/router
Length of output: 690
🏁 Script executed:
# Check if there are any tests specifically for this function
rg -l "extractCssFromCode" . --type ts --type js 2>/dev/null | head -10Repository: TanStack/router
Length of output: 123
🏁 Script executed:
# Look for any edge case handling or comments about the regex
cat -n packages/start-plugin-core/src/dev-server-plugin/dev-styles.ts | sed -n '20,40p'Repository: TanStack/router
Length of output: 888
🏁 Script executed:
# Search Vite documentation or check how Vite actually generates __vite__css output
web_search
Vite __vite__css variable output format how are quotes escapedRepository: TanStack/router
Length of output: 152
🌐 Web query:
Vite __vite__css variable format how quotes are escaped in transformed CSS
💡 Result:
I couldn't find authoritative docs showing exactly how Vite encodes/escapes quotes inside the internal __vite__css variable. Can you paste a short example of the transformed CSS (the module output that contains __vite__css) or say which Vite version and transformer (esbuild / lightningcss) you're using? With that I can inspect the exact escaping and explain.
🏁 Script executed:
# Let's look at the actual extractCssFromCode function more carefully
cat -n packages/start-plugin-core/src/dev-server-plugin/dev-styles.ts | sed -n '150,170p'Repository: TanStack/router
Length of output: 846
🏁 Script executed:
# Find where extractCssFromCode is defined and get the full function
ast-grep --pattern 'function extractCssFromCode($_) { $$$ }'Repository: TanStack/router
Length of output: 41
🏁 Script executed:
# Try a simpler search to find the function definition
rg -A 10 "^function extractCssFromCode" packages/start-plugin-core/src/dev-server-plugin/dev-styles.tsRepository: TanStack/router
Length of output: 329
🏁 Script executed:
# Check if there are tests for dev-styles.ts
find . -path "*/test*" -o -path "*/spec*" | xargs grep -l "dev-styles\|extractCssFromCode" 2>/dev/null | head -10Repository: TanStack/router
Length of output: 41
🏁 Script executed:
# Look at how Vite is configured in this project to understand the version and transformer
cat -n packages/start-plugin-core/package.json | head -30Repository: TanStack/router
Length of output: 1092
🏁 Script executed:
# Check vite version in root package.json
rg "\"vite\":" --type json | head -5Repository: TanStack/router
Length of output: 326
🏁 Script executed:
# Search for any actual examples of __vite__css output in the codebase
rg "const.*__vite__css.*=" packages/start-plugin-core/ -A 2Repository: TanStack/router
Length of output: 367
🏁 Script executed:
# Let's look at actual Vite source or documentation in node_modules if available
fd -type f "vite" packages/start-plugin-core/node_modules 2>/dev/null | head -5Repository: TanStack/router
Length of output: 41
🏁 Script executed:
# Check if there are any unit tests that might show actual examples of __vite__css output
find . -name "*.test.ts" -o -name "*.spec.ts" | xargs grep -l "vite\|css" 2>/dev/null | head -5Repository: TanStack/router
Length of output: 353
🏁 Script executed:
# Look at test files in the start-plugin-core package specifically
find ./packages/start-plugin-core -name "*.test.*" -o -name "*.spec.*" 2>/dev/nullRepository: TanStack/router
Length of output: 590
🏁 Script executed:
# Check git history to see when and why this regex was added
cd packages/start-plugin-core/src/dev-server-plugin && git log --oneline -p dev-styles.ts | head -150Repository: TanStack/router
Length of output: 4905
🏁 Script executed:
# Look for any comments or issues about the VITE_CSS_REGEX in the codebase
rg "VITE_CSS_REGEX\|__vite__css" packages/start-plugin-core/ -B 3 -A 3 --type tsRepository: TanStack/router
Length of output: 41
🏁 Script executed:
# Check if the replacement /\\"/g handles escaped quotes, suggesting Vite escapes them
cat -n packages/start-plugin-core/src/dev-server-plugin/dev-styles.ts | sed -n '155,175p'Repository: TanStack/router
Length of output: 853
🏁 Script executed:
# Check the CSS module tests to see if they might show example CSS output
cat -n ./e2e/react-start/css-modules/tests/css.spec.ts | head -50Repository: TanStack/router
Length of output: 2301
🏁 Script executed:
# Look for any snapshots or fixtures related to CSS modules
find ./e2e -name "*snapshot*" -o -name "*fixture*" | xargs grep -l "css" 2>/dev/null | head -5Repository: TanStack/router
Length of output: 41
🏁 Script executed:
# Check if there's documentation or examples showing __vite__css format
rg "vite.*css\|__vite" e2e/ --type js --type ts -A 3 | head -50Repository: TanStack/router
Length of output: 41
🏁 Script executed:
# Let me check if there's a way to understand Vite's actual CSS output format
# Look at Vite's source or any vendor code in node_modules
fd "vite" packages/start-plugin-core/node_modules -type d 2>/dev/null | head -3Repository: TanStack/router
Length of output: 41
🏁 Script executed:
# Try to find any actual transformed CSS examples or tests showing the __vite__css pattern
rg "const.*__vite__css" . --type ts --type js -B 2 -A 2 2>/dev/null | head -100Repository: TanStack/router
Length of output: 579
🏁 Script executed:
# Let's create a simple test to understand the regex behavior with different quote scenarios
python3 << 'EOF'
import re
VITE_CSS_REGEX = r"const\s+__vite__css\s*=\s*[\"'`]([\s\S]*?)[\"'`]"
# Test cases
test_cases = [
('const __vite__css = "normal css"', "normal css"), # Normal case
('const __vite__css = "css with \\"escaped\\" quotes"', 'css with \\"escaped\\" quotes'), # Escaped quotes (Vite likely output)
('const __vite__css = "css with \'single\' quotes"', 'css with '), # Mixed quotes - problematic case
("const __vite__css = 'css with \"double\" quotes'", 'css with '), # Mixed quotes reversed
('const __vite__css = `backtick "with" quotes`', 'backtick '), # Backtick with other quotes
]
print("Testing current VITE_CSS_REGEX behavior:")
print("=" * 70)
for test_input, expected in test_cases:
match = re.search(VITE_CSS_REGEX, test_input)
if match:
actual = match.group(1)
status = "✓" if actual == expected else "✗"
print(f"{status} Input: {test_input}")
print(f" Expected: {expected!r}")
print(f" Got: {actual!r}")
else:
print(f"✗ No match: {test_input}")
print()
# Test the suggested backreference fix
VITE_CSS_REGEX_FIXED = r"const\s+__vite__css\s*=\s*([\"'`])([\s\S]*?)\1"
print("\n" + "=" * 70)
print("Testing FIXED regex with backreference:")
print("=" * 70)
for test_input, _ in test_cases:
match = re.search(VITE_CSS_REGEX_FIXED, test_input)
if match:
captured_quote = match.group(1)
captured_css = match.group(2)
print(f"✓ Input: {test_input}")
print(f" Quote: {captured_quote!r}, CSS: {captured_css!r}")
else:
print(f"✗ No match (requires matching quotes): {test_input}")
print()
EOFRepository: TanStack/router
Length of output: 145
🏁 Script executed:
# Let me try a simpler Python test without the timeout
python3 -c "
import re
# Current regex
current = r\"const\s+__vite__css\s*=\s*[\\\"'\`]([\s\S]*?)[\\\"'\`]\"
# Test case: Vite with escaped quotes (most likely scenario)
test1 = 'const __vite__css = \"css content with \\\\\" escaped \\\\\" quotes\"'
m = re.search(current, test1)
print('Test 1 (escaped quotes):', m.group(1) if m else 'NO MATCH')
# Test case: Mixed quotes (problematic scenario from review)
test2 = 'const __vite__css = \"content with \\'single\\' quotes\"'
m = re.search(current, test2)
print('Test 2 (mixed quotes):', m.group(1) if m else 'NO MATCH')
print('Test 2 expected: content with \\'single\\' quotes')
"Repository: TanStack/router
Length of output: 185
🏁 Script executed:
# Check Vite's source code behavior - look at their CSS transformation plugin
# Search for how Vite actually generates __vite__css
rg "__vite__css" . --type ts --type js -B 5 -A 5 2>/dev/null | head -100Repository: TanStack/router
Length of output: 1174
🏁 Script executed:
# Check if there are any actual integration tests showing transformed code
find . -path "*/test*" -name "*.ts" -o -name "*.js" | xargs grep -l "transformResult\|ssrTransformResult" 2>/dev/null | head -5Repository: TanStack/router
Length of output: 41
Fix regex to use backreferences for matching quote pairs.
The regex fails when CSS content contains quotes that don't match the outer quote type. For example, with input const __vite__css = "text with 'single' quotes", the regex incorrectly stops at the first ' and captures only text with .
The presence of .replace(/\\"/g, '"') in extractCssFromCode confirms Vite outputs escaped quotes, which also breaks the current pattern by stopping at the escape sequence.
Use a backreference to ensure the closing quote matches the opening quote type:
🔧 Suggested fix
-const VITE_CSS_REGEX = /const\s+__vite__css\s*=\s*["'`]([\s\S]*?)["'`]/
+const VITE_CSS_REGEX = /const\s+__vite__css\s*=\s*(["'`])([\s\S]*?)\1/Update extractCssFromCode to use match[2] instead of match[1] for the captured CSS content.
🤖 Prompt for AI Agents
In @packages/start-plugin-core/src/dev-server-plugin/dev-styles.ts at line 23,
The VITE_CSS_REGEX should use a backreference to ensure the closing quote
matches the opening quote and account for escaped quotes; replace the pattern
with one that captures the quote character and the content separately (so the
CSS is in group 2) and update extractCssFromCode to use match[2] instead of
match[1]; keep the existing .replace(/\\"/g, '"') unmodified so escaped quotes
are handled after extraction.
| // Capture CSS modules content during transform | ||
| transform: { | ||
| filter: { | ||
| id: CSS_MODULES_REGEX, | ||
| }, | ||
| handler(code, id) { | ||
| cssModulesCache[normalizeCssModuleCacheKey(id)] = code | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
Vite plugin transform hook CSS modules raw content order
💡 Result:
Short answer
- Vite runs transform hooks sequentially in plugin order (user plugins with enforce: 'pre' → Vite core plugins → user plugins → enforce: 'post'). Use enforce or plugin ordering to control when your transform sees a file. [1][2]
Why this matters for CSS Modules
- Vite’s core CSS handling (including the internal css-post/css module plugins like vite:css-post) runs as part of the core plugin chain, so by the time a normal user transform runs it may receive CSS after Vite’s CSS-module processing (or receive the JS proxy module that Vite emits for a CSS module). If you need the raw .css content of a CSS module before Vite’s processing, you must run earlier than the core css plugins. [2][3]
What to do
- If you need raw CSS-module source in your transform hook:
- Make your plugin run before Vite core plugins: set enforce: 'pre' and ensure plugin order places it before Vite’s css plugins. [1][2]
- Or use an existing patch plugin that makes CSS Modules emit as JS earlier (e.g., vite-css-modules) so other plugins can see module internals. [3][4]
References
- Vite plugin ordering / enforce behavior. [1]
- Plugin container: transform hooks are sequential and executed in plugin order. [2]
- vite-css-modules (patch that changes when/how CSS Modules are emitted so other plugins can access them). [3][4]
🏁 Script executed:
# First, let me examine the code context around the transform hook
find . -type f -name "plugin.ts" -path "*/dev-server-plugin/*" | head -5Repository: TanStack/router
Length of output: 119
🏁 Script executed:
# Get more context about the file and the transform hook
cat -n packages/start-plugin-core/src/dev-server-plugin/plugin.ts | head -70Repository: TanStack/router
Length of output: 3264
Add enforce: 'pre' to guarantee raw CSS modules content.
The transform hook at lines 35-43 must run before Vite's core CSS plugins to receive raw CSS content for CSS modules. Without enforce: 'pre', the hook may receive already-processed CSS or a JS module proxy instead of the raw CSS source. Update the plugin configuration to:
{
name: 'tanstack-start-core:dev-server',
enforce: 'pre',
...
The inline comment at lines 25-26 claims raw CSS is received, but Vite's plugin ordering guarantees this only when the plugin runs in the pre phase before core CSS handlers.
🤖 Prompt for AI Agents
In @packages/start-plugin-core/src/dev-server-plugin/plugin.ts around lines 35 -
43, The plugin's transform hook (transform.handler / CSS_MODULES_REGEX) may run
after Vite's core CSS handlers and receive processed output; add enforce: 'pre'
to the plugin object (name: 'tanstack-start-core:dev-server') so the plugin runs
in the pre phase and captures raw CSS module source before Vite's core CSS
plugins.
Summary by CodeRabbit
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.