[james-elicx]

Summary

• apply Next-compatible static/media asset naming to App Router RSC and SSR build environments
• preserve explicit user assetFileNames configuration
• reproduce the issue through the Cloudflare multi-environment build and require SSR CSS URLs to match files in dist/client

Fixes #2290.

Next.js parity

Ported behavior from test/e2e/app-dir/scss/url-global/url-global.test.ts, which expects CSS url() assets under /_next/static/media/ and verifies those URLs are served.

Validation

• vp test run tests/css-url-assets.test.ts tests/build-optimization.test.ts (158 passed)
• vp check packages/vinext/src/index.ts tests/css-url-assets.test.ts tests/build-optimization.test.ts
• git diff --check

[james-elicx]

/bigbonk review for issues

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/@vinext/cloudflare@2291

npm i https://pkg.pr.new/vinext@2291

commit: 5b70139

[github-actions]

Example	Preview	Production	Original
app-router-cloudflare	preview	production
pages-router-cloudflare	preview	production
app-router-playground	preview	production	original
realworld-api-rest	preview	production
nextra-docs-template	preview	production
benchmarks	preview	production
hackernews	preview	production	original
workers-cache	preview	production
web	preview	production

[github-actions]

Performance benchmarks

Compared 5b70139 against base 4d90f8b using alternating same-runner rounds. Next.js was unchanged and skipped.

0 improved · 0 regressed · 6 within ±1.5%

Scenario	Framework	Baseline	Current	Change
Client bundle size (gzip)	vinext	117.1 KB	117.1 KB	⚫ -0.0%
Client entry size (gzip)	vinext	112.5 KB	112.5 KB	⚫ -0.0%
Dev server cold start	vinext	2.48 s	2.46 s	⚫ -0.9%
Production build time	vinext	2.89 s	2.89 s	⚫ +0.1%
RSC entry closure size (gzip)	vinext	92.8 KB	92.8 KB	⚫ -0.0%
Server bundle size (gzip)	vinext	156.6 KB	156.6 KB	⚫ -0.0%

View detailed results and traces

_{🟢 improvement · 🔴 regression · ⚫ change below 1.5% · paired base/head}

[ask-bonk]

@james-elicx Bonk workflow was cancelled.

View workflow run · To retry, trigger Bonk again.

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: fix(build): align SSR CSS asset paths

I reviewed the change against the Next.js parity reference and verified behavior locally. This is a solid, well-scoped fix. Summary below.

Correctness — looks good

• Root cause is addressed correctly. The vinext:css-url-assets-defaults plugin now applies createClientAssetFileNames(resolveAssetsDir(...)) to the rsc/ssr environments, so SSR-emitted CSS url() references land under /_next/static/media/ instead of the bare /_next/static/<name>-<hash>.<ext> shape. This matches the client environment exactly — both call createClientAssetFileNames with the same resolveAssetsDir(nextConfig.assetPrefix ?? "") expression (index.ts:1844 vs index.ts:3117), so the emitted path and the content hash align, and the test's expect(serverAssetUrls).toEqual(clientAssetUrls) + on-disk dist/client existence check holds.

• Hook ordering is safe. configEnvironment reads hasAppDir and nextConfig, both populated by the main config hook. Vite runs all config hooks before configEnvironment, and the unit test exercises this (calls mainPlugin.config() then configEnvironment("ssr", ...)), so the dependency is real but correctly ordered and covered.

• No conflict with the RSC framework-chunk output. createRscFrameworkChunkOutputConfig sets only codeSplitting/manualChunks, not assetFileNames, so Vite's deep-merge keeps both. Good.

• User override is respected. The hasAssetFileNames guard (handling both array and object output) correctly bails when the user set their own assetFileNames, matching the stated goal and the new "custom assetFileNames → null" test case.

Next.js parity

Confirmed the ported behavior matches test/e2e/app-dir/scss/url-global/url-global.test.ts — Next.js serves CSS url() assets under /_next/static/media/ and verifies they resolve. The fix brings SSR output in line with that contract.

Validation (ran locally)

• vp test run tests/build-optimization.test.ts → 149 passed
• vp test run tests/css-url-assets.test.ts → 9 passed
• vp check packages/vinext/src/index.ts tests/css-url-assets.test.ts tests/build-optimization.test.ts → clean (format, lint, types)

Minor observations (non-blocking)

1. Array output write path. The read side detects assetFileNames across array elements, but the write side always returns a single object output via withBuildBundlerOptions. If a user supplied an array output for rsc/ssr without any assetFileNames, merging an object-shaped output into an array-shaped one could behave unexpectedly. This is an extremely rare config for server environments and matches existing patterns in the file, so it's fine to leave — just flagging it.

