feat(start): add inline CSS runtime controls and asset URL templates#7380
Open
schiller-manuel wants to merge 3 commits into
Open
feat(start): add inline CSS runtime controls and asset URL templates#7380schiller-manuel wants to merge 3 commits into
schiller-manuel wants to merge 3 commits into
Conversation
github-actions
Bot
added
documentation
Contributor
|
View your CI Pipeline Execution ↗ for commit 1263b50
☁️ Nx Cloud last updated this comment at |
Contributor
🚀 Changeset Version Preview5 package(s) bumped directly, 19 bumped as dependents. 🟨 Minor bumps
🟩 Patch bumps
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds runtime-configurable inline CSS and opt-in CSS URL templates for TanStack Start. The schema now supports both boolean and object configuration with ChangesInline CSS Feature Implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
Contributor
Bundle Size Benchmarks
Current gzip tracks all emitted client JS chunks. Initial gzip tracks only the entry/import graph. Trend sparkline is historical current gzip ending with this PR measurement; lower is better. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/start/framework/react/guide/css-styling.md`:
- Around line 197-204: The example incorrectly calls handler.fetch(...); update
the custom runtime wrapper example so it invokes the handler as a callable
function (handler(request, { inlineCss })) rather than accessing a non-existent
.fetch property; change the fetch implementation inside createServerEntry to
call handler(request, { inlineCss: request.headers.get('x-inline-css') !==
'false' }) so it matches the handler returned by
createStartHandler()/createServerEntry and the e2e tests.
In `@packages/start-server-core/src/createStartHandler.ts`:
- Around line 316-317: The module-scoped cache Map cachedFinalManifestPromises
is shared across all handlers and causes cross-handler pollution; move its
declaration into the createStartHandler scope (or the handler factory) so each
handler instance gets its own Map, and update all usages (including the warmup
code paths that currently write into cachedFinalManifestPromises) to reference
the instance-local variable instead of the module-scoped one; similarly relocate
or convert the other module-scoped caches referenced around the other spots
(lines ~423-428, ~639-660) to instance-local variables so different
transformAssets configs cannot reuse another handler’s cached manifest.
In `@packages/start-server-core/src/transformAssetUrls.ts`:
- Around line 469-474: The returned inlineCss is passed by reference from
source.manifest.inlineCss, allowing downstream mutation of styles/templates to
leak into the cached base manifest; instead, when opts?.inlineCss !== false set
inlineCss to a cloned copy of source.manifest.inlineCss (use a safe clone
strategy such as structuredClone or a shallow copy via spread/map depending on
the shape) so mutations to the returned value don't affect the original
manifest; update the return in transformAssetUrls.ts where inlineCss is assigned
to use the clone and keep the existing opts?.inlineCss check.
- Around line 219-230: The current code fills transformedStyles in the order
promises resolve, which can reorder inlineCss.styles; change the mapping so you
produce an array of promises that each resolve to a tuple [stylesheetHref,
transformedCss] (use transformInlineCssTemplate for templates or css fallback)
and then await Promise.all on that array and assign results into
transformedStyles in the original source order by iterating the resolved tuples;
reference inlineCss.styles, inlineCss.templates, transformInlineCssTemplate, and
transformedStyles.
🪄 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: 8c3e2c7b-271a-4677-ab05-ece2e4902ad8
📥 Commits
Reviewing files that changed from the base of the PR and between b1c061a and 700fd012f9a3a59bac7fa7aa56678e3b356f3dc6.
⛔ Files ignored due to path filters (1)
e2e/react-start/css-inline/src/styles/inline-dot.svgis excluded by!**/*.svg
📒 Files selected for processing (35)
.changeset/quiet-dragons-collect.mddocs/start/config.jsondocs/start/framework/react/guide/cdn-asset-urls.mddocs/start/framework/react/guide/css-styling.mddocs/start/framework/react/guide/early-hints.mddocs/start/framework/solid/guide/css-styling.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/src/server.tse2e/react-start/css-inline/tests/css-inline.spec.tse2e/react-start/css-inline/vite.config.tse2e/react-start/server-routes/src/routeTree.gen.tse2e/react-start/transform-asset-urls/package.jsone2e/react-start/transform-asset-urls/playwright.config.tse2e/react-start/transform-asset-urls/src/server.tspackages/router-core/src/manifest.tspackages/start-plugin-core/src/rsbuild/plugin.tspackages/start-plugin-core/src/rsbuild/schema.tspackages/start-plugin-core/src/rsbuild/start-router-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/vite/plugin.tspackages/start-plugin-core/src/vite/schema.tspackages/start-plugin-core/tests/start-manifest-plugin/manifestBuilder.test.tspackages/start-server-core/skills/start-server-core/SKILL.mdpackages/start-server-core/src/createStartHandler.tspackages/start-server-core/src/index.tsxpackages/start-server-core/src/inlineCss.tspackages/start-server-core/src/request-handler.tspackages/start-server-core/src/transformAssetUrls.tspackages/start-server-core/tests/createStartHandler.test.tspackages/start-server-core/tests/transformAssets.test.ts
💤 Files with no reviewable changes (1)
- e2e/react-start/transform-asset-urls/playwright.config.ts
![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.
Merging this PR will not alter performance
Comparing 
Footnotes
|
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
schiller-manuel
force-pushed
the
css-inline-runtime
branch
from
eb7fde7 to
29f09f2
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
e2e/react-start/css-inline/playwright.config.ts (1)
7-10: ⚡ Quick winValidate
CSS_INLINE_TRANSFORM_ASSETSinstead of silently coercing unknown valuesRight now any non-
"true"value is treated as disabled, which can mask bad env input. Please fail fast (or normalize explicitly) so misconfiguration is obvious.Proposed patch
+function readBooleanEnv(name: string, defaultValue: boolean): boolean { + const raw = process.env[name] + if (raw == null) return defaultValue + if (raw === 'true') return true + if (raw === 'false') return false + throw new Error(`${name} must be "true" or "false", received "${raw}"`) +} + const toolchain = process.env.E2E_TOOLCHAIN ?? 'vite' -const inlineCssTransformAssets = - process.env.CSS_INLINE_TRANSFORM_ASSETS ?? 'false' +const inlineCssTransformAssets = readBooleanEnv( + 'CSS_INLINE_TRANSFORM_ASSETS', + false, +) const transformAssetsSuffix = - inlineCssTransformAssets === 'true' ? '-transform-assets' : '' + inlineCssTransformAssets ? '-transform-assets' : '' const distDir = process.env.E2E_DIST_DIR ?? `dist-${toolchain}${transformAssetsSuffix}-ssr` @@ env: { E2E_DIST_DIR: distDir, PORT: String(PORT), - CSS_INLINE_TRANSFORM_ASSETS: inlineCssTransformAssets, + CSS_INLINE_TRANSFORM_ASSETS: inlineCssTransformAssets ? 'true' : 'false', },As per coding guidelines,
**/*.{ts,tsx}: Use TypeScript strict mode with extensive type safety.Also applies to: 41-41
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/react-start/css-inline/playwright.config.ts` around lines 7 - 10, The code silently treats any non-"true" CSS_INLINE_TRANSFORM_ASSETS value as disabled; update the initialization of inlineCssTransformAssets and transformAssetsSuffix so the env var is validated explicitly: accept only "true" or "false" (or empty) and either throw a clear error for any other value or normalize by forcing to lowercase and mapping "1"/"0"/"yes"/"no" to "true"/"false"; use the inlineCssTransformAssets variable (string) to derive a boolean (e.g., isInline = inlineCssTransformAssets === 'true') and then compute transformAssetsSuffix from that boolean, and ensure invalid input causes a fast fail with a descriptive message mentioning CSS_INLINE_TRANSFORM_ASSETS and the offending value.packages/start-plugin-core/src/start-manifest-plugin/inlineCss.ts (1)
109-114: 💤 Low valueInconsistent index calculation pattern between
Urlandimportvisitors.The
Urlvisitor pushes tourlsfirst then usesurls.length - 1, while theimportrule usesurls.lengththen pushes. Both produce correct indices, but the inconsistency reduces readability.Consider aligning the pattern:
♻️ Suggested alignment (push-then-reference style)
Rule: { import(rule: any) { if (!shouldTransformInlineCssUrl(rule.value.url)) { return rule } const rebasedUrl = rebaseCssUrl(rule.value.url, options.cssHref) + + if (templates) { + urls.push(rebasedUrl) + } + const value = { url: templates - ? createInlineCssUrlPlaceholder(urls.length) + ? createInlineCssUrlPlaceholder(urls.length - 1) : rebasedUrl, loc: rule.value.loc, ...(rule.value.media ? { media: rule.value.media } : {}), ...(rule.value.layer ? { layer: rule.value.layer } : {}), ...(rule.value.supports ? { supports: rule.value.supports } : {}), } - if (templates) { - urls.push(rebasedUrl) - } - return { ...rule, value, } }, },Also applies to: 129-141
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/start-plugin-core/src/start-manifest-plugin/inlineCss.ts` around lines 109 - 114, The Url visitor and the import rule use two different index patterns when adding entries to the urls array (Url pushes then references urls.length - 1; import references urls.length then pushes), which is inconsistent; standardize to a single pattern (prefer push-then-reference) across both visitors by updating the import handling to push the rebasedUrl into urls first and then call createInlineCssUrlPlaceholder(urls.length - 1) so both Url visitor and import rule use the same push-then-reference flow, and ensure you update all similar blocks (including the region around 129-141) to use the same approach for the urls array and placeholder creation.packages/start-server-core/src/createStartHandler.ts (1)
632-660: 💤 Low valueWarmup always warms the
inline-cssvariant regardless of handler default.When
warmupTransformManifestis true, warmup pre-computes theinline-cssmanifest. If the handler'sinlineCssoption defaults tofalse, the warmed cache entry may never be used while thelinked-cssvariant is computed on first request.This is a minor inefficiency for uncommon configurations. Consider documenting this behavior or conditionally warming based on the handler's effective default.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/start-server-core/src/createStartHandler.ts` around lines 632 - 660, The warmup currently always computes the 'inline-css' variant regardless of the handler's effective inlineCss default; change the warmup gate to only precompute the manifest variant that matches the handler's effective default (inline or linked). Concretely, determine the handler's default inlineCss setting (e.g., derive a boolean like handlerDefaultInlineCss from the handler/options used to create the start handler) and replace the hardcoded 'inline-css' warmup key and transform arguments with a conditional that selects either 'inline-css' (and inlineCss: true) or the linked-css key (and inlineCss: false) before calling getBaseManifest, getTransformFn, transformManifestAssets or buildManifestWithClientEntry; keep the same error/retry logic and still clear cachedCreateTransformPromise on failure.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@e2e/react-start/css-inline/playwright.config.ts`:
- Around line 7-10: The code silently treats any non-"true"
CSS_INLINE_TRANSFORM_ASSETS value as disabled; update the initialization of
inlineCssTransformAssets and transformAssetsSuffix so the env var is validated
explicitly: accept only "true" or "false" (or empty) and either throw a clear
error for any other value or normalize by forcing to lowercase and mapping
"1"/"0"/"yes"/"no" to "true"/"false"; use the inlineCssTransformAssets variable
(string) to derive a boolean (e.g., isInline = inlineCssTransformAssets ===
'true') and then compute transformAssetsSuffix from that boolean, and ensure
invalid input causes a fast fail with a descriptive message mentioning
CSS_INLINE_TRANSFORM_ASSETS and the offending value.
In `@packages/start-plugin-core/src/start-manifest-plugin/inlineCss.ts`:
- Around line 109-114: The Url visitor and the import rule use two different
index patterns when adding entries to the urls array (Url pushes then references
urls.length - 1; import references urls.length then pushes), which is
inconsistent; standardize to a single pattern (prefer push-then-reference)
across both visitors by updating the import handling to push the rebasedUrl into
urls first and then call createInlineCssUrlPlaceholder(urls.length - 1) so both
Url visitor and import rule use the same push-then-reference flow, and ensure
you update all similar blocks (including the region around 129-141) to use the
same approach for the urls array and placeholder creation.
In `@packages/start-server-core/src/createStartHandler.ts`:
- Around line 632-660: The warmup currently always computes the 'inline-css'
variant regardless of the handler's effective inlineCss default; change the
warmup gate to only precompute the manifest variant that matches the handler's
effective default (inline or linked). Concretely, determine the handler's
default inlineCss setting (e.g., derive a boolean like handlerDefaultInlineCss
from the handler/options used to create the start handler) and replace the
hardcoded 'inline-css' warmup key and transform arguments with a conditional
that selects either 'inline-css' (and inlineCss: true) or the linked-css key
(and inlineCss: false) before calling getBaseManifest, getTransformFn,
transformManifestAssets or buildManifestWithClientEntry; keep the same
error/retry logic and still clear cachedCreateTransformPromise on failure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c276de89-fa5d-4bdd-9738-a4c7d14f31cb
📥 Commits
Reviewing files that changed from the base of the PR and between 700fd012f9a3a59bac7fa7aa56678e3b356f3dc6 and 29f09f2.
⛔ Files ignored due to path filters (1)
e2e/react-start/css-inline/src/styles/inline-dot.svgis excluded by!**/*.svg
📒 Files selected for processing (35)
.changeset/quiet-dragons-collect.mddocs/start/config.jsondocs/start/framework/react/guide/cdn-asset-urls.mddocs/start/framework/react/guide/css-styling.mddocs/start/framework/react/guide/early-hints.mddocs/start/framework/solid/guide/css-styling.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/src/server.tse2e/react-start/css-inline/tests/css-inline.spec.tse2e/react-start/css-inline/vite.config.tse2e/react-start/server-routes/src/routeTree.gen.tse2e/react-start/transform-asset-urls/package.jsone2e/react-start/transform-asset-urls/playwright.config.tse2e/react-start/transform-asset-urls/src/server.tspackages/router-core/src/manifest.tspackages/start-plugin-core/src/rsbuild/plugin.tspackages/start-plugin-core/src/rsbuild/schema.tspackages/start-plugin-core/src/rsbuild/start-router-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/vite/plugin.tspackages/start-plugin-core/src/vite/schema.tspackages/start-plugin-core/tests/start-manifest-plugin/manifestBuilder.test.tspackages/start-server-core/skills/start-server-core/SKILL.mdpackages/start-server-core/src/createStartHandler.tspackages/start-server-core/src/index.tsxpackages/start-server-core/src/inlineCss.tspackages/start-server-core/src/request-handler.tspackages/start-server-core/src/transformAssetUrls.tspackages/start-server-core/tests/createStartHandler.test.tspackages/start-server-core/tests/transformAssets.test.ts
💤 Files with no reviewable changes (1)
- e2e/react-start/transform-asset-urls/playwright.config.ts
✅ Files skipped from review due to trivial changes (9)
- docs/start/framework/react/guide/early-hints.md
- packages/start-server-core/skills/start-server-core/SKILL.md
- .changeset/quiet-dragons-collect.md
- docs/start/framework/solid/guide/css-styling.md
- e2e/react-start/css-inline/package.json
- packages/start-plugin-core/src/rsbuild/schema.ts
- packages/start-server-core/tests/createStartHandler.test.ts
- docs/start/framework/react/guide/cdn-asset-urls.md
- e2e/react-start/server-routes/src/routeTree.gen.ts
🚧 Files skipped from review as they are similar to previous changes (20)
- packages/start-server-core/src/inlineCss.ts
- packages/start-plugin-core/src/vite/plugin.ts
- packages/start-plugin-core/src/rsbuild/plugin.ts
- packages/start-plugin-core/src/rsbuild/start-router-plugin.ts
- e2e/react-start/css-inline/vite.config.ts
- packages/start-plugin-core/src/schema.ts
- packages/router-core/src/manifest.ts
- e2e/react-start/css-inline/tests/css-inline.spec.ts
- packages/start-plugin-core/src/rsbuild/virtual-modules.ts
- packages/start-plugin-core/tests/start-manifest-plugin/manifestBuilder.test.ts
- e2e/react-start/transform-asset-urls/package.json
- packages/start-server-core/src/index.tsx
- e2e/react-start/css-inline/src/server.ts
- e2e/react-start/css-inline/rsbuild.config.ts
- docs/start/config.json
- packages/start-server-core/src/request-handler.ts
- packages/start-plugin-core/src/vite/schema.ts
- packages/start-server-core/tests/transformAssets.test.ts
- e2e/react-start/transform-asset-urls/src/server.ts
- packages/start-server-core/src/transformAssetUrls.ts
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
schiller-manuel
force-pushed
the
css-inline-runtime
branch
from
28adacb to
1263b50
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/start-server-core/src/createStartHandler.ts`:
- Around line 420-423: The current branch stores a rejected
computeFinalManifest() promise in cachedFinalManifestPromises (via
cachedFinalManifestPromise and cacheKey), which permanently poisons that cache
entry on transient failures; fix this by attaching an error handler to the
promise returned by computeFinalManifest() that, on rejection, removes the entry
from cachedFinalManifestPromises and clears cachedFinalManifestPromise for that
cacheKey, then rethrows the error so callers still observe the failure; ensure
the successful promise is still stored as before and only failed promises are
evicted.
🪄 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: 41412420-4083-4bc4-89b1-8ce8bba3eb5c
📒 Files selected for processing (3)
e2e/react-start/css-inline/playwright.config.tspackages/start-plugin-core/src/start-manifest-plugin/inlineCss.tspackages/start-server-core/src/createStartHandler.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- e2e/react-start/css-inline/playwright.config.ts
- packages/start-plugin-core/src/start-manifest-plugin/inlineCss.ts
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
schiller-manuel
force-pushed
the
css-inline-runtime
branch
from
a5b7e6f to
d68d38b
Compare
Contributor
There was a problem hiding this comment.
Nx Cloud has identified a possible root cause for your failed CI:
We identified that the failing test invalidates transitive server function compiler state during HMR is not related to this PR's changes, as the same failure is reproducible on branch 7382 with identical errors. Since the tanstack-react-start-e2e-hmr project was not modified by this PR, this appears to be a pre-existing environment issue that should be investigated independently.
No code changes were suggested for this issue.
Trigger a rerun:
🎓 Learn more about Self-Healing CI on nx.dev
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Documentation
Chores
Tests