fix(build): correct CSS ordering for global-not-found (#1549)

[james-elicx]

Finishes #1549 (initial work in #1654).

Problem

For app/global-not-found.tsx (the Next.js 16 route-miss 404 document that replaces the root layout), the production RSC build colocated React into the RSC entry chunk — which also carries the root layout's CSS. Because global-not-found.tsx imports that entry chunk for its React runtime helpers, it inherited the root layout's stylesheet in its serverResources metadata. The 404 document then linked the layout's CSS last, so the layout's rules won the cascade over global-not-found's own styles.

Fix

createRscFrameworkChunkOutputConfig (in packages/vinext/src/build/client-build-config.ts) isolates React (and the react-server-dom-webpack flight runtime) into a dedicated, CSS-free framework chunk in the RSC server build. global-not-found now imports that chunk for its React helpers instead of the layout-bearing entry chunk, so it no longer inherits the layout's CSS. The matcher mirrors the existing client-build framework chunk and returns the bundler-correct shape (advancedChunks for Vite 8+ / rolldown, manualChunks for Vite 7 / Rollup).

Test

Adds tests/app-router-global-not-found-css-order.test.ts, a production-build regression test (ported from upstream test/e2e/app-dir/initial-css-order — the global-not-found case):

• Builds the global-not-found-css-order fixture and serves it via the Vite preview server.
• Matched route (/): the root layout's CSS is served (green wins); global-not-found's CSS is absent.
• Route-miss 404 (/does-not-exist): only global-not-found's CSS is served — red wins (gnf-b imported last) and the layout's green does not leak in.

Verified the test fails without the framework-chunk split (the 404 leaks the layout's green) and passes with it.

Closes #1549

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: fix(build): correct CSS ordering for global-not-found (#1549)

I reviewed the fix end-to-end: verified the architecture, ran the new test, the broader build-optimization suite, the existing dev-server global-not-found test, and type/lint checks. The fix is correct and well-targeted — splitting React (+ the flight runtime) into a CSS-free framework chunk so global-not-found.tsx's dynamic-import chunk no longer transitively pulls in the RSC entry chunk's layout CSS. I confirmed the test fails-mode reasoning holds and all checks pass.

One non-blocking finding worth addressing before merge, plus a couple of minor notes.

1. (Should fix) New code uses the deprecated advancedChunks API; the client build already migrated to codeSplitting

createRscFrameworkChunkOutputConfig emits output.advancedChunks for the Vite 8+ branch. This is deprecated and prints a warning on every production build:

[warn] `advancedChunks` option is deprecated, please use `codeSplitting` instead.

The client build in this same repo already uses the non-deprecated form — getClientOutputConfigForVite (packages/vinext/src/index.ts:633-639) returns codeSplitting: clientCodeSplittingConfig for Vite 8+, built by createClientCodeSplittingConfig (client-build-config.ts:146). The rolldown types confirm advancedChunks is @deprecated in favor of codeSplitting, and codeSplitting.groups[].test accepts a regex (the rolldown docs example is literally test: /node_modules[\\/]react/).

The fix is a drop-in for the Vite 8 branch — same group shape under codeSplitting:

if (viteMajorVersion >= 8) {
  return {
    codeSplitting: {
      groups: [{ name: "framework", test: RSC_FRAMEWORK_CHUNK_TEST }],
    },
  };
}

This also aligns with the AGENTS.md guidance to treat advancedChunks as a migration target rather than a pattern to copy forward, and keeps the RSC build consistent with the client build (which uses minSize + groups too — worth considering whether the RSC framework group wants a minSize as well, though React always exceeds any reasonable threshold so it's not required).

2. (Minor) No focused unit test for the new version-branched helper

createRscFrameworkChunkOutputConfig is only exercised by the integration build test, which in this repo only runs the Vite 8 path. The Vite 7 (manualChunks) branch is untested. There's an established convention for exactly this in tests/build-optimization.test.ts — describe("getClientTreeshakeConfigForVite") asserts the v7/v8/v9/v10 branches. AGENTS.md asks for a focused unit test when adding helpers with branching like this. A small test asserting both branch shapes (and that RSC_FRAMEWORK_CHUNK_TEST / isRscFrameworkModule match react/react-dom/scheduler/react-server-dom-webpack but not react-icons / @react-aria/*) would lock in the behavior cheaply.

3. (Nit) Two sources of truth for the framework package list

RSC_FRAMEWORK_CHUNK_TEST (regex) and isRscFrameworkModule (via getPackageName) independently encode the same four-package list. They agree today, but they can drift. Consider deriving both from a single const FRAMEWORK_PACKAGES = [...] so the regex and the predicate can't diverge. Not blocking.

What I verified

• tests/app-router-global-not-found-css-order.test.ts — passes (2/2); confirms / keeps layout green and /does-not-exist resolves to red with no green leak.
• tests/build-optimization.test.ts (82), tests/entry-templates.test.ts (25), tests/nextjs-compat/global-not-found.test.ts (5) — all pass.
• vp check on the new test — clean (format, lint, types).
• Regex edge cases — correctly excludes react-icons and @react-aria/*, handles pnpm paths.
• output: placement in the RSC build block doesn't overwrite any pre-existing output config.

Nice fixture and test — the comment explaining why it must be a production build test (and why blue gets minified away while green/red survive) is genuinely useful.

[ask-bonk]

Review posted to PR #1858.

Summary

The fix is correct and well-targeted. It isolates React + the RSC flight runtime into a dedicated CSS-free framework chunk in the RSC server build, so global-not-found.tsx's dynamic-import chunk stops transitively inheriting the root layout's CSS from the RSC entry chunk. I verified the new regression test passes, along with the build-optimization, entry-templates, and existing dev-server global-not-found suites, plus clean type/lint checks.

Findings I posted:

1. (Should fix) The new createRscFrameworkChunkOutputConfig uses the deprecated advancedChunks API for Vite 8+, which prints a warning on every prod build — even though the client build in the same repo already migrated to codeSplitting. I gave the drop-in replacement. This also matches AGENTS.md's guidance to treat advancedChunks as a migration target.

2. (Minor) No focused unit test for the version-branched helper; the Vite 7 manualChunks branch is untested. There's an existing convention (getClientTreeshakeConfigForVite tests) to follow.

3. (Nit) The package list is duplicated between the regex (RSC_FRAMEWORK_CHUNK_TEST) and the predicate (isRscFrameworkModule) — could drift; suggested a single source of truth.

No correctness or dev/prod parity bugs found — the regex correctly avoids over-matching (react-icons, @react-aria/*), handles pnpm paths, and the output: placement doesn't clobber existing config.

github run

[james-elicx]

/bigbonk review for issues

[pkg-pr-new]

Open in StackBlitz

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

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

commit: ab1a7ea

[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

[ask-bonk]

The output is

github run

[james-elicx]

/bigbonk review for issues

[james-elicx]

/bigbonk review for issues

[ask-bonk]

@james-elicx Bonk workflow was cancelled.

View workflow run · To retry, trigger Bonk again.

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Re-review: fix(build): correct CSS ordering for global-not-found (#1549)

The latest commit (ab1a7ea) addresses all three findings from the prior review round. The fix is correct, well-targeted, and ready to merge.

Prior findings — all resolved

1. (was Should fix) advancedChunks → codeSplitting — createRscFrameworkChunkOutputConfig now emits codeSplitting.groups for Vite 8+ (client-build-config.ts:209-216), matching the client build's non-deprecated form. No more deprecation warning on every prod build. ✅
2. (was Minor) Missing focused unit test — both branches are now covered: describe("createRscFrameworkChunkOutputConfig") asserts the Vite 7 manualChunks shape and the Vite 8/9 codeSplitting shape (and explicitly that it is not advancedChunks/manualChunks). ✅
3. (was Nit) Two sources of truth — RSC_FRAMEWORK_CHUNK_TEST and isRscFrameworkModule now both derive from a single FRAMEWORK_PACKAGES const (client-build-config.ts:185), and a describe("RSC framework package matching") block verifies both stay in agreement (react/react-dom/scheduler/react-server-dom-webpack match; react-icons, @react-aria/*, and pnpm paths handled correctly). ✅

What I verified on this revision

• tests/build-optimization.test.ts — new createRscFrameworkChunkOutputConfig (2/2) and RSC framework package matching (2/2) suites pass.
• tests/app-router-global-not-found-css-order.test.ts — passes (2/2): / keeps the layout's green, /does-not-exist resolves to background-color:red with no green leak.
• tests/nextjs-compat/global-not-found.test.ts (5) + tests/entry-templates.test.ts (25) — pass, no regressions.
• vp check on client-build-config.ts and index.ts — clean (format, lint, types). The codeSplitting.groups[].test: RegExp shape type-checks against the Vite 8 / rolldown types.

Correctness / parity checks

• No output clobbering: the rsc: environment build block (index.ts:2082-2095) previously set only input; adding output does not overwrite any pre-existing RSC output config in that object. End-to-end build + preview confirms no plugin-level conflict.
• Dev/prod + Cloudflare parity: the framework-chunk split is applied to the rsc environment unconditionally — the hasCloudflarePlugin || hasNitroPlugin guard only gates the resolve.external/noExternal block, not build.output. So both the standard App Router prod build and the Cloudflare Workers build inherit the fix. No separate worker build path needs a duplicate fix.
• No minSize on the RSC framework group is intentional and correct: omitting it guarantees the split happens regardless of chunk size, which is exactly what the CSS-isolation goal requires (React always exceeds any threshold anyway).
• Regex robustness: [\\\\/]node_modules[\\\\/](...)[\\\\/] correctly matches the inner node_modules/react/ segment of pnpm paths and avoids over-matching react-icons / @react-aria/*.

LGTM.

[ask-bonk]

Review posted to PR #1858.

Summary

I re-reviewed PR #1858 at its current head (ab1a7ea). The latest commit already resolved all three findings from the prior review round, and the fix is correct and ready to merge.

Prior findings — all resolved:

1. Migrated from the deprecated advancedChunks to codeSplitting for Vite 8+, matching the client build (no more per-build deprecation warning).
2. Added focused unit tests covering both the Vite 7 manualChunks and Vite 8/9 codeSplitting branches.
3. Collapsed the duplicated package list into a single FRAMEWORK_PACKAGES const that both the regex and predicate derive from, with a test asserting they agree.

What I verified:

• New unit tests + integration test pass (/ keeps layout green; /does-not-exist resolves to red with no green leak).
• No regressions in nextjs-compat/global-not-found and entry-templates.
• vp check clean on changed source.

Correctness/parity checks (no new issues found):

• The added output in the rsc: env block doesn't clobber any pre-existing config.
• The framework-chunk split applies to both the standard App Router and Cloudflare Workers builds (the cloudflare guard only gates resolve, not build.output) — no parity gap.
• Omitting minSize on the RSC framework group is intentional and correct for guaranteeing the CSS-isolating split.
• The regex correctly handles pnpm paths and avoids over-matching react-icons/@react-aria/*.

LGTM — no blocking issues.

github run