Respect the node: prefix for node.js core modules used as externals#21286
Merged
Conversation
… targets
NodeTargetPlugin passed user-authored externals through verbatim, so a
`require("node:fs")` import stayed `require("node:fs")` even when the target
lacks `node:` support (output.environment.nodePrefixForCoreModules === false),
breaking on older Node. Strip the prefix from core modules in that case.
Developer-provided externals were matched by exact request string, so an
external keyed `node:fs` never matched a bare `require("fs")` (and vice
versa), forcing developers to list both forms or hit unresolved-request
errors. Externals for node.js core modules now match either prefix form.
Extract the builtins list into a shared lib/node/nodeBuiltins module so the
externals factory, node, deno and bun targets reuse one source of truth.
…ed targets
Move the `node:` prefix stripping to ExternalModule code generation so it
applies uniformly to both automatic (NodeTargetPlugin) and developer-provided
externals. A value like `externals: { "node:fs": "commonjs node:fs" }` now
emits `require("fs")` when the target lacks `node:` support, instead of the
unresolvable `require("node:fs")`. NodeTargetPlugin reverts to the plain
builtins list since the stripping no longer happens at match time.
A universal bundle (`["node", "web"]` + `output.module`) may run on deno or
bun, which can only resolve a node.js core module through the `node:`
specifier. Emit the prefix for such bundles when the target supports it, so a
bare `require("fs")` becomes `require("node:fs")`. Single-runtime targets keep
the author's original notation as before.
🦋 Changeset detectedLatest commit: 07dbf40 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 (54fa902). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@54fa902
yarn add -D webpack@https://pkg.pr.new/webpack@54fa902
pnpm add -D webpack@https://pkg.pr.new/webpack@54fa902 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #21286 +/- ##
=======================================
Coverage 92.87% 92.87%
=======================================
Files 592 594 +2
Lines 65025 64980 -45
Branches 18166 18155 -11
=======================================
- Hits 60390 60353 -37
+ Misses 4635 4627 -8
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:
|
The universal external resolves core modules via process.getBuiltinModule / createRequire at runtime, which is unavailable on older node (the test failed on the node 10-18 integration runners). Filter it like node-commonjs-universal so it only runs where that API exists.
Merging this PR will not alter performance
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Memory | benchmark "many-chunks-commonjs", scenario '{"name":"mode-production","mode":"production"}' |
6.9 MB | 9 MB | -23.97% |
| ❌ | Memory | benchmark "devtool-eval-source-map", scenario '{"name":"mode-production","mode":"production"}' |
6.1 MB | 7.8 MB | -21.85% |
| ⚡ | Memory | benchmark "wasm-modules-async", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' |
332.3 KB | 191.2 KB | +73.75% |
| ⚡ | Memory | benchmark "many-modules-commonjs", scenario '{"name":"mode-production","mode":"production"}' |
8.8 MB | 7.3 MB | +20.42% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing claude/node-prefix-externals-08wj1l (07dbf40) with main (5e7db5d)
Contributor
Types CoverageCoverage after merging claude/node-prefix-externals-08wj1l into main will be
Coverage Report |
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
Externals ignored the
node:prefix capability for node.js core modules. This PR makes them respect it in three ways: strip the prefix when the target can't resolve it (output.environment.nodePrefixForCoreModules === false), match the prefixed and bare forms interchangeably so a developer-provided external keyednode:fsalso matches a barerequire("fs")(and vice versa), and add the prefix for a universal (["node", "web"]+output.module) bundle, which may run on Deno/Bun where the prefix is required. Supported single-runtime targets keep the author's original notation unchanged.What kind of change does this PR introduce?
fix
Did you add tests for your changes?
Yes —
test/configCases/externals/node-prefix(capability stripping for automatic externals),node-prefix-matching(prefixed/bare matching),node-prefix-user-externals(stripping developer-provided external values), andnode-prefix-universal(prefixing for universal bundles).Does this PR introduce a breaking change?
No. On supported single-runtime targets the author's notation is preserved verbatim; output only changes toward correctness (stripping on unsupported targets, prefixing for universal bundles).
If relevant, what needs to be documented once your changes are merged or what have you already documented?
n/a — the behavior follows the existing
output.environment.nodePrefixForCoreModulescapability.Use of AI
AI (Claude) was used to investigate the codebase, implement the changes, and write the test cases; all output was reviewed and verified against the test suite by the author.
Generated by Claude Code