importSync() vs commonjs packages

[chancancode]

My understanding that the intention is for importSync() to behave exactly like import(), but synchronous. In turns, that should mean importSync() should behave exactly like a namespace import, i.e. import * as NS from "...";.

I found that this is not the case, at least when importing from commonjs packages:

ember new --yarn --embroider zomg
yarn add -D @embroider/macros

import { importSync } from '@embroider/macros';

import * as ESM1 from 'qunit'; 
// => { QUnit, assert, begin, config ... }
// const ESM1 = __webpack_require__( /*! qunit */ "../../qunit/qunit/qunit.js");
// Notably: ESM1.default === undefined, which is not per spec, but it's how webpack commonjs interop behave

const ESM2 = importSync('qunit');
// => { default: { QUnit, assert, begin, config ... } }
// const ESM2 = esCompat(__webpack_require__( /*! qunit */ "../../qunit/qunit/qunit.js"));
// where
// function esCompat(m) {
//  return m?.__esModule ? m : { default: m };
// }

import { module as M1 } from 'qunit';
// => function module(...) { ... }
// const M1 = __webpack_require__( /*! qunit */ "../../qunit/qunit/qunit.js").module;

const M2 = importSync('qunit').module;
// => undefined
// const M2 = esCompat(__webpack_require__( /*! qunit */ "../../qunit/qunit/qunit.js"));

import Q1 from 'qunit';
// => { QUnit, assert, begin, config ... }
// const Q1 = __webpack_require__.n(__webpack_require__( /*! qunit */ "../../qunit/qunit/qunit.js"));
// where
// function n(module)=>{
//   return module && module.__esModule ? ()=>(module['default']) : ()=>(module);
// }

const Q2 = importSync('qunit').defualt;
// => undefined
// const Q2 = esCompat(__webpack_require__( /*! qunit */ "../../qunit/qunit/qunit.js")).default;

esCompat is added in #1076

From the brief PR description, it seems like my understand is correct (that importSync should behave like imports) and it was added to fix this very bug, albeit perhaps with a different scenario in mind. It seems like it is trying to perform a similar function as the __webpack_require__.n shim, but as you can see the implementation is quite different and it causes a different and undesirable result, at least in this case.

In case qunit is special, I also tested on the "virtual-dom" NPM package as that's what I happen to have in the app, with similar results.

[chancancode]

Perhaps esCompat should do something like this:

function esCompat(m) {
  if (m?.__esModule) {
    return m;
  } else {
    return { default: m, ...m };
  }
}

This is slightly different than the webpack result (importSync("foo") !== import * as Foo from "foo"), in that the webpack namespace import does not add the default key, but this is more to spec anyway.

If we want to really follow the spec, the object also should be sealed and have a null prototype, but the existing code already doesn't care about that it seems.

[ef4]

Thanks, this makes sense. I agree, we can return { default: m, ...m } where we currently return only { default: m }.

[chancancode]

Cool, I can try to PR the change