Merged
Conversation
replaces transformAssetUrls
github-actions
bot
added
documentation
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 (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a kind-aware transformAssets pipeline and HeadContent Changes
Sequence Diagram(s)sequenceDiagram
participant DevServer as Dev/Start Handler
participant Transform as transformAssets
participant Manifest as Start Manifest
participant Head as HeadContent / useTags
participant Browser as Browser / CDN
DevServer->>Manifest: load manifest (routes, assets, preloads)
DevServer->>Transform: request transform per asset (kind, url)
Transform-->>DevServer: { href, crossOrigin? }
DevServer->>Manifest: apply transformed href/crossOrigin
Browser->>Head: request page (SSR/CSR)
Head->>Manifest: read manifest-managed assets/preloads
Head->>Browser: emit <link rel="modulepreload|stylesheet" href=... crossorigin=...>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 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📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx affected --targets=test:eslint,test:unit,tes... |
❌ Failed | 9m 19s | View ↗ |
nx run-many --target=build --exclude=examples/*... |
✅ Succeeded | 34s | View ↗ |
☁️ Nx Cloud last updated this comment at 2026-03-22 12:10:51 UTC
Contributor
🚀 Changeset Version PreviewNo changeset entries found. Merging this PR will not cause a version bump for any packages. |
Contributor
Bundle Size Benchmarks
Trend sparkline is historical gzip bytes 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: 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/createStartHandler.ts (1)
472-490:⚠️ Potential issue | 🟠 MajorDon’t memoize a failed or dev-only
createTransform()call.This promise is cached even when
TSS_DEV_SERVER === 'true', and a sync/async failure on the first request stays cached forever. That makes dev miss the documented cache bypass and turns a transientcreateTransform()failure into a process-wide outage until restart.Suggested change
- if (cache) { + if (cache && process.env.TSS_DEV_SERVER !== 'true') { if (!cachedCreateTransformPromise) { - cachedCreateTransformPromise = Promise.resolve( - resolvedTransformConfig.createTransform(opts), - ) + cachedCreateTransformPromise = Promise.resolve() + .then(() => resolvedTransformConfig.createTransform(opts)) + .catch((error) => { + cachedCreateTransformPromise = undefined + throw error + }) } return cachedCreateTransformPromise }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/start-server-core/src/createStartHandler.ts` around lines 472 - 490, The cachedCreateTransformPromise is being stored unconditionally which preserves failures and ignores the dev-server cache-bypass; modify getTransformFn (and use of cachedCreateTransformPromise) so that caching only occurs when cache is enabled and process.env.TSS_DEV_SERVER !== 'true', and ensure you never store a permanently rejected promise: when you create the promise from resolvedTransformConfig.createTransform(opts) wrap it so on rejection you clear cachedCreateTransformPromise (set it back to undefined) before re-throwing the error; keep the same return behavior for resolvedTransformConfig.transformFn when type !== 'createTransform'.
🧹 Nitpick comments (1)
e2e/react-start/transform-asset-urls/src/server.ts (1)
6-16: Consider importingTransformAssetsFnfrom the package instead of redefining it locally.The local type definition duplicates the type exported from
@tanstack/react-start/server. Importing it directly would keep the e2e test aligned with the actual package type and avoid drift.♻️ Suggested import
-import type { TransformAssetUrls } from '@tanstack/react-start/server' - -type TransformAssetsFn = (ctx: { - kind: 'modulepreload' | 'stylesheet' | 'clientEntry' - url: string -}) => - | string - | { - href: string - crossOrigin?: 'anonymous' | 'use-credentials' - } +import type { + TransformAssetsFn, + TransformAssetUrls, +} from '@tanstack/react-start/server'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/react-start/transform-asset-urls/src/server.ts` around lines 6 - 16, Replace the local duplicated type alias TransformAssetsFn with the exported type from the package by adding an import for TransformAssetsFn from '@tanstack/react-start/server' (alongside the existing TransformAssetUrls import) and remove the local type definition; update any references to use the imported TransformAssetsFn type (search for the symbol TransformAssetsFn and the local type block) so the test uses the canonical type from the package.
🤖 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/start-server-core/src/createStartHandler.ts`:
- Around line 441-456: The selection logic currently uses truthy checks so an
explicit empty-string config (valid no-op) is treated as absent; change the
ternary conditions to explicit undefined checks: use "transformAssetsOption !==
undefined" and "transformAssetUrlsOption !== undefined" when computing
transformOption so empty-string values are preserved, and keep the calls to
resolveTransformAssetsConfig and adaptTransformAssetUrlsConfigToTransformAssets
exactly as written for transformAssetsOption and transformAssetUrlsOption
respectively.
In `@packages/start-server-core/src/transformAssetUrls.ts`:
- Around line 384-386: The code currently assigns manifest = opts?.clone ?
structuredClone(source.manifest) : source.manifest which lets the async rewrite
mutate the cached source.manifest when clone is false; change the logic so the
async rewrite never mutates source.manifest directly (i.e., always operate on a
copy). Specifically, replace the conditional assignment so manifest is a
structuredClone of source.manifest before the async rewrite runs (or create a
fresh deep copy immediately before calling the async rewrite routine), ensuring
the original source.manifest remains immutable and avoiding double-prefixing on
retries.
- Around line 149-151: Restrict the cross-origin shorthand so it cannot include
clientEntry: update the TransformAssetsCrossOriginConfig type (the union around
AssetCrossOrigin | Partial<Record<TransformAssetKind, AssetCrossOrigin>>) to
exclude TransformAssetKind.clientEntry from the Partial key set, or add
validation in the config resolution logic that detects and rejects a shorthand
object containing a clientEntry key; ensure the error message mentions
clientEntry is invalid for the shorthand and point users to use the full
AssetCrossOrigin shape for client-entry if needed.
---
Outside diff comments:
In `@packages/start-server-core/src/createStartHandler.ts`:
- Around line 472-490: The cachedCreateTransformPromise is being stored
unconditionally which preserves failures and ignores the dev-server
cache-bypass; modify getTransformFn (and use of cachedCreateTransformPromise) so
that caching only occurs when cache is enabled and process.env.TSS_DEV_SERVER
!== 'true', and ensure you never store a permanently rejected promise: when you
create the promise from resolvedTransformConfig.createTransform(opts) wrap it so
on rejection you clear cachedCreateTransformPromise (set it back to undefined)
before re-throwing the error; keep the same return behavior for
resolvedTransformConfig.transformFn when type !== 'createTransform'.
---
Nitpick comments:
In `@e2e/react-start/transform-asset-urls/src/server.ts`:
- Around line 6-16: Replace the local duplicated type alias TransformAssetsFn
with the exported type from the package by adding an import for
TransformAssetsFn from '@tanstack/react-start/server' (alongside the existing
TransformAssetUrls import) and remove the local type definition; update any
references to use the imported TransformAssetsFn type (search for the symbol
TransformAssetsFn and the local type block) so the test uses the canonical type
from the package.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 21495e80-71fe-4cc3-ab21-835f3fe3e65f
📒 Files selected for processing (31)
docs/router/guide/document-head-management.mddocs/start/framework/react/guide/cdn-asset-urls.mde2e/react-start/basic-rsc/src/routeTree.gen.tse2e/react-start/clerk-basic/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/routes/__root.tsxe2e/react-start/transform-asset-urls/src/routes/index.tsxe2e/react-start/transform-asset-urls/src/server.tse2e/react-start/transform-asset-urls/tests/cdn-server.mjse2e/react-start/transform-asset-urls/tests/transform-asset-urls.spec.tspackages/react-router/src/HeadContent.dev.tsxpackages/react-router/src/HeadContent.tsxpackages/react-router/src/Match.tsxpackages/react-router/src/headContentUtils.tsxpackages/react-router/tests/Scripts.test.tsxpackages/router-core/src/index.tspackages/router-core/src/manifest.tspackages/solid-router/src/HeadContent.dev.tsxpackages/solid-router/src/HeadContent.tsxpackages/solid-router/src/headContentUtils.tsxpackages/solid-router/tests/Scripts.test.tsxpackages/start-server-core/src/createStartHandler.tspackages/start-server-core/src/index.tsxpackages/start-server-core/src/router-manifest.tspackages/start-server-core/src/transformAssetUrls.tspackages/start-server-core/tests/transformAssets.test.tspackages/vue-router/src/HeadContent.dev.tsxpackages/vue-router/src/HeadContent.tsxpackages/vue-router/src/headContentUtils.tsxpackages/vue-router/tests/Scripts.test.tsx
schiller-manuel
force-pushed
the
transformAssets
branch
from
9ae82df to
b04c703
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/start-server-core/src/transformAssetUrls.ts (1)
413-431: Consider: All<link>tags treated as stylesheets.The transform uses
kind: 'stylesheet'for all<link>tags with anhref. If the manifest ever contains other link types (preconnect, preload for fonts, icons, etc.), they would be incorrectly categorized. Currently this appears safe given how assets are built, but worth documenting or adding a check forrel="stylesheet"if the manifest schema evolves.🔧 Optional: Add explicit rel check
if (asset.tag === 'link' && asset.attrs?.href) { + // Only transform stylesheet links; other link types should retain original URLs + const isStylesheet = asset.attrs.rel === 'stylesheet' const result = normalizeTransformAssetResult( await transformFn({ url: asset.attrs.href, - kind: 'stylesheet', + kind: isStylesheet ? 'stylesheet' : 'modulepreload', }), )🤖 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 413 - 431, The loop treating every <link> with an href as a stylesheet can misclassify other rel types; update the logic around route.assets iteration to inspect asset.attrs?.rel (and/or asset.attrs.rel?.split(/\s+/) includes 'stylesheet') before calling transformFn with kind: 'stylesheet', and for non-stylesheet rel values either skip transform or map rel to an appropriate kind when calling transformFn (use the same symbols: route.assets, asset.tag, asset.attrs.href, asset.attrs.rel, transformFn and normalizeTransformAssetResult) so only true stylesheet links are sent as kind 'stylesheet' and others are handled appropriately.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/start-server-core/src/transformAssetUrls.ts`:
- Around line 413-431: The loop treating every <link> with an href as a
stylesheet can misclassify other rel types; update the logic around route.assets
iteration to inspect asset.attrs?.rel (and/or asset.attrs.rel?.split(/\s+/)
includes 'stylesheet') before calling transformFn with kind: 'stylesheet', and
for non-stylesheet rel values either skip transform or map rel to an appropriate
kind when calling transformFn (use the same symbols: route.assets, asset.tag,
asset.attrs.href, asset.attrs.rel, transformFn and
normalizeTransformAssetResult) so only true stylesheet links are sent as kind
'stylesheet' and others are handled appropriately.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 92473a29-690c-4460-b801-6d34b03f9e1c
📒 Files selected for processing (5)
packages/react-router/src/Match.tsxpackages/start-plugin-core/src/start-manifest-plugin/manifestBuilder.tspackages/start-server-core/src/createStartHandler.tspackages/start-server-core/src/transformAssetUrls.tspackages/start-server-core/tests/transformAssets.test.ts
✅ Files skipped from review due to trivial changes (1)
- packages/start-server-core/tests/transformAssets.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/react-router/src/Match.tsx
- packages/start-server-core/src/createStartHandler.ts
tannerlinsley
approved these changes
Mar 22, 2026
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
2 participants
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.
replaces transformAssetUrls
Summary by CodeRabbit
New Features
Documentation
Tests