Default output.globalObject to globalThis for universal targets#21314
Merged
Conversation
A UMD library targeting both web and node (the universal signature, tp.web === null && tp.node === null) only resolved output.globalObject to `globalThis` when output was ESM. Classic non-module UMD builds fell back to `self`, which is undefined in Node — the #6522 symptom. Key off target support for `globalThis` instead, unless a merged version explicitly lacks it.
Replace the issue-6522 case with a minimal multi-compiler test under target/ that builds the universal (web + node) non-ESM library as both `umd` and `global` — the two library types that embed output.globalObject. Drop the unnecessary node/externalsPresets options; the bundle simply running in node proves `self` (undefined there) was not used. Tighten the defaults.js comment.
Any ESM-capable runtime supports `globalThis` (ES2020), so module output should never fall back to `self`. This extends the universal-target fix to plain `web`/`webworker` ESM builds. Update the output-module defaults snapshot.
Set output.environment.globalThis via conditionallyOptimistic(tp, output.module), matching the sibling `module`/`dynamicImport` flags: ESM output only runs where `globalThis` (ES2020) exists. This renders shimmed `global` as the `globalThis` literal and drops the global runtime module for module output. Revert defaulting output.globalObject to `globalThis` for plain web ESM: `self` already works in every web context, and switching the chunk-loading global to `globalThis` broke web+ESM jsonp chunk loading. The universal (web + node) fix is unaffected. Update the output-module defaults snapshots.
Covers the tp.globalThis !== false guard: output.globalObject stays self and environment.globalThis stays false when a merged version (node<12) reports no globalThis, even for the universal target.
🦋 Changeset detectedLatest commit: c8e0314 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 (676f564). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@676f564
yarn add -D webpack@https://pkg.pr.new/webpack@676f564
pnpm add -D webpack@https://pkg.pr.new/webpack@676f564 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes universal (web + node) library builds that previously defaulted output.globalObject to self (breaking in Node) by preferring globalThis when the merged target supports it, and it aligns output.environment.globalThis inference with existing ESM-output environment inference patterns.
Changes:
- Default
output.globalObjectto"globalThis"for universal targets unless the merged target explicitly reportsglobalThis: false. - Infer
output.environment.globalThis: truefor ESM output viaconditionallyOptimistic(tp.globalThis, output.module). - Add regression coverage via a new config case and updated Defaults snapshots (including a node10 universal-merge guard case).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/config/defaults.js | Adjusts output.globalObject defaulting for universal targets and infers environment.globalThis for ESM output. |
| test/Defaults.unittest.js | Updates inline snapshots for ESM output and adds a universal + node10 case ensuring globalThis remains disabled when absent. |
| test/configCases/target/universal-global-object/webpack.config.js | New integration config compiling universal UMD + global library outputs with classic (non-ESM) output. |
| test/configCases/target/universal-global-object/test.config.js | Selects the correct bundle per sub-configuration for the new multi-compiler test case. |
| test/configCases/target/universal-global-object/index.js | Runtime assertion that the bundle loads in Node (i.e., does not reference self). |
| .changeset/universal-umd-global-object.md | Adds a patch changeset describing the default behavior changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address Copilot review: compress the multi-line comments in the new config case, index.js, and the Defaults guard case to single lines, and shorten the changeset to one short sentence, per the repo's comment conventions.
Merging this PR will degrade performance by 37.53%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Memory | benchmark "side-effects-reexport", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' |
126.9 KB | 859.3 KB | -85.23% |
| ❌ | Memory | benchmark "asset-modules-bytes", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' |
326.6 KB | 855.8 KB | -61.84% |
| ❌ | Memory | benchmark "many-modules-esm", scenario '{"name":"mode-development","mode":"development"}' |
1.2 MB | 1.9 MB | -38.69% |
| ❌ | Memory | benchmark "wasm-modules-async", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' |
190 KB | 245.9 KB | -22.73% |
| ❌ | Memory | benchmark "many-chunks-esm", scenario '{"name":"mode-production","mode":"production"}' |
7.4 MB | 9.4 MB | -20.61% |
| ⚡ | Memory | benchmark "wasm-modules-sync", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' |
360.1 KB | 128.4 KB | ×2.8 |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing fix/universal-global-object (c8e0314) with main (1a97d13)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #21314 +/- ##
=======================================
Coverage 92.86% 92.87%
=======================================
Files 594 594
Lines 65210 65210
Branches 18143 18143
=======================================
+ Hits 60559 60562 +3
+ Misses 4651 4648 -3
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 emitted UMD/global bundles reference the globalThis runtime global, which does not exist before Node 12, so the config case crashed with 'globalThis is not defined' on the legacy-node CI matrix. Gate it with supportsGlobalThis().
Address Copilot review: mergeTargetProperties yields tp.globalThis === null for a universal target with mixed globalThis support (e.g. node10 + node12), and the previous !== false guard treated that as supported. Key off === undefined so mixed (null) and unsupported (false) both fall back to self. Add a Defaults case for the mixed target, and tighten the two inline/test comments.
It broke the Node 10 CI matrix: making output.environment.globalThis true for universal module output renders the shimmed `global` as the `globalThis` literal, which is undefined on Node <12, so runtime environment checks (e.g. hot/lazy-compilation-universal.js picking the node vs web backend) misfire and pick the web path. The optimism only applied to the version-agnostic universal target (which spans old Node), so it had no safe benefit. The universal output.globalObject fix is unaffected.
Comment on lines
+1732
to
+1734
| // Universal target (web + node): default to `globalThis` (the only accessor | ||
| // available everywhere) when no merged target reports it; a reported value | ||
| // (`false`, or `null` for mixed support) falls through to `self`. |
Comment on lines
+1
to
+4
| it("should load in node without referencing `self`", () => { | ||
| // `self` (undefined in node) would throw at load; reaching here proves it isn't referenced. | ||
| expect(hello()).toBe("hello"); | ||
| }); |
Contributor
Types CoverageCoverage after merging fix/universal-global-object 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
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
Fixes #6522. A UMD (or
global) library built for both web and node — the universal target (["web", "node"]/"universal") — resolvedoutput.globalObjecttoself, which isundefinedin Node, so the emitted})(self, …)wrapper threwReferenceError: self is not definedwhen the bundle wasrequired there.output.globalObjectnow defaults toglobalThisfor the universal target when no merged target reportsglobalThissupport (tp.globalThis === undefined); a target that reports it as absent (false, ornullfor mixed support such asnode10+node12) still falls back toself.What kind of change does this PR introduce?
fix
Did you add tests for your changes?
Yes.
test/configCases/target/universal-global-object/builds the universal non-ESM library as bothumdandglobal(the two library types that embed the global object) and asserts it loads in Node without aselfReferenceError(gated to Node 12+, where the emittedglobalThisexists, viatest.filter.js).test/Defaults.unittest.jsadds cases for a universal target whose version lacksglobalThis(false) and for mixedglobalThissupport (null), both keepingoutput.globalObject: "self".Does this PR introduce a breaking change?
No. It only changes the default for the universal target, which was previously broken (it emitted
self, which crashes in Node).If relevant, what needs to be documented once your changes are merged or what have you already documented?
n/a
Use of AI
AI (Claude Code) was used to investigate the defaults resolution, map affected target/library combinations, write the tests, and draft this PR. All changes were reviewed and verified against the full
ConfigTestCases,StatsTestCases, andDefaults.unittestsuites.🤖 Generated with Claude Code