fix(ssr): convert bundled CJS require() of node builtins for webworker SSR (fix #22618)#22619
fix(ssr): convert bundled CJS require() of node builtins for webworker SSR (fix #22618)#22619basuke wants to merge 1 commit into
Conversation
…r SSR (fix vitejs#22618) vitejs#21963 added `esmExternalRequirePlugin()` for `ssr.target: 'webworker'` builds, but invoked it with no `external`, so the plugin matched nothing. A bundled CommonJS dependency that internally calls `require("<node builtin>")` (e.g. node-forge requiring "crypto") therefore fell through to Rolldown's throwing `__require` stub — breaking on workerd (and failing the build when the SSR output is evaluated, as in SvelteKit's adapter-cloudflare). Pass `nodeLikeBuiltins` as the plugin's `external` so those nested builtin requires are converted to ESM imports the runtime (e.g. workerd's nodejs_compat) resolves, matching how Vite 7 / Rollup handled them. Adds a regression fixture (a bundled CJS module requiring `node:util`) to the ssr-webworker playground, with runtime and build-output assertions.
1266678 to
2e7844f
Compare
|
The failing check (Build&Test node-24, macOS only) is an unrelated flaky test — |
| // (e.g. workerd's nodejs_compat) can resolve. | ||
| if (isSsrTargetWebworkerEnvironment) { | ||
| plugins.push(esmExternalRequirePlugin()) | ||
| plugins.push(esmExternalRequirePlugin({ external: nodeLikeBuiltins })) |
There was a problem hiding this comment.
It seems adding this removes the [plugin rolldown:vite-resolve] Automatically externalized node built-in module "node:util" imported from "src\cjs-node-builtin.cjs". Consider adding it to environments.ssr.external if it is intended. warning. I think we need to keep that.
Description
Follow-up to #21963 (fix #21969), which added
esmExternalRequirePlugin()forssr.target: 'webworker'builds so external CJSrequire()becomes ESM imports (avoidingcreateRequire(import.meta.url), which throws on workerd).The plugin was invoked with no
externaloption, so it matched nothing. A bundled CommonJS dependency that internally callsrequire("<node builtin>")— e.g.node-forgedoingrequire("crypto")— therefore fell through to Rolldown's throwing__requirestub:This breaks on Cloudflare Workers (workerd), and fails the build outright when the SSR output is evaluated during analysis (e.g. SvelteKit
adapter-cloudflare). Vite 7 / Rollup converted the same nested require to a staticimport nodeCrypto from "crypto", so this is a regression for the webworker target.Fixes #22618.
Change
Pass
nodeLikeBuiltins(the existing helper inutils.ts) as the plugin'sexternal, so nested builtinrequire()in bundled CJS is converted to ESM imports that the runtime (workerd'snodejs_compat) resolves:Alternatives explored
ssr.target: 'webworker'alone — insufficient (this is exactly the gap).ssr.noExternal: true— builds, butcryptoresolves to a browser stub missingrandomBytesat runtime.ssr.external) — the nestedrequire()inside the bundled CJS wrapper still hits the throwing stub.Tests
Extends the
ssr-webworkerplayground with a bundled CJS fixture thatrequire("node:util"), plus a runtime assertion (executes under miniflare) and a build-output assertion (require("node:util")must become an import, not a__requirestub). Verified these fail onmain(var util = __require("node:util")+ throwing stub) and pass with the change. Fullssr-webworkersuite green in both serve and build modes.