Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Include named exports in CJS shims when using
importSync
`importSync()` is intended to work like `import()` (buy sync), which, in turns, is expected to work like a namespace import – `import * as NS from "foo";`. In general, we would expect that to be interchangable with `const NS = importSync("foo");`. Prior to this commit, this is not true for commonjs packages: ```js import * as QUnit1 from "qunit"; // => { QUnit, assert, begin, config, ... } QUnit1.assert // => function const QUnit2 = importSync("qunit"); // => { default: { QUnit, assert, begin, config, ... } } QUnit2.assert // => undefined ``` Of course, using ES imports on commonjs packages is, in itself, an ill-defined concept, but nevertheless, it is a very common interop requirement. The previous behavior is introduced in embroider-build#1076. The intent appears to be fixing a different interop issue, where we would expect these to work the same way: ```js import QUnit1 from "qunit"; // => { Qunit, assert, begin, config, ... } QUnit1.assert // => function const QUnit2 = importSync("qunit").default; // => { QUnit, assert, begin, config, ... } QUnit2.assert // => function ``` The fix in embroider-build#1076, however, broke the exepctation that `import()` should behave like a namespace improt statement for CJS modules. This commit attempts to satisfy both of these requirements, with one caveat: `importSync("foo").default` is present on the shimed CJS modules, but in `import * as FOO from "foo";`, `FOO.default` is undefined. Arguably, this is a bug in webpack's implementation – if you are able to write an import statement that references the default export, you should be able to do the same in the namespace import as per the spec. So, the implementation here is different but more correct.
- Loading branch information