Build: Preserve __esModule on window.wp.* IIFE globals for webpack interop#78699
Build: Preserve __esModule on window.wp.* IIFE globals for webpack interop#78699kraftbj wants to merge 6 commits into
__esModule on window.wp.* IIFE globals for webpack interop#78699Conversation
…terop
The esbuild build pipeline appends a footer that does `Object.assign({},
ns)` to materialize each WP package's IIFE namespace as data properties.
`Object.assign` only copies enumerable own properties, so esbuild's
non-enumerable `__esModule: true` marker was being stripped.
Without the marker, webpack's `__webpack_require__.n` helper in external
consumers (e.g. WooCommerce) takes the CJS branch and returns the whole
namespace instead of `.default`, breaking default imports from
`wordpress/*` packages and crashing the Cart block on Gutenberg trunk.
The fix branches on `wpScriptDefaultExport`:
- Non-default-export packages: re-seed the `Object.assign` target with a
non-enumerable, writable `__esModule` when the source has it. Writable
so a future esbuild emit that makes the source's `__esModule`
enumerable would not trip a strict-mode TypeError.
- `wpScriptDefaultExport` packages: keep the plain `Object.assign({},
ns)` since the global has been unwrapped to the default value and no
longer represents an ES module namespace; any stray `__esModule` flag
on the unwrapped default is correctly dropped so webpack's interop
takes the CJS branch and returns the value directly.
Fixes WordPress#78697
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
prettyboymp
left a comment
prettyboymp
left a comment
There was a problem hiding this comment.
I tested against WooCommerce core and this fixed the issue with the conversion of is-shallow-equal that was introduced.
|
Warning: Type of PR label mismatch To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.
Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task. |
|
Let's loop in @youknowriad for feedback, since this is related to #72140 as mentioned 🙂 |
|
Thanks for the fix, trying to understand a bit better. esbuild switch happened on the previous WP release, is this a new issue? Why is it only surfacing now on Woo? |
|
The cause of this bug is a very recent optimization by @ellatrix in #78303. One of the optimizations we did during our RSM projects 🙂 The |
| // copy is the simplest way to materialize each value once. | ||
| footerParts.push( | ||
| `if(${ globalName }&&typeof ${ globalName }==='object'){${ globalName }=Object.assign({},${ globalName });}` | ||
| ); |
There was a problem hiding this comment.
I think that all we need to do is that instead of Object.assign({}, ...) we do Object.assign({ __esModule: true }, ...). That covers everything, the patch doesn't need to be that complicated.
There was a problem hiding this comment.
Good call, that's much cleaner — pre-seeding the target also makes the writable concern moot in the common case. One tweak: a plain { __esModule: true } literal defines the property as enumerable, but esbuild's __toCommonJS (and webpack's own __webpack_require__.r) mark it non-enumerable, so the literal would leak __esModule into Object.keys(), spread, and downstream Object.assign. Keeping it non-enumerable is still a one-liner:
`if(${ globalName }&&typeof ${ globalName }==='object'){${ globalName }=Object.assign(Object.defineProperty({},'__esModule',{value:true,writable:true}),${ globalName });}`That drops both the conditional and the two-branch split. I kept writable: true as a cheap guard so a future esbuild change emitting an enumerable __esModule wouldn't trip a strict-mode write when Object.assign copies it — happy to drop it if you'd rather keep it minimal. Pushed in be32a3d.
There was a problem hiding this comment.
Yes, it's great that Object.defineProperty returns the object 🙂
Per review feedback, collapse the branched footer into one line that pre-seeds the Object.assign target with a non-enumerable, writable `__esModule`. This drops the conditional and the wpScriptDefaultExport split while keeping the descriptor faithful to esbuild's __toCommonJS output (non-enumerable, so it does not leak into Object.keys, spread, or downstream Object.assign).
| // copy is the simplest way to materialize each value once. | ||
| footerParts.push( | ||
| `if(${ globalName }&&typeof ${ globalName }==='object'){${ globalName }=Object.assign({},${ globalName });}` | ||
| ); |
There was a problem hiding this comment.
Yes, it's great that Object.defineProperty returns the object 🙂
| // `__webpack_require__.r`) emit, so it does not leak into | ||
| // `Object.keys`, spread, or downstream `Object.assign`. `writable` | ||
| // guards against a strict-mode write if a future esbuild ever | ||
| // emitted an enumerable `__esModule` for `Object.assign` to copy. |
There was a problem hiding this comment.
This comment is too long and includes too many internal details about bundlers. I would write it as:
Seed the copy with a non-enumerable
__esModuleflag so that bundlers treat it as an ESM module, triggering interop conventions for default exports.
There was a problem hiding this comment.
Done in cbca833 — used your wording, kept the existing line about materializing the getters.
|
Superceded by #78917 |
6 participants
What?
Closes #78697
The esbuild build pipeline introduced in #72140 appends a footer to every
window.wp.*IIFE bundle that materializes esbuild's getter-based exports as data properties viaObject.assign( {}, ns ). The shallow copy drops esbuild's non-enumerable__esModule: truemarker, which broke webpack default-import interop for external consumers (e.g. WooCommerce — Cart/Checkout blocks crash against Gutenberg trunk becauseisShallowEqualresolves to the whole namespace object instead of the default function).Why?
Object.assign( {}, source )only copies enumerable own properties. esbuild's__toCommonJShelper declares__esModuleviaObject.defineProperty( ns, '__esModule', { value: true } ), which is non-enumerable by default, so the marker was lost on the rebuilt namespace.Once
__esModuleis gone, webpack's__webpack_require__.n( wp.foo )interop helper takes the CommonJS branch and returns() => wp.foo(the whole namespace) instead of() => wp.foo.default. Consumers doingimport isShallowEqual from 'wordpress/is-shallow-equal'then call the namespace as a function and crash:```
TypeError: isShallowEqual is not a function
```
Every Gutenberg package with a default export and
wpScriptenabled was affected. The original webpack pipeline preserved the marker via the IIFE's internalObject.defineProperty(…, '__esModule'), so the bundled WordPress core build still works — only Gutenberg trunk (and the nightly build) regress.How?
The second footer (the one that materializes getters) now branches on
wpScriptDefaultExport:wpScriptDefaultExportis falsy): the global is the ES module namespace. Re-seed theObject.assigntarget with a non-enumerable, writable__esModulewhen the source has it. The perf optimization (getters → data properties, ~2x cheaper per access in V8) is preserved because the enumerable getters still flow throughObject.assign. The target's__esModuleis marked writable so a hypothetical future esbuild emit that makes the source's__esModuleenumerable would not trip a strict-modeTypeErrorwhenObject.assignwalks it.wpScriptDefaultExportpackages: the first footer has already unwrapped the global to its default value (today always a function or class for the eight current opt-ins:api-fetch,deprecated,dom-ready,redux-routine,server-side-render,warning,token-list,shortcode). Keep the plainObject.assign( {}, ns )to materialize any getters; this also drops any stray__esModuleflag the unwrapped default might carry, which is correct — the global no longer represents an ES module namespace, so webpack's interop should take the CJS branch and return the value directly. (For function/class defaults the second footer is a no-op anyway becausetypeof !== 'object', so no observable change for current packages.)The new comment block documents both branches.
Testing Instructions
npm run build:packages) and inspect the IIFE footer for one of the affected globals:```
tail -c 200 build/is-shallow-equal/index.min.js
```
The footer now reads (roughly):
```
if(wp.isShallowEqual&&typeof wp.isShallowEqual==='object'){wp.isShallowEqual=Object.assign(wp.isShallowEqual.__esModule?Object.defineProperty({},'__esModule',{value:true,writable:true}):{},wp.isShallowEqual);}
```
```js
wp.isShallowEqual.__esModule // true
Object.keys( wp.isShallowEqual ) // does not include '__esModule'
typeof wp.isShallowEqual.default // 'function'
```
TypeError: vr(...) is not a function(webpack's__webpack_require__.nhelper) fromwc-blocks-data.js. After this PR the Cart block renders.wpScriptDefaultExportpackages still expose their default value as the global (typeof wp.apiFetch === 'function',new wp.shortcode( … ), etc.).Testing Instructions for Keyboard
Not applicable — build-tool change with no UI surface.
Screenshots or screencast
Not applicable.
Use of AI Tools
Yes — Claude Code was used to draft the fix from the analysis in the linked issue and to run iterative adversarial reviews (multiple passes of compound-engineering and Codex CLI). I verified each step, wrote the test scenarios, and reviewed every line.