fix(app-router): isolate page CSS chunks in production

[NathanDrake2406]

Overview

Area	Detail
Goal	Match Next.js production App Router CSS isolation for sibling hash routes
Core change	Page modules in the generated RSC manifest load through cached dynamic import() loaders instead of static imports
Boundary	Production App Router page CSS chunks, prerender generateStaticParams, and intercepted/source-route renders
Primary files	packages/vinext/src/entries/app-rsc-manifest.ts, packages/vinext/src/entries/app-rsc-entry.ts, packages/vinext/src/server/app-rsc-handler.ts, tests/app-router.test.ts
Impact	Sibling page-level global CSS no longer leaks scroll-padding-top into unrelated hash pages

Why

Native hash scrolling uses the browser's layout state. Next.js calls scrollIntoView() for hash fragments, so a 20px-short scroll offset means unrelated CSS is present, not that hash math is wrong.

Area	Principle / invariant	What this PR changes
Route CSS	A matched page should only receive CSS for that page's route tree	Page modules are not statically imported into the shared RSC manifest chunk
Runtime loading	Route matching stays synchronous, rendering gets loaded modules	The handler loads the matched route once before page dispatch and action rerenders await matched/source routes
Prerender	Missing generateStaticParams must remain distinguishable from malformed return values	Lazy page sources return an internal missing-export sentinel consumed by the existing resolver

What changed

Scenario	Before	After
/hash with sibling /hash-with-scroll-offset	Linked CSS could include scroll-padding-top:20px from the sibling page	The no-offset route's linked CSS excludes the sibling page rule
Page generateStaticParams	Dynamic page modules were statically imported just to inspect an optional export	Page sources load lazily and report missing exports explicitly
Intercepted/action rerenders	Source route lookup assumed modules were already static imports	Source route lookups can await the same cached route-module loader

Validation

• vp test run tests/app-router.test.ts -t "keeps production route-level global CSS isolated"
• vp test run tests/app-router.test.ts tests/app-prerender-endpoints.test.ts tests/app-page-request.test.ts tests/app-page-dispatch.test.ts tests/app-server-action-execution.test.ts tests/app-rsc-handler.test.ts tests/entry-templates.test.ts
• vp check
• Commit hook: entry-template snapshots, vp check, and knip
• Upstream deploy suite: vp env exec --node 24 ./scripts/run-nextjs-deploy-suite.sh /Users/nathan/Projects/vinext/.refs/nextjs-v16.2.6 --retries 0 -c 1 --debug test/e2e/app-dir/navigation/navigation.test.ts
  • The original /hash-link-back-to-same-page scroll-offset failure now passes.
  • /hash-with-scroll-offset passes.
  • The suite still fails on separate pending navigation parity bugs: Pages useParams log capture, hash with-query-param RSC request detection, redirect-with-loading duplicate render, nested navigation, Promise proxy navigation, and scroll restoration.

Risk / compatibility

• Page modules now load at dispatch time rather than RSC manifest module initialization time.
• Static route matching remains synchronous because route metadata stays in the manifest.
• Root fallback boundaries and global-not-found behavior keep their existing loading model.
• Layout/template/error/loading modules are still static imports in this PR; this PR targets the page-level CSS leak observed in the upstream navigation fixture.

References

Reference	Why it matters
Next.js navigation hash tests	Source of the deploy-suite failure and expected hash-scroll behavior
Next.js layout-router hash scrolling	Shows hash navigation delegates to native scrollIntoView()
Next.js segment-cache navigation	Shows same-path hash navigations are treated as hash-only scroll updates

[pkg-pr-new]

Open in StackBlitz

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

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

commit: 885fe60

[james-elicx]

/bigbonk review

[ask-bonk]

Review: isolate page CSS chunks in production

Nice, well-structured change. The core idea — moving page modules from static manifest imports to cached dynamic import() loaders so a route only pulls in its own CSS chunk — is sound and matches Next.js's per-route CSS scoping. I traced the request, action, intercept, and prerender paths and the lazy-load gating is consistent.

Correctness — verified

• Request path: app-rsc-handler.ts:664-666 loads the matched route via loadRouteModules before dispatch, so handlePageRequest (route.page?.default, segment config, generateStaticParams) reads a populated route.page. ✓
• Action path: handleServerActionRequest is now async and awaits __loadRouteMatch(matchRoute(...)) before reading __actionMatch.route.page; the inner matchRoute option is also wrapped, so action rerenders/redirect targets load first. ✓
• Intercept path: findIntercept returns a per-request shallow copy ({ ...entry, matchedParams } in app-rsc-route-matching.ts:129), so the intercept.page = await intercept.__pageLoader() mutation in app-page-request.ts:336 does not mutate the shared lookup entry. ✓
• Prerender parity: the switch from mod?.generateStaticParams (filtered out when absent) to an always-present __createLazyGenerateStaticParamsSource(...) that returns the MISSING_GENERATE_STATIC_PARAMS sentinel is handled equivalently downstream. The build prerender path already treats a function-returning-null the same as a missing function (build/prerender.ts:1096 vs :1139), so the output: 'export' error and no-static-params skip behaviors are preserved. ✓