2. Test coverage shifted from plain multi-env to Cloudflare-only. The App Router integration test was rewritten to build through @cloudflare/vite-plugin (which faithfully reproduces the reported scenario in #2290). The fix itself is Cloudflare-independent (configEnvironment for rsc/ssr doesn't check for the Cloudflare plugin), so the non-Cloudflare multi-env path is no longer directly asserted for SSR-CSS parity. Acceptable since the new test reproduces the actual bug report, but worth noting in case a future regression slips into a non-Cloudflare build.

Overall: correct, well-tested, and consistent with repo conventions and Next.js behavior. LGTM.

[ask-bonk]

Review posted on PR #2291. Summary of my findings:

Verdict: LGTM — correct, well-tested, no blocking issues

The fix correctly resolves #2290 (SSR CSS url() assets pointing at non-client paths).

Key correctness checks (all pass):

• SSR/RSC now use the same createClientAssetFileNames(resolveAssetsDir(...)) as the client env (identical expression at index.ts:1844 and index.ts:3117), so paths and content hashes align — the URLs resolve against dist/client.
• Hook ordering is safe: configEnvironment depends on hasAppDir/nextConfig set by the config hook, and Vite guarantees config runs first (the unit test exercises this).
• No merge conflict with createRscFrameworkChunkOutputConfig (it only sets codeSplitting, not assetFileNames).
• User assetFileNames override is correctly respected and tested.

Parity: Matches the ported Next.js test (url-global.test.ts) which expects assets under /_next/static/media/.

Validation (ran locally): both test files pass (149 + 9), and vp check is clean.

Minor non-blocking notes:

1. The array-output write path always returns a single-object output — a theoretical edge case if a user supplies an array output without assetFileNames for rsc/ssr (extremely rare).
2. The App Router integration test was rewritten to go through the Cloudflare plugin (faithfully reproducing SSR CSS url() assets reference non-client media paths #2290), so the non-Cloudflare multi-env SSR-CSS path is no longer directly asserted — acceptable since the fix is Cloudflare-independent.

github run

[james-elicx]

/bigbonk re-review

[ask-bonk]

Re-review: fix(build): align SSR CSS asset paths

Re-reviewed after the new commit 4ca24e6 ("test(build): cover SSR asset naming edge cases"). The two non-blocking notes from my prior review were addressed — array-output detection now has a matching write path, and the App Router integration test was widened to cover both the plain multi-env build and the Cloudflare build via describe.each(["plain", "cloudflare"]).

Verified locally:

• vp test run tests/build-optimization.test.ts → 149 passed
• vp test run tests/css-url-assets.test.ts → 11 passed (the App Router cases now run twice — plain + cloudflare)
• vp check packages/vinext/src/index.ts tests/css-url-assets.test.ts tests/build-optimization.test.ts → clean

The core fix is still correct: rsc/ssr now use the identical createClientAssetFileNames(resolveAssetsDir(nextConfig.assetPrefix ?? "")) as the client env, so SSR CSS url() assets land under /_next/static/media/ and resolve against dist/client. Hook ordering, the user-override bail-out, and the object-output merge path all check out (mergeConfig merges the single-object case cleanly).

One real issue: the array-output write path is defeated by mergeConfig

The new array branch:

output: Array.isArray(output)
  ? output.map((item) => ({ ...item, assetFileNames }))
  : { assetFileNames },

is returned from configEnvironment, and Vite applies it with environments[name] = mergeConfig(environments[name], res) (vite-plus-core, the configEnvironment runner). mergeConfig concatenates arrays, it does not merge them by index. So for a user-supplied array output without assetFileNames, the result is 4 output entries, not 2:

// base output (user)
[ { entryFileNames: "first.js" }, { chunkFileNames: "second.js" } ]
// + returned (mapped copy)
[ { entryFileNames: "first.js", assetFileNames: fn }, { chunkFileNames: "second.js", assetFileNames: fn } ]
// = merged
[
  { entryFileNames: "first.js" },                          // original — still NO assetFileNames
  { chunkFileNames: "second.js" },                         // original — still NO assetFileNames
  { entryFileNames: "first.js", assetFileNames: fn },      // appended duplicate
  { chunkFileNames: "second.js", assetFileNames: fn }
]

Reproduced directly with mergeConfig from the resolved vite (vite version: 8.0.16).

Effect: Rolldown would emit one output bundle per array entry, so the original outputs still emit assets at the non-Next path (the bug this PR fixes), plus two duplicate output passes. The new unit test passes only because it asserts on the raw return value of configEnvironment in isolation — it never goes through the mergeConfig step that runs in a real build, so it doesn't catch the concatenation.

This is an extremely rare config (an array output for a server environment), it pre-existed before this PR, and the common object path is fixed and well-covered — so I don't consider it a merge blocker. But the array branch as written doesn't actually achieve its goal, and the test gives false confidence. Two options:

1. Drop the array branch and just document that array output for rsc/ssr isn't supported (keep the single-object return; the hasAssetFileNames array read check can stay to bail out safely).
2. Keep it but test through mergeConfig (or an actual build) so the assertion reflects what Vite really does — which will surface the concatenation and force a real fix (e.g., not returning an array at all, since you can't index-merge via mergeConfig).

Everything else LGTM.

[ask-bonk]

Re-review posted on PR #2291.

Summary

The new commit 4ca24e6 addressed both non-blocking notes from the prior review (array-output detection write path + widening the integration test to describe.each(["plain", "cloudflare"])). All local checks pass: 149 build-optimization tests, 11 css-url-assets tests, and vp check clean.

The core fix remains correct — rsc/ssr now use the identical createClientAssetFileNames/resolveAssetsDir expression as the client env, so SSR CSS url() assets land under /_next/static/media/ and resolve against dist/client.

One real (but very narrow) issue I found: the new array-output write path is defeated by Vite's mergeConfig. The configEnvironment return value is applied via mergeConfig(environments[name], res), and mergeConfig concatenates arrays rather than merging by index. I reproduced this against the resolved vite 8.0.16 — a user-supplied array output without assetFileNames yields 4 output entries (2 originals still missing assetFileNames + 2 appended copies), not the intended 2. The new unit test passes only because it asserts on the raw return value in isolation and never exercises mergeConfig, so it gives false confidence.

This is an extremely rare config (array output for a server environment), it pre-existed this PR, and the common object path is fixed and well-covered — so not a merge blocker, but I flagged it with two remediation options (drop the array branch, or test through mergeConfig/a real build so the test reflects actual Vite behavior).

github run

[james-elicx]

/bigbonk re-review

[ask-bonk]

Re-review: fix(build): align SSR CSS asset paths

Re-reviewed after the new commit 5b70139 ("fix(build): preserve array output configuration"). This commit directly addresses the one real issue I flagged in my previous re-review — the array-output write path being defeated by Vite's mergeConfig.

The array-output issue is resolved

The author took option 1 from my prior review: drop the array-augmentation branch entirely and bail out when output is an array.

// Vite concatenates arrays returned from config hooks rather than
// merging output entries by index, so an array-shaped user config
// cannot be safely augmented here. Preserve it unchanged.
if (Array.isArray(output) || output?.assetFileNames !== undefined) return null;

This is correct:

• The mergeConfig array-concatenation footgun is gone — we never return an array output, so there's nothing for Vite to mis-merge.
• The comment accurately documents why (index-merge is impossible via mergeConfig), so the constraint won't be accidentally reintroduced.
• The object-output path is unchanged and still correct: it returns { rolldownOptions: { output: { assetFileNames } } }, which mergeConfig deep-merges cleanly into a user's object-shaped output.
• The user-override bail-out (output?.assetFileNames !== undefined) is preserved.

The unit test was updated to assert the new behavior — an array-shaped output now expect(...).toBeNull() (tests/build-optimization.test.ts:913-925). This replaces the previously misleading assertion that checked the raw mapped return value (which never went through mergeConfig and gave false confidence). The test now reflects what actually happens in a real build.

Trade-off is acceptable

The behavioral consequence: if a user supplies an array output for the rsc/ssr environment without assetFileNames, vinext no longer injects the Next-compatible static/media naming for those environments, so SSR CSS url() assets would fall back to the default path. This is an extremely rare config for a server environment, it's strictly safer than the previous (broken) array branch which produced duplicate output passes, and the common object path — the one that actually fixes #2290 — is correct and well-covered. Bailing out cleanly is the right call.

Verified locally

• vp test run tests/build-optimization.test.ts tests/css-url-assets.test.ts → 160 passed (149 + 11)
• vp check packages/vinext/src/index.ts tests/build-optimization.test.ts tests/css-url-assets.test.ts → clean (format, lint, types)

Verdict: LGTM — ship it

The core fix remains correct (rsc/ssr use the identical createClientAssetFileNames(resolveAssetsDir(nextConfig.assetPrefix ?? "")) as the client env, so SSR CSS url() assets land under /_next/static/media/ and resolve against dist/client), and the one outstanding issue from my prior review is now properly resolved with a matching test. No remaining blocking or non-blocking concerns.

[ask-bonk]

Re-review posted on PR #2291.

Summary

The new commit 5b70139 ("fix(build): preserve array output configuration") resolves the one real issue I flagged in my previous re-review — the array-output write path being defeated by Vite's mergeConfig array concatenation.

The author took option 1: drop the array-augmentation branch and bail out (return null) when output is an array, with a comment documenting why index-merging is impossible via mergeConfig. The misleading unit test (which asserted on the raw mapped return value and never exercised mergeConfig) was replaced with one that asserts the array case now returns null — so the test reflects actual build behavior.

Verified locally:

• vp test run tests/build-optimization.test.ts tests/css-url-assets.test.ts → 160 passed (149 + 11)
• vp check on the three changed-ish files → clean

The core fix remains correct (rsc/ssr use the identical createClientAssetFileNames/resolveAssetsDir expression as the client env, so SSR CSS url() assets land under /_next/static/media/). No remaining blocking or non-blocking concerns. LGTM.

github run