Skip to content

fix(prerender): expose the production build phase - #2846

Merged
james-elicx merged 2 commits into
mainfrom
codex/fix-cache-components-inline-actions
Aug 10, 2026
Merged

fix(prerender): expose the production build phase#2846
james-elicx merged 2 commits into
mainfrom
codex/fix-cache-components-inline-actions

Conversation

@james-elicx

Copy link
Copy Markdown
Member

Summary

  • expose NEXT_PHASE=phase-production-build while prerender workers and in-process prerender servers execute application code
  • preserve and restore the caller environment safely across nested and overlapping prerender scopes
  • load omitted functional next.config.* values with the production-build phase
  • add regression coverage for prerendering inline server actions that capture JSX and async component values

Validation

  • REPO="$(pwd)" NEXTJS_DIR="/Users/jamesanderson/Developer/vinext/.nextjs-ref" ./scripts/run-targeted-nextjs-e2e.sh test/e2e/app-dir/cache-components/cache-components.server-action.test.ts — 3/3 passed
  • vp test run tests/prerender-phase.test.ts tests/run-prerender-concurrency.test.ts tests/prerender-server-pool.test.ts — 12/12 passed
  • vp test run tests/prerender.test.ts -t "renders inline server actions during the production build phase" — passed
  • focused vp check for all changed source and test files — passed

The targeted Next.js E2E is the primary acceptance check for this fix.

@james-elicx

Copy link
Copy Markdown
Member Author

/bigbonk review for issues

@pkg-pr-new

pkg-pr-new Bot commented Aug 10, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/@vinext/cloudflare@2846
npm i https://pkg.pr.new/create-vinext-app@2846
npm i https://pkg.pr.new/@vinext/types@2846
npm i https://pkg.pr.new/vinext@2846

commit: f039b92

@github-actions

Copy link
Copy Markdown
Contributor
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

ask-bonk Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

/bigbonk review — PR #2846 fix(prerender): expose the production build phase

Reviewed all 12 changed files, ran the targeted tests, and checked dev/prod parity across the prerender code paths. No blocking issues found. The change is correct, well-scoped, and the test coverage matches Next.js behavior.

What the PR does correctly

enterPrerenderPhase() is a sound reference-counted scope.

  • The WeakMap<env, state> keyed on the shared process.env object means all nested/overlapping entries (runPrerenderprerenderApp/prerenderPages, and withPrerenderEndpoints) increment/decrement one depth counter, and only the final restore rewrites the env. The captured previousNextPhase/previousPrerenderFlag are snapshotted on the first (outermost) entry, so the ordinary caller phase is restored exactly once. The restored idempotency guard also makes double-invocation safe.
  • The overlapping-scope test (tests/prerender-phase.test.ts:20) correctly proves order-independence: restoreOuter() before restoreInner() still leaves the phase active until the last restore, then restores phase-production-server.

Ordering is correct in every entry point.

  • enterPrerenderPhase() is called before startProdServer/startPrerenderServerPool in both run-prerender.ts:227 and prerender.ts:1044, so VINEXT_PRERENDER/NEXT_PHASE are set before any server or worker reads them. The forked pool (prerender-server-pool.ts:147-148) passes both vars through the fork env, matching the "set before any module loads" contract documented in prerender-server-entry.ts.

Config phase is threaded explicitly, not via env.

  • loadNextConfig(root, PHASE_PRODUCTION_BUILD) at run-prerender.ts:176 and prerender-paths.ts:378 passes the phase as a function argument, so functional (phase) => config configs observe phase-production-build regardless of when process.env.NEXT_PHASE is mutated. This is the correct approach and matches how Next.js invokes config functions. Verified end-to-end by tests/run-prerender-concurrency.test.ts:73.

The finally restructuring is a strict improvement, not a regression.

  • The new nested try { close pool; restore cache handler; close server } finally { restorePrerenderPhase() } guarantees the phase is always restored even if pool/server teardown throws. The relative ordering of renderPool.close()setCacheHandler(previousHandler) is unchanged from main, so no cache-handler-restore behavior changed.

