Skip to content

fix(build): drop browser export condition for Workers SSR rendering#1849

Draft
james-elicx wants to merge 1 commit into
mainfrom
fix/workers-use-client-html-element
Draft

fix(build): drop browser export condition for Workers SSR rendering#1849
james-elicx wants to merge 1 commit into
mainfrom
fix/workers-use-client-html-element

Conversation

@james-elicx

Copy link
Copy Markdown
Member

Problem

A "use client" component that imports a library which registers a custom web component (extends HTMLElement) at module-init time crashes on Cloudflare Workers with:

ReferenceError: HTMLElement is not defined

Affected library in the report: @number-flow/react (animated numbers). Any library that does class … extends HTMLElement at module load triggers it.

Root cause

The crash hinges on this pattern inside number-flow (via esm-env):

const Base = BROWSER ? HTMLElement : class {};   // BROWSER from esm-env
class NumberFlowElement extends Base {}          // evaluated at import time

esm-env's BROWSER resolves to true only under the browser export condition, and false otherwise. So the crash requires the browser condition to be active during server rendering.

@cloudflare/vite-plugin sets conditions: ["workerd", "worker", "module", "browser", …] for every worker environment — including the rsc/ssr environments vinext renders "use client" components through. Because browser precedes development/production in esm-env's export map, BROWSER resolves to true, and the SSR client-reference chunk bakes const i = HTMLElement, a = class extends i{}, which throws at module-init in workerd (no DOM globals).

Plain Node SSR already resolves with server conditions (no browser), which is exactly why next build && next start works — and why this only reproduces on Workers, not on vinext build && vinext start (verified: the Node SSR bundle resolves BROWSER=false and renders fine).

Fix

In configResolved (after the Cloudflare plugin has merged its conditions), strip the browser export condition from the rsc and ssr environments' resolve.conditions and dep-optimizer conditions. SSR/RSC never run in a real browser, so isomorphic libraries now resolve their server-safe entry — matching Node SSR. The client environment is left untouched, so the browser bundle is unaffected. It is a no-op on plain Node (those environments never carry browser).

  • plugins/rsc-client-shim-excludes.ts: add withoutBrowserCondition() + stripBrowserConditionFromServerEnv() (pure, typed helpers).
  • index.ts: call the helper for the rsc/ssr environments.

Testing

tests/use-client-html-element-build.test.ts:

  • Unit tests for withoutBrowserCondition / stripBrowserConditionFromServerEnv.
  • A Cloudflare build regression test using local esm-env/number-flow/@number-flow/react look-alikes that share the exact resolution shape. It asserts the worker server bundle resolves the server-safe entry (BROWSER=false) while the client bundle keeps the browser entry. Fails before this change, passes after.

No regressions in the affected build/SSR suites: tsconfig-path-alias-build, prerender, build-optimization, features, app-router-production-build, app-router-production-server, app-router-worker-entry, app-router-client-preloading, app-router-rsc-plugin. vp check (format/lint/types) passes.

Notes / scope

  • Scoped to App Router (rsc/ssr), which is the reported and reproduced case. A pure Pages-Router-on-Workers app importing such a library could in theory hit the same condition — that's a separate environment-setup path and would warrant its own reproduction + follow-up.
  • The report also mentions vinext build && vinext start (Node) crashing; that part does not reproduce with current vinext (Node SSR resolves BROWSER=false). The Workers crash is the real, verifiable bug this PR fixes.

A "use client" component that imports a library which extends HTMLElement at module-init time (e.g. @number-flow/react via number-flow + esm-env) crashed Cloudflare Workers with "ReferenceError: HTMLElement is not defined".

@cloudflare/vite-plugin injects `browser` into the rsc/ssr worker environments' resolve conditions, so esm-env's BROWSER flag resolves to true and the bundle evaluates `class extends HTMLElement` during SSR, where no DOM globals exist. Plain Node SSR resolves with server conditions (no `browser`), which is why `next build && next start` works.

Strip `browser` from the rsc/ssr environments in configResolved so they resolve the server-safe entry, matching Node SSR. The client environment is left untouched.

Adds unit tests for the helper plus a Cloudflare build regression test that asserts the server bundle resolves the non-browser entry.
@pkg-pr-new

pkg-pr-new Bot commented Jun 8, 2026

Copy link
Copy Markdown

Open in StackBlitz

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

commit: 8043086

@github-actions

github-actions Bot commented Jun 8, 2026

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

@james-elicx

Copy link
Copy Markdown
Member Author

/bigbonk review

@ask-bonk

ask-bonk Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

@james-elicx Bonk workflow was cancelled.

View workflow run · To retry, trigger Bonk again.

@james-elicx

Copy link
Copy Markdown
Member Author

/bigbonk review for issues

@ask-bonk