Notes (non-blocking)

1. No direct unit test for the new helper module. Per AGENTS.md ("If you move behavior out of a template string, add a focused unit test for the new helper module"), app-route-module-loader.ts has meaningful logic (WeakMap dedup, shared-promise reuse, page mutation) but is only covered indirectly via app-router.test.ts. A small focused test would lock in the dedup/caching contract.

2. Rejected loads are cached permanently. See inline comment — both createRouteModuleLoader and loadRouteModules cache the in-flight promise, so a transient import() failure poisons the route for the lifetime of the worker instance with no retry. Likely acceptable for built chunks, but worth a comment documenting the intent.

3. Minor inefficiency: intercept route modules are keyed in the loadRouteStates WeakMap by the per-request { ...entry } copy, so the route-level load cache is not reused across requests for intercepts (the underlying createRouteModuleLoader promise cache still dedups the actual import(), so this is harmless).

Nothing blocking from my read.

[ask-bonk]

Rejected loads are cached forever. If loadLazyModules(route) rejects, state.promise holds the rejected promise and state.loaded stays false, so every later loadRouteModules(route) call falls into the state.promise branch (line 47-50) and re-awaits the same rejection — the route is permanently broken within this worker instance with no retry. createRouteModuleLoader (line 34-40) has the same behavior. This is probably fine for built page chunks (a failure means a genuinely missing/broken chunk), but it's a deliberate-looking tradeoff that isn't documented. Consider a short comment noting the no-retry caching is intentional, or clearing state.promise on rejection if transient retries are desired.

[ask-bonk]

Behavioral note (not a bug): because lazy sources are now always functions, a dynamic route with a page that has no generateStaticParams now eagerly import()s the page module during prerender enumeration just to discover the export is absent (returning the sentinel). Previously the absent export was filtered out at resolver-build time without loading the module. This is fine since prerender runs at build time, but it does mean more module loads during generateStaticParams discovery. Worth confirming this doesn't measurably slow large App Router prerender builds.

[ask-bonk]

Review posted successfully to PR #1738.

Summary

I reviewed the PR, which isolates page CSS chunks in production by moving App Router page modules from static manifest imports to cached dynamic import() loaders.

Verdict: well-structured, nothing blocking.

I traced all four affected paths and confirmed the lazy-load gating is consistent:

• Request path — app-rsc-handler.ts:664-666 loads the route before dispatch, so route.page is populated when read.
• Action path — handleServerActionRequest correctly became async and awaits the loaded match before reading route.page.
• Intercept path — findIntercept returns a per-request shallow copy, so the intercept.page = await ... mutation doesn't corrupt the shared manifest lookup entry.
• Prerender parity — verified the new MISSING_GENERATE_STATIC_PARAMS sentinel design preserves the old behavior in both the runtime resolver and the build prerender path (output: 'export' errors and no-static-params skips still work, since the code already handled function-returns-null identically for the CF Workers Proxy case).

Non-blocking notes I flagged:

1. No direct unit test for the new app-route-module-loader.ts helper (AGENTS.md asks for one when extracting logic from template strings).
2. Rejected import()s are cached permanently with no retry (inline comment) — likely fine for built chunks but undocumented.
3. Minor: intercept WeakMap caching keyed on per-request copies (harmless, since the underlying loader promise still dedups).

github run

[NathanDrake2406]

Thanks for the review! Addressed all three non-blocking notes in 1e33ba5:

1. Unit test for the helper module — added tests/app-route-module-loader.test.ts (14 tests) covering createRouteModuleLoader (promise dedup, rejection propagation), loadRouteModules (page assignment, concurrent dedup, no-op cases, missing page key), loadRouteMatch (null short-circuit, match load), and createLazyGenerateStaticParamsSource (delegation, missing sentinel for null / non-function exports).

2. Rejected-load cache — added a comment in createRouteModuleLoader documenting that the loaders point at built chunks whose URL is stable for a deploy, so a rejection means the chunk is genuinely missing and rolling forward (rebuild/redeploy) is the recovery path. Rejecting within the same worker would just race a known-failing chunk and confuse error responses.

