fix(app-router): exclude dev overlay from production client bundle by ponharu · Pull Request #2284 · cloudflare/vinext

ponharu · 2026-06-23T04:18:14Z

Summary

Move App Router dev error overlay imports behind dev-only dynamic imports
Keep RSC HMR overlay cleanup inside the dev/HMR code path
Add a production build regression test that rejects dev overlay/HMR-only strings in client chunks

Verification

pnpm test tests/app-router-production-build.test.ts
PLAYWRIGHT_PROJECT=app-router pnpm exec playwright test --project=app-router tests/e2e/app-router/dev-error-overlay.spec.ts
pnpm run check

pkg-pr-new · 2026-06-23T04:19:29Z

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

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

commit: 795a8c0

github-actions · 2026-06-23T04:22:43Z

Performance benchmarks

Compared 795a8c0 against base 767c79a 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.2 KB	117.1 KB	⚫ -0.1%
Client entry size (gzip)	vinext	112.6 KB	112.5 KB	⚫ -0.1%
Dev server cold start	vinext	2.27 s	2.24 s	⚫ -1.3%
Production build time	vinext	2.61 s	2.58 s	⚫ -1.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}

james-elicx · 2026-06-23T08:42:05Z

/bigbonk review for issues and edge cases

ask-bonk

Reviewed the change. The fix is correct and well-scoped.

What the bug was: the App Router browser entry statically imported ./dev-error-overlay.js at the top of the module, so the ~1800-line dev overlay (plus its react-dom/client createRoot usage) was pulled into the production client bundle even though it was only referenced inside if (import.meta.env.DEV) / if (import.meta.hot) blocks. import.meta.hot alone is only undefined at runtime in prod — it does not let the bundler statically dead-code-eliminate the block, and the static import kept the module alive regardless.

Why this fix works:

The top-level static import is replaced with a typeof import(...) type-only import (erased at compile time) plus dynamic await import() behind import.meta.env.DEV.
Changing the HMR guard from if (import.meta.hot) to if (import.meta.env.DEV && import.meta.hot) (line 2115) is the key bit: import.meta.env.DEV is statically replaced with false in prod, so Rolldown can eliminate the whole block, including the "rsc:update" literal and the second dynamic import of the overlay.
The null-fallback in bootstrapHydration correctly routes to the prod onCaughtError/onRecoverableError options.

Parity: This now matches the pattern already used by the Pages Router client entry (pages-client-entry.ts:173), so the two entries are consistent. No Cloudflare worker entry imports the overlay, so nothing else is affected.

Verification: vp test run tests/app-router-production-build.test.ts passes (9/9) and vp check on the modified file is clean.

One non-blocking nit inline.

ask-bonk · 2026-06-23T08:46:09Z

      // re-populates it. Without this, an old "DropZone is not defined"
      // error would linger after the developer fixed the bug.
-      dismissOverlay();
+      const devErrorOverlay = await import("./dev-error-overlay.js");


Non-blocking: this is a second dynamic import("./dev-error-overlay.js") site. It's harmless (the module is already loaded in main() at line 1397 and the import resolves from cache, and installDevErrorOverlay is idempotent via its installed guard), so functionally fine. Just noting it reads a little redundantly versus the eager load in main(). Not worth changing unless you want to thread the already-loaded reference through.

ask-bonk · 2026-06-23T08:46:20Z

Review posted successfully. Here's my summary:

Review: PR #2284 — exclude dev overlay from production client bundle

Verdict: Correct and well-scoped. Approving with one non-blocking nit.

What the bug was

The App Router browser entry statically imported ./dev-error-overlay.js at module top. Even though the overlay was only used inside if (import.meta.env.DEV) / if (import.meta.hot) blocks, the static import kept the ~1800-line module (and its react-dom/client createRoot) alive in the production client bundle. import.meta.hot is only undefined at runtime — it doesn't enable static dead-code elimination.

Why the fix is right

Static import → typeof import(...) type-only import (erased) + dynamic await import() behind import.meta.env.DEV.
The critical change is the HMR guard at line 2115: if (import.meta.hot) → if (import.meta.env.DEV && import.meta.hot). import.meta.env.DEV is statically replaced with false in prod, so Rolldown eliminates the entire block (including the "rsc:update" literal and the second dynamic import).
The null fallback in bootstrapHydration correctly routes to the production error options.

Verification I ran

vp test run tests/app-router-production-build.test.ts — 9/9 pass
vp check on the modified file — clean
Confirmed parity with the existing Pages Router pattern (pages-client-entry.ts:173), and that no Cloudflare worker entry imports the overlay.

Non-blocking nit

Line 2167 introduces a second dynamic import of the same module. It's harmless (cached, and installDevErrorOverlay is idempotent), just slightly redundant versus threading the already-loaded reference from main().

No correctness, edge-case, or parity issues found that warrant blocking.

github run

james-elicx · 2026-06-23T09:26:02Z

/bigbonk re-review

ask-bonk

Re-review (/bigbonk)

Verdict: Correct, well-scoped, and the prior nit was addressed. Approving.

What the fix does