Test coverage

  • tests/prerender-phase.test.ts — unit coverage for enter/restore + overlapping scopes.
  • tests/prerender-server-pool.test.ts:75 — asserts forked workers boot with VINEXT_PRERENDER=1 and NEXT_PHASE=phase-production-build.
  • tests/prerender.test.ts:929 — the ported Next.js regression (inline server action capturing JSX + async component values) asserts <div id="phase">at buildtime</div> and, importantly, that NEXT_PHASE is restored to phase-production-server afterward. Good defense against scope leakage.
  • Includes the AGENTS.md-required back-link to the source Next.js test (cache-components.server-action.test.ts).

Ran locally:

  • vp test run tests/prerender-phase.test.ts tests/run-prerender-concurrency.test.ts tests/prerender-server-pool.test.ts12/12 passed
  • vp check on all 6 changed source files → format + lint + types clean

Minor nits (non-blocking)

  1. PrerenderPhaseEnv.NODE_ENV? (prerender-phase.ts:11) is declared but never read or written. It can be dropped from the type to avoid implying the helper touches NODE_ENV.
  2. A next.config that reads process.env.NEXT_PHASE at module-evaluation time (rather than via the (phase) => … functional form) would still see the caller's phase, because loadNextConfig runs before enterPrerenderPhase(). This matches Next.js (which only supplies the phase via the functional arg), so it's the correct behavior — worth a one-line comment if you want to make the intent explicit, but not required.

LGTM.

@ask-bonk

ask-bonk Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Review posted to PR #2846: #2846 (comment)

Summary

I reviewed all 12 changed files, ran the targeted tests, and verified dev/prod parity across the prerender code paths. No blocking issues found.

Key findings

Correct:

  • enterPrerenderPhase() is a sound reference-counted scope. The WeakMap keyed on process.env + depth counter handles all nested/overlapping entries, snapshots the caller phase once on outermost entry, and the restored flag makes double-invocation safe. The overlapping-scope test proves order-independence.
  • Ordering is correct everywhere: enterPrerenderPhase() runs before startProdServer/startPrerenderServerPool, so env vars are set before any server or forked worker reads them.
  • Config phase is threaded explicitly via loadNextConfig(root, PHASE_PRODUCTION_BUILD) (a function arg, not env), which is the right approach and matches Next.js.
  • The finally restructuring is a strict improvement — it guarantees phase restoration even if teardown throws, without changing the existing cache-handler-restore ordering.

Verified locally:

  • 12/12 targeted tests passed (prerender-phase, run-prerender-concurrency, prerender-server-pool)
  • vp check clean on all 6 changed source files

Minor nits (non-blocking):

  1. PrerenderPhaseEnv.NODE_ENV? is declared but never used — can be dropped.
  2. A config reading process.env.NEXT_PHASE at module-eval time won't see the build phase (loads before enterPrerenderPhase()), but this matches Next.js behavior.

The PR is well-scoped with test coverage that mirrors the ported Next.js regression, including the required source back-link.

github run

@github-actions

Copy link
Copy Markdown
Contributor

Performance benchmarks

Compared f039b92 against base 9962354 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 134.7 KB 134.7 KB ⚫ +0.0%
Client entry size (gzip) vinext 122.2 KB 122.2 KB ⚫ -0.0%
Dev server cold start vinext 3.22 s 3.20 s ⚫ -0.5%
Production build time vinext 3.51 s 3.49 s ⚫ -0.7%
RSC entry closure size (gzip) vinext 114.2 KB 114.3 KB ⚫ +0.0%
Server bundle size (gzip) vinext 191.8 KB 191.9 KB ⚫ +0.0%

View detailed results and traces

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

@james-elicx
james-elicx marked this pull request as ready for review August 10, 2026 01:33
@james-elicx

Copy link
Copy Markdown
Member Author

Mapping note for the cache-components backlog from Actions run 31290819291: I think this PR fixes test/e2e/app-dir/cache-components/cache-components.server-action.test.ts (should prerender pages with inline server actions). Its targeted Next.js wrapper passes 3/3.

@james-elicx
james-elicx merged commit acc2b20 into main Aug 10, 2026
66 checks passed
@james-elicx
james-elicx deleted the codex/fix-cache-components-inline-actions branch August 10, 2026 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant