feat: improve external loading for universal (node + web) targets#21187
Merged
alexander-akait merged 6 commits intoJun 15, 2026
Conversation
In a universal target (e.g. `["node", "web"]` with `output.module`), a
`node-commonjs` external emitted a hoisted `import { createRequire } from
"module"`, which crashes the whole ESM bundle when loaded in a browser even
when the `require()` sits behind a runtime guard.
Obtain `createRequire` via `process.getBuiltinModule("module")` instead:
inline-guarded when the node version is known to support it
(`environment.nodeBuiltinModuleGetter`), otherwise wrapped in try/catch so the
bundle still loads where `process` is absent. Pure-node builds are unchanged.
Verifies the other safe universal external path: a dynamic `import()` of a node built-in stays lazy (no hoisted static import that would crash a browser at load) and resolves correctly when run in node.
… buildInfo Read `externalsPresets` from `runtimeTemplate.compilation` at code generation instead of stashing a `universalExternal` flag on every external module's `buildInfo`. `runtimeTemplate` is threaded into every `codeGeneration` call (including concatenated inner modules, where the context `compilation` is absent), so the universal target is detected directly without per-module state. Add a regression test covering a node-commonjs external that gets concatenated.
Adds a matrix config case asserting the external types usable in a universal ESM build run correctly: node-commonjs (require), var (expression), module (static import) and import (dynamic).
…universal Expands the matrix case to every external type usable in a universal ESM build (node-commonjs, var, assign, module, promise, import) and adds a negative case asserting require-based externals (commonjs family, this) throw there.
🦋 Changeset detectedLatest commit: cf27ea1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
|
This PR is packaged and the instant preview is available (877f3dc). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@877f3dc
yarn add -D webpack@https://pkg.pr.new/webpack@877f3dc
pnpm add -D webpack@https://pkg.pr.new/webpack@877f3dc |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #21187 +/- ##
==========================================
- Coverage 92.70% 92.69% -0.01%
==========================================
Files 588 588
Lines 64079 64091 +12
Branches 17780 17785 +5
==========================================
+ Hits 59407 59412 +5
- Misses 4672 4679 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
… ESM Two more external types now work in a universal ESM target: - commonjs/commonjs2/commonjs-module/commonjs-static load via `createRequire` in ESM output (with the defensive universal loader), gated on node being available so pure-web bundles keep the plain `require` form. - `output.globalObject` resolves to `globalThis` for a version-less universal module target, so `global` externals work in both node and the browser. The defensive node-commonjs loader obtains `createRequire` from `process.getBuiltinModule()`, guarded with `typeof process` for the browser and `RuntimeTemplate.optionalChaining` (optional chaining or an `&&` fallback) for old node without `getBuiltinModule`.
c817c89 to
cf27ea1
Compare
Merging this PR will improve performance by 85.08%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | Memory | benchmark "asset-modules-inline", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' |
1,294.9 KB | 325.1 KB | ×4 |
| ⚡ | Memory | benchmark "many-modules-commonjs", scenario '{"name":"mode-production","mode":"production"}' |
9.2 MB | 7.2 MB | +27.4% |
| ⚡ | Memory | benchmark "wasm-modules-sync", scenario '{"name":"mode-production","mode":"production"}' |
8 MB | 6.4 MB | +24.93% |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing claude/commonjs-universal-target-loading-9i96be (cf27ea1) with main (793aae6)
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
In a universal (
target: ["node", "web"]) ESM build, anode-commonjsexternal emitted a hoistedimport { createRequire } from "module", which crashes the whole bundle when it loads in a browser — even when therequire()sits behind a runtime guard likeconst m = isNode ? require("fs") : require("memory-fs"). This obtainscreateRequirelazily fromprocess.getBuiltinModule()instead, guarded withtypeof process(browser) andRuntimeTemplate.optionalChaining(old node withoutgetBuiltinModule), so nothing is hoisted that breaks the browser. The same defensive loader now also covers thecommonjs/commonjs2/commonjs-module/commonjs-staticfamily in ESM output (gated on node being available, so pure-web bundles keep the plainrequireform), andoutput.globalObjectresolves toglobalThisfor a version-less universalmoduletarget soglobalexternals work in both node and the browser.What kind of change does this PR introduce?
feat
Did you add tests for your changes?
Yes —
test/configCases/externals/node-commonjs-universal(loader strategies),node-commonjs-universal-concatenated(concatenation path),universal-dynamic-import,universal-external-types(every supported external type) anduniversal-unsupported-externals(the unsupported ones), plus thetest/helpers/supportsProcessGetBuiltinModule.jsfilter helper.Does this PR introduce a breaking change?
No.
If relevant, what needs to be documented once your changes are merged or what have you already documented?
Document that
commonjs/node-commonjsexternals are supported in ESM output (loaded viacreateRequire) and that a universalmoduletarget usesglobalThisas the global object.Use of AI
Yes — implemented with Claude (Claude Code): design, code and tests were generated with AI and reviewed/validated by the author per the webpack AI policy.
https://claude.ai/code/session_0194U8nMBSXzmUv9ex86frCQ
Generated by Claude Code