ask-bonk Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Review — PR #1849: drop browser export condition for Workers SSR

Reviewed the root-cause analysis, the helper implementation, the index.ts wiring, and the test. Verified the behavior against the real @cloudflare/vite-plugin@1.31.0 and the real esm-env export map. The fix is correct, well-targeted, and well-tested. Details below, plus a few minor notes.

Verification

  • CF plugin really injects browser. Confirmed in @cloudflare/vite-plugin@1.31.0: defaultConditions = ["workerd", "worker", "module", "browser"], applied to both resolve.conditions ([...defaultConditions, "development|production"]) and the dep optimizer (conditionNames: [...defaultConditions, "development"], and esbuildOptions.conditions on the esbuild path). The helper strips all three, so dev (wrangler dev) and prod build stay in sync. Good.
  • Environment targeting is exactly right. The CF plugin here is configured viteEnvironment: { name: "rsc", childEnvironments: ["ssr"] }, so rsc/ssr are the only worker environments carrying browser. The PR strips both and leaves client untouched — correct. No other server environment exists to miss.
  • Real esm-env matches the fixture's resolution shape. esm-env@1.2.2 ./browser export: { browser: ./true.js, development: ./false.js, production: ./false.js, default: ./browser-fallback.js }. With browser removed, the CF-injected development/production conditions match → resolves the false (server-safe) entry. Confirmed the fix produces BROWSER=false server-side.
  • No-op on plain Node confirmed. vinext does not add browser to its own rsc/ssr environment config anywhere; that condition only comes from the CF plugin. So the helper is a genuine no-op on Node builds, as claimed.
  • Ran vp test run tests/use-client-html-element-build.test.ts → 6/6 pass (including the integration build assertion). vp check on all three changed files → clean (format/lint/types).

Minor notes (non-blocking)

  1. Test fixture ./browser default differs from real esm-env. The fixture uses default: "./false.js", while real esm-env uses default: "./browser-fallback.js" (export default typeof window !== 'undefined'). The outcome is identical here because the CF-injected development/production conditions match before default is reached, so the test still exercises the right path. But if you want the fixture to be a faithful look-alike (and to catch a future regression where neither development nor production is present and resolution falls through to default), consider mirroring the globalThis.window fallback. Optional.

  2. hasAppDir guard vs. the helper's own no-op guard. The strip is gated behind if (hasAppDir), but the helper already no-ops on undefined envs. For a Pages-Router-only Workers app, rsc/ssr don't exist, so calling the helper unconditionally would also be safe. The current guard is harmless, just slightly redundant with the helper's internal guard. Not worth changing.

  3. Pages Router on Workers remains exposed (acknowledged in the PR). A pure Pages-Router-on-Workers app importing such a library could hit the same browser-condition crash via the CF parent/entry environment, which this PR doesn't touch. The PR explicitly scopes this out and flags it as a follow-up — reasonable per the "verify Next.js parity, document deliberate scope" guidance. Worth filing a tracking issue so it isn't lost.

Parity / behavior

No hydration-mismatch concern beyond stock Next.js behavior: the server renders the no-DOM branch and the client hydrates the DOM/web-component branch, which is the intended SSR pattern and matches next build && next start. The defensive choice to strip browser from server environments (rather than relying on libraries to gate correctly) is sound and well-documented in the code comments.

LGTM.

@ask-bonk

ask-bonk Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Posted the review to PR #1849. Summary of findings:

The fix is correct, well-targeted, and well-tested. I verified it against the actual @cloudflare/vite-plugin@1.31.0 source and the real esm-env export map:

  • The CF plugin really does inject browser into resolve.conditions, the rolldown conditionNames, and the esbuild conditions for worker environments — the helper strips all three, keeping dev and prod in sync.
  • rsc/ssr are exactly the worker environments configured (viteEnvironment: { name: "rsc", childEnvironments: ["ssr"] }); the PR targets both and correctly leaves client alone.
  • Real esm-env@1.2.2's ./browser export resolves to the server-safe false entry once browser is removed (because CF's development/production conditions match), confirming the root-cause analysis.
  • Genuine no-op on plain Node — vinext never adds browser to its own server env config.
  • Tests pass (6/6) and vp check is clean.

Minor non-blocking notes I flagged:

  1. The test fixture's ./browser default is false.js rather than mirroring real esm-env's browser-fallback.js (typeof window !== 'undefined') — same outcome here, but a slightly less faithful look-alike.
  2. The hasAppDir guard is slightly redundant with the helper's own undefined no-op guard (harmless).
  3. Pages-Router-on-Workers remains exposed to the same crash — acknowledged and scoped out by the author; worth a tracking issue.

Overall: LGTM.

github run

@james-elicx

Copy link
Copy Markdown
Member Author

/bigbonk adversarial review for edge cases / side effects / unexpected bugs

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