3. Per-request intercept copy in the WeakMap — added a comment above lazyRouteStates explaining that the WeakMap keys on the per-request shallow copy returned by findIntercept, so the loaded flag is per-request. The actual import() dedup still happens in the createRouteModuleLoader closure promise, which is keyed on the shared manifest entry, so concurrent loads for the same intercept still share a single in-flight import.

CI is green on a9852d2 — let me know if you want any further tweaks.

[NathanDrake2406]

Agreed — a positive case proving the modal CSS is emitted when the intercept is active is a good follow-up, but not blocking. The runtime already loads the intercept page before render (the __pageLoader resolution in resolveAppPageInterceptState), and the lazy page-CSS path is covered by the regular page tests, so the gap is purely a regression guard for the "intercept active" direction. Happy to add it in a follow-up if you'd like — filing nothing right now since you said it shouldn't block this PR.

[james-elicx]

/bigbonk review

[ask-bonk]

Re-review: isolate page CSS chunks in production

I independently traced all four request paths and re-ran the targeted suites. The change is sound and the three follow-ups from the prior review are addressed. Nothing blocking.

Verified

• Request path — app-rsc-handler.ts:663-666 awaits loadRouteModules(matchedRoute) before dispatchMatchedPage reads route.page?.default (app-rsc-entry.ts:595). ✓
• Action path — handleServerActionRscRequest is async; matchRoute is wired to __loadRouteMatch(matchRoute(...)) and getSourceRoute to __loadRouteModules(routes[i]), so route.page is populated before reads at app-server-action-execution.ts:645 (canRenderActionRedirectTarget) and app-rsc-entry.ts:797. ✓
• Intercept path — findIntercept returns a fresh shallow copy per call (app-rsc-route-matching.ts:129), so the intercept.page = await intercept.__pageLoader() mutation (app-page-request.ts:335-336) never touches the shared interceptLookup entry. The shared __pageLoader closure still dedups the underlying import(). ✓
• Prerender parity — the always-present lazy source returns MISSING_GENERATE_STATIC_PARAMS, and the new resolvedThisSource/resolvedAnySource tracking makes a layout-only pattern (page absent) behave identically to the old length === 1 branch. The sentinel is converted to null at the resolver boundary (lines 37, 78), which the build prerender path already treats the same as a missing function (build/prerender.ts:1096, :1117, :1135-1149) — so output: 'export' errors and no-static-params skips are preserved. ✓

Checks I ran locally

• vp test run tests/app-route-module-loader.test.ts tests/app-prerender-endpoints.test.ts tests/app-page-request.test.ts tests/entry-templates.test.ts → 75 passed
• vp test run tests/app-router.test.ts -t "global CSS isolated" → both isolation tests pass (sibling-route and intercepted-modal)
• vp check on the changed server modules + new test → clean (format, lint, types)

Notes (non-blocking, already acknowledged)

1. Rejected import() results are cached for the worker lifetime in both createRouteModuleLoader and loadRouteModules (a rejection poisons the route with no retry). The added comment documents this as intentional for stable built chunks; fine.
2. The intercept-modal test only asserts the negative direction (modal CSS absent on direct /feed visit). A positive guard (modal CSS present when the intercept is active) would lock in the lazy-load-on-intercept path as a regression guard — good follow-up, not blocking.

LGTM.

[ask-bonk]

Review posted successfully to PR #1738.

Summary

I independently re-reviewed the PR, which isolates App Router page CSS chunks in production by moving page modules from static manifest imports to cached dynamic import() loaders.

Verdict: well-structured, nothing blocking.

I traced all four affected paths against the actual code (not just the diff) and confirmed lazy-load gating is consistent:

• Request path — route loaded via loadRouteModules before dispatch reads route.page.
• Action path — handler became async; both matchRoute and getSourceRoute are wrapped with loaders, so route.page is populated before all reads.
• Intercept path — findIntercept returns a per-request shallow copy (app-rsc-route-matching.ts:129), so the intercept.page mutation never corrupts the shared lookup entry; the __pageLoader closure still dedups the underlying import().
• Prerender parity — the new MISSING_GENERATE_STATIC_PARAMS sentinel + resolvedThisSource/resolvedAnySource tracking makes a layout-only pattern behave identically to the old length === 1 branch. The sentinel→null conversion is handled equivalently by the build prerender path (verified at build/prerender.ts:1096/1117/1135-1149).

Verification I ran locally: 75 unit tests pass, both CSS-isolation integration tests pass, and vp check is clean on the changed modules.

The two non-blocking notes (permanent rejection caching, missing positive intercept-CSS assertion) were already acknowledged by the author and prior reviewer.

github run

[james-elicx]

/bigbonk review for issues

[ask-bonk]

UnknownError: "Overloaded"

github run

[ask-bonk]

@james-elicx Bonk workflow failed. Check the logs for details.

