Repository navigation
fix(externals): keep a core module external as the code spells it when node: is supported - #22593
Conversation
…n `node:` is supported #22565 also prefixed a bare core module external with `node:` whenever `output.environment.nodePrefixForCoreModules` was on. An unversioned `node` target turns that option on, so bundles run on Node 12 required `node:worker_threads` and failed. A bare external now stays bare again; only a universal bundle gains the prefix. Claude-Session: https://claude.ai/code/session_01ViD41i2KdwwGxrDk7yrjeD
… ordering declarations Ported from #22586: main throws minifying an at-rule block of several declarations, which fails this branch's lint, unit and CSS checks. Claude-Session: https://claude.ai/code/session_01ViD41i2KdwwGxrDk7yrjeD
|
| Name | Type |
|---|---|
| webpack | Patch |
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
|
This PR is packaged and the instant preview is available (0027179). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@0027179
yarn add -D webpack@https://pkg.pr.new/webpack@0027179
pnpm add -D webpack@https://pkg.pr.new/webpack@0027179 |
Types CoverageCoverage after merging fix/node-prefix-keep-external-spelling into main will be
Coverage Report |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ⬆️ High Change: Bug fix Merge Risk: ⚪ Minimal · up to Core-module externals retain or gain prefixes according to target support, avoiding unsupported prefixed requests on older Node runtimes. No concrete merge-blocking behavior is established; proceed with normal CI validation. Pre-merge checks |
  |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Title check |  |
The title uses the required Conventional Commit format, uses the allowed type fix, and accurately describes the main external-module change. The branch prefix is not provided, so the required type-t… |
Provide the branch name or confirm that its prefix is fix. |
✅ Passed checks (3 passed)
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Title check
-
Explanation
The title uses the required Conventional Commit format, uses the allowed type
fix, and accurately describes the main external-module change. The branch prefix is not provided, so the required type-to-branch match cannot be verified.
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #22593 +/- ##
==========================================
+ Coverage 95.54% 96.20% +0.65%
==========================================
Files 774 774
Lines 128230 128462 +232
Branches 42484 42560 +76
==========================================
+ Hits 122514 123582 +1068
+ Misses 5716 4880 -836
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:
|
Generated code sizeComparing base ( Warning The baseline is
Read 594 asset(s) changed size, test untouched, biggest 20 by raw or gzip change
13 asset(s) changed size, test edited
6 asset(s) this pull request adds
1 asset(s) this pull request no longer emits
No runtime that both runs build changed which runtime modules it carries. 1 runtime(s) this pull request adds or no longer builds
Built |
Summary
#22565 also added
node:to a bare core module external wheneveroutput.environment.nodePrefixForCoreModulesis on. An unversionedtarget: "node"turns that option on, so the bundle requiresnode:worker_threads, which Node 12 can't load. The Node 12 integration job now fails withCannot find module 'node:…'(56 tests) on every PR.This keeps the first half of #22565 (both spellings share one module id) and undoes the second: a bare external stays bare again, and only a universal bundle gains the prefix.
The second commit ports #22586's one-line CSS fix, so
lint,unitand the CSS checks can pass here; it no-ops once #22586 lands.What kind of change does this PR introduce?
fix
Did you add tests for your changes?
Yes:
test/configCases/externals/node-prefix-user-externalsnow asserts a bare external stays bare with the option on (fails onmain). Snapshots and the two node cases #22565 updated are restored.Does this PR introduce a breaking change?
No. It restores the released behavior; the unreleased changeset from #22565 is trimmed to match.
If relevant, what needs to be documented once your changes are merged or what have you already documented?
n/a
Use of AI
Written with Claude Code: it traced the Node 12 failures to #22565, made the change and ran the affected config cases; I reviewed it.
🤖 Generated with Claude Code
https://claude.ai/code/session_01ViD41i2KdwwGxrDk7yrjeD
Generated by Claude Code
Summary by CodeRabbit
node:prefix just because that option is enabled. Universal targets still add the prefix to unprefixed requests; other targets preserve the notation used in the request.node:prefixes are still removed.