fix: spec-compliant defer namespace [[Set]] and identity#20913
Conversation
`ns.foo = value` previously generated `<importVar>.a.foo = value` whose `.a` getter eagerly required the deferred module, violating the TC39 import-defer `[[Set]]` algorithm. Tap `assignMemberChain` for `harmonySpecifierTag` to walk only the bare `ns` identifier so it resolves to the deferred-namespace proxy (whose `set` trap returns false without triggering evaluation), and leave the `.foo = value` assignment as plain code. Cache the deferred-namespace proxy per-`moduleId` in a new global `__webpack_module_deferred_namespace_cache__`, and stop the `__webpack_require__.z` cached-module fast path from short-circuiting to the underlying exports for namespace-mode imports. This keeps the deferred and eager namespaces as distinct objects (per spec) and shares one Deferred Module Namespace Exotic Object across all defer-import call sites for the same module.
|
🦋 Changeset detectedLatest commit: 80cf471 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 |
|
This PR is packaged and the instant preview is available (2fd88dd). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@2fd88dd
yarn add -D webpack@https://pkg.pr.new/webpack@2fd88dd
pnpm add -D webpack@https://pkg.pr.new/webpack@2fd88dd |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #20913 +/- ##
=======================================
Coverage 91.37% 91.37%
=======================================
Files 563 563
Lines 55822 55868 +46
Branches 14788 14814 +26
=======================================
+ Hits 51006 51049 +43
- Misses 4816 4819 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR improves webpack’s import defer * as ns behavior to better align with the TC39 import-defer proposal, particularly around (1) not triggering evaluation on ns.x = value, and (2) deferred-namespace identity being stable and distinct from eager namespaces.
Changes:
- Adjust parsing of simple
ns.foo = valueassignments so webpack doesn’t rewrite them via the eager-evaluating.agetter path. - Add a new runtime-level deferred-namespace cache to share a single Deferred Module Namespace proxy per module across defer-import call sites.
- Update tests/known-skips to reflect the improved spec compliance (with production-concatenation still listed as problematic for identity).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
test/test262.spectest.js |
Unskips defer [[Set]] / identity tests where fixed; keeps identity skipped for production concatenation. |
test/configCases/defer-import/same-module/index.js |
Updates expectation: eager and deferred namespaces for same module must not be the same object. |
lib/runtime/MakeDeferredNamespaceObjectRuntime.js |
Caches deferred namespace proxy per module and avoids returning underlying exports for mode & 8. |
lib/javascript/JavascriptModulesPlugin.js |
Emits new __webpack_module_deferred_namespace_cache__ runtime global when needed. |
lib/dependencies/HarmonyImportDependencyParserPlugin.js |
Special-cases ns.foo = value for defer imports to avoid eager evaluation via .a. |
.changeset/defer-import-spec-compliant-set-and-identity.md |
Adds a patch changeset describing the behavior change. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "var cached = __webpack_module_deferred_namespace_cache__[moduleId];", | ||
| "if (cached !== undefined) return cached;", | ||
| "var cachedModule = __webpack_module_cache__[moduleId];", | ||
| "if (cachedModule && cachedModule.error === undefined) {", | ||
| "if (cachedModule && cachedModule.error === undefined && !(mode & 8)) {", | ||
| Template.indent([ |
Address Copilot's review on #20913: key the deferred-namespace cache by `(moduleId, shapeMode)` instead of just `moduleId`. `shapeMode` is `mode & ~16` so the irrelevant `createFakeNamespaceObject` async-wrap bit is normalized out — static defer (mode 8) and dynamic `await import.defer` (mode 8 | 16) for the same module still share the same Deferred Module Namespace object — but distinct exports-type shapes (e.g. one importer treats a CJS module as "default-with-named", another as "namespace") get distinct cache entries instead of returning the wrong shape. Add config tests: - `identity-across-call-sites`: same module imported defer multiple times, plus static defer + dynamic `import.defer`, must yield the same object; defer must stay distinct from eager. - `set-does-not-trigger-evaluation`: assigning to a deferred namespace property (exported or not) throws TypeError in strict mode without triggering module evaluation; subsequent property read still evaluates.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "var shapeMode = mode & ~16;", | ||
| "var byMode = __webpack_module_deferred_namespace_cache__[moduleId];", | ||
| "if (byMode && byMode[shapeMode] !== undefined) return byMode[shapeMode];", | ||
| "if (!byMode) byMode = __webpack_module_deferred_namespace_cache__[moduleId] = {};", |
| : "", | ||
| "if (mode & 8) return exports;", | ||
| `return ${RuntimeGlobals.createFakeNamespaceObject}(exports, mode);` | ||
| `return byMode[shapeMode] = ${RuntimeGlobals.createFakeNamespaceObject}(exports, mode);` |
Address Copilot follow-up on #20913: the cache key was masking off bit 16 (`shapeMode = mode & ~16`) but the original `mode` was still being passed to `createFakeNamespaceObject`. Whichever call site ran first would decide whether the cached entry had `createFakeNamespace Object`'s "return value when it's Promise-like" short-circuit applied, so static `import defer` and dynamic `await import.defer` could observe different wrapping semantics for the same cache slot. Strip bit 16 once at the top of the runtime helper so the cache key, the gate that decides between the cached-module fast path and the lazy Proxy, and the value passed to `createFakeNamespaceObject` all use the same `mode`. The bit is irrelevant in this runtime helper anyway — the value passed into `createFakeNamespaceObject` here is always the resolved module exports (after unwrapping the async-module export symbol when present), never a Promise.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address Copilot follow-up on #20913: now that the deferred-namespace Proxy is cached globally, mutations to its handler at init time persist across all defer-import call sites for the lifetime of the bundle. The previous post-init optimization deleted *all* handler traps, including `set`, `deleteProperty`, and `defineProperty`, which caused `ns.notExported = "x"` to silently mutate the proxy target after the first read had triggered evaluation — `[[Set]]` on a Module Namespace Exotic Object must always return false per the TC39 import-defer spec. Keep the three mutation traps and only drop the read-side traps (`get`, `has`, `ownKeys`, `getOwnPropertyDescriptor`); with the resolved namespace's own keys mirrored onto `ns_target`, the default `Reflect` behavior already returns the right values via the live-binding getters. Add a `set-does-not-trigger-evaluation` regression test asserting that strict-mode assignment to both an exported and a non-exported key still throws TypeError after the deferred module has been evaluated, and that no spurious property is created on the target.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
`set-does-not-trigger-evaluation` reads `globalThis.evaluations` which is unavailable on Node.js 10, and `identity-across-call-sites` uses `await import.defer(...)` whose nested async/await pattern hits the Node.js 10 bug `supportsAsync` already gates against.
Types CoverageCoverage after merging claude/fix-defer-tests-ZGm5c into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ns.foo = valuepreviously generated<importVar>.a.foo = valuewhose
.agetter eagerly required the deferred module, violatingthe TC39 import-defer
[[Set]]algorithm. TapassignMemberChainfor
harmonySpecifierTagto walk only the barensidentifier soit resolves to the deferred-namespace proxy (whose
settrapreturns false without triggering evaluation), and leave the
.foo = valueassignment as plain code.Cache the deferred-namespace proxy per-
moduleIdin a new global__webpack_module_deferred_namespace_cache__, and stop the__webpack_require__.zcached-module fast path from short-circuitingto the underlying exports for namespace-mode imports. This keeps
the deferred and eager namespaces as distinct objects (per spec)
and shares one Deferred Module Namespace Exotic Object across all
defer-import call sites for the same module.