View workflow run · To retry, trigger Bonk again.

[james-elicx]

/bigbonk review for issues

[ask-bonk]

UnknownError: "Overloaded"

github run

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review for issues: isolate page CSS chunks in production

I traced all four lazy-load paths against the actual code (not just the diff) and ran the targeted suites locally. No blocking issues. The change is correct, the dedup/cache contract is sound, and prerender parity holds.

Verified correct

• Request path — app-rsc-handler.ts:670-673 awaits loadRouteModules(matchedRoute) before dispatchMatchedPage reads route.page?.default (app-rsc-entry.ts:597). ✓
• Action path — handleServerActionRscRequest is async; matchRoute is wrapped with __loadRouteMatch and getSourceRoute with __loadRouteModules, so route.page is populated before canRenderActionRedirectTarget reads it (app-server-action-execution.ts:645) and before the segment-config read at app-rsc-entry.ts:799. ✓
• Intercept path — findIntercept returns a per-request shallow copy (app-rsc-route-matching.ts:129) carrying the shared __pageLoader, so the intercept.page = await intercept.__pageLoader() mutation (app-page-request.ts:335-336) never corrupts the shared lookup entry, and the shared loader closure still dedups the import(). ✓
• Prerender parity — the always-present lazy source returning MISSING_GENERATE_STATIC_PARAMS, combined with resolvedThisSource/resolvedAnySource, makes a layout-GSP + missing-page-GSP route behave identically to the old filtered single-source path. The resolver's null return is treated equivalently to a missing function by the build path (build/prerender.ts:1096, :1117, :1139), so output: 'export' errors and no-static-params skips are preserved. ✓

Checks I ran

• vp test run tests/app-route-module-loader.test.ts tests/app-prerender-endpoints.test.ts → 28 passed
• vp test run tests/entry-templates.test.ts tests/app-page-request.test.ts → 48 passed
• vp test run tests/app-router-production-server.test.ts → 42 passed (both CSS-isolation tests green)

One new observation (non-blocking, scope note)

Parallel-slot page modules (slot.pagePath, app-rsc-manifest.ts:163) are still statically imported (imports.getImportVar), unlike the matched route page and the intercepting-route page on line 169 (which now use getLazyImportVar). A non-intercepting parallel-route page that imports route-global CSS would therefore still contribute its CSS to the shared manifest chunk — the same class of leak this PR fixes for the main page. This is consistent with the PR's stated scope, and the current fixtures don't exercise it (the @modal slot only has an intercepting page), so it's not a regression. Worth tracking as a follow-up so the remaining gap is explicit.

The other two known notes (permanent rejection caching, missing positive intercept-CSS assertion) were already raised and acknowledged. LGTM.

[ask-bonk]

This intercepting-route page is now lazy (getLazyImportVar), but the parallel-slot page on line 163 (if (slot.pagePath) imports.getImportVar(slot.pagePath);) is still a static import. A non-intercepting parallel-route page that imports route-global CSS would still leak its CSS into the shared manifest chunk — the same class of leak this PR fixes for the main page. Not a regression and out of this PR's scope (current fixtures only have an intercepting modal page), but worth a follow-up so the remaining gap is tracked.

[ask-bonk]

Review posted successfully to PR #1738.

Summary

I reviewed the PR for issues, tracing all four lazy-load paths against the actual source and running the targeted test suites locally.

Verdict: no blocking issues. The change is correct.

What I verified

• Request path — modules loaded before route.page is read in dispatch.
• Action path — handler is async, both matchRoute and getSourceRoute wrapped with loaders, so route.page is populated before all reads (including canRenderActionRedirectTarget).
• Intercept path — findIntercept returns a per-request shallow copy carrying the shared __pageLoader, so the intercept.page mutation never corrupts the shared lookup entry; the loader closure still dedups the import().
• Prerender parity — the MISSING_GENERATE_STATIC_PARAMS sentinel + resolvedThisSource/resolvedAnySource tracking preserves the old filtered-source behavior; the resolver's null return is treated identically to a missing function by build/prerender.ts.

Tests run locally (all green)

• app-route-module-loader + app-prerender-endpoints → 28 passed
• entry-templates + app-page-request → 48 passed
• app-router-production-server → 42 passed (both CSS-isolation tests)

One new observation (non-blocking)

Parallel-slot page modules (slot.pagePath) are still statically imported, unlike the main route page and intercepting-route pages. A non-intercepting parallel-route page importing route-global CSS would still leak into the shared chunk — same class of leak, but out of this PR's scope and not exercised by the fixtures. I flagged it inline as a follow-up.

The two previously-known notes (permanent rejection caching, missing positive intercept-CSS assertion) were already acknowledged by the author and prior reviews.

github run