The App Router browser entry (app-browser-entry.ts) previously had a top-level static import of ./dev-error-overlay.js. Even though the overlay was only used inside import.meta.env.DEV / import.meta.hot guards, a static import keeps the ~1800-line module (and its react-dom/client createRoot usage) alive in the production client bundle — import.meta.hot is merely undefined at runtime in prod and does not enable static DCE.

The fix:

Replaces the static import with a type-only typeof import(...) (erased) plus a single dynamic await import("./dev-error-overlay.js") behind import.meta.env.DEV (line 1397).
Threads the loaded module reference through bootstrapHydration(rscStream, devErrorOverlay) and into the HMR closure (devErrorOverlay?.dismissOverlay() at line 2167).
Tightens the HMR guard at line 2115 from if (import.meta.hot) to if (import.meta.env.DEV && import.meta.hot), which is the key change that lets Rolldown statically eliminate the whole HMR block (including the "rsc:update" literal at line 2197 and the applyRscHmrUpdate body) in prod.
The hydrate-root options branch now keys on import.meta.env.DEV && devErrorOverlay, falling back to the prod onCaughtError/onRecoverableError path.

Prior nit resolved

The previous review flagged a second dynamic import of the overlay inside the HMR handler. That is no longer present — line 2167 uses the threaded devErrorOverlay?.dismissOverlay() reference. There is now exactly one import("./dev-error-overlay.js") site (confirmed via grep).

Verification I ran

vp test run tests/app-router-production-build.test.ts — 9/9 pass (47s)
vp check packages/vinext/src/server/app-browser-entry.ts — clean (format, lint, types)
Confirmed "rsc:update" and dev-error-overlay literals only appear in the dev-gated path / source comments (stripped by minification), so the new test needles are meaningful regression guards.
Confirmed parity with the established Pages Router pattern at pages-client-entry.ts:173-181 — both entries now dynamic-import the overlay behind import.meta.env.DEV.
No Cloudflare worker entry imports the overlay, so nothing else is affected.

No correctness, edge-case, or parity issues found.

ask-bonk · 2026-06-23T09:29:25Z

Re-review posted and approved.

Summary

This PR correctly fixes #1724 by excluding the App Router dev error overlay from the production client bundle.

Verified:

The static top-level import of dev-error-overlay.js is replaced with a single dynamic await import() behind import.meta.env.DEV, plus a type-only typeof import(...) for typing.
The module reference is threaded through bootstrapHydration and into the HMR closure — the prior review's nit about a redundant second dynamic import is now resolved (only one import site exists).
The key change — tightening if (import.meta.hot) → if (import.meta.env.DEV && import.meta.hot) at line 2115 — lets Rolldown statically eliminate the entire HMR block in prod.
New test needles (dev-error-overlay, installDevErrorOverlay, rsc:update, etc.) are meaningful regression guards; those strings only live in the dev-gated path or comments.

Tests run:

tests/app-router-production-build.test.ts — 9/9 pass
vp check on the modified file — clean
Parity confirmed with the Pages Router pattern (pages-client-entry.ts:173)

No correctness, edge-case, or parity issues found. Approved.

github run

fix(app-router): exclude dev overlay from production client bundle

16e6e64

ask-bonk Bot reviewed Jun 23, 2026

View reviewed changes

fix(app-router): reuse loaded dev overlay during HMR

795a8c0

ask-bonk Bot approved these changes Jun 23, 2026

View reviewed changes

james-elicx merged commit 66a7b4e into cloudflare:main Jun 23, 2026
47 checks passed

github-actions Bot mentioned this pull request Jun 23, 2026

chore: version packages #2259

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(app-router): exclude dev overlay from production client bundle#2284

fix(app-router): exclude dev overlay from production client bundle#2284
james-elicx merged 2 commits into
cloudflare:mainfrom
ponharu:fix/app-router-dev-overlay-production-bundle

ponharu commented Jun 23, 2026

Uh oh!

pkg-pr-new Bot commented Jun 23, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jun 23, 2026 •

edited

Loading

Uh oh!

james-elicx commented Jun 23, 2026

Uh oh!

ask-bonk Bot left a comment

Uh oh!

ask-bonk Bot Jun 23, 2026

Uh oh!

ask-bonk Bot commented Jun 23, 2026

Uh oh!

james-elicx commented Jun 23, 2026

Uh oh!

ask-bonk Bot left a comment

Uh oh!

ask-bonk Bot commented Jun 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

ponharu commented Jun 23, 2026

Summary

Verification

Uh oh!

pkg-pr-new Bot commented Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Performance benchmarks

Uh oh!

james-elicx commented Jun 23, 2026

Uh oh!

ask-bonk Bot left a comment

Choose a reason for hiding this comment

Uh oh!

ask-bonk Bot Jun 23, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk Bot commented Jun 23, 2026

Review: PR #2284 — exclude dev overlay from production client bundle

What the bug was

Why the fix is right

Verification I ran

Non-blocking nit

Uh oh!

james-elicx commented Jun 23, 2026

Uh oh!

ask-bonk Bot left a comment

Choose a reason for hiding this comment

Re-review (/bigbonk)

What the fix does

Prior nit resolved

Verification I ran

Uh oh!

ask-bonk Bot commented Jun 23, 2026

Summary

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pkg-pr-new Bot commented Jun 23, 2026 •

edited

Loading

github-actions Bot commented Jun 23, 2026 •

edited

Loading