Plugins: named exports not working with __esModule if default also present

[jbt]

• Rollup Plugin Name: commonjs
• Rollup Plugin Version: 13
• Rollup Version: tested with 2.13.1, 2.14, 2.15
• Operating System (or Browser):
• Node Version: 10, 12

How Do We Reproduce?

I discovered this when working with the is-hotkey somewhere deep in my dependency tree. That module seems to be compiled from esm to commonjs using fairly standard babel configuration.

The essence of the published module structure is:

'use strict';

Object.defineProperty(exports, "__esModule", {
  value: true
});

// Definitions etc.

exports.default = isHotkey;
exports.isHotkey = isHotkey;
exports.isKeyHotkey = isKeyHotkey;
// other exports...

Repl here: https://repl.it/repls/WoefulColorfulWebportal#input.js

The authored esm version of that module exports both default and named exports, which are then transpiled to the above structure and distributed as commonjs

Expected Behavior

For esm modules transpiled and distributed as commonjs, I expect to be able to use named exports the same as if I was using the original esm directly. Such as:

import { isKeyHotkey } from 'is-hotkey';

console.log(isKeyHotkey); // should be a function

(this previously worked up to plugin-commonjs@12)

Actual Behavior

Named exports are all undefined as of plugin-commonjs@13 if there is also a default export present, because only the default export is extracted by unwrapExports.

Here's an even-more-reduced-down version: https://repl.it/repls/MonthlyZestyCertification#input.js - this one uses the simplest possible example I could think of with default + named exports transpiled to cjs by babel, babel repl link

I tried manually patching the specific is-hotkey module in my deps and immediately ran into the same error on a different dependency that followed the same pattern, so it feels like this may not be just an uncommon edge-case.

I've read over #423 and #427 and apologies if I'm missing anything in the discussion - it looks like this change was deliberate, but it seems to be breaking on what feels like a fairly common setup.

[FredKSchott]

Another user has run into this problem as well: https://www.pika.dev/npm/snowpack/discuss/367

Looking at the unwrapExports() code:

function unwrapExports (x) {
    return x && x.__esModule && Object.prototype.hasOwnProperty.call(x, 'default') ? x['default'] : x;
}

Is this just a simple bug?

function unwrapExports (x) {
-   return x && x.__esModule && Object.prototype.hasOwnProperty.call(x, 'default') ? x['default'] : x;
+   return x && !x.__esModule && Object.prototype.hasOwnProperty.call(x, 'default') ? x['default'] : x;
}

unwrapExports is given the already processed Common.js module, where x.foo === module.exports.foo. We should only need to unwrap further if x was not compiled with __esModule support, in which case everything would still live on the default export. If __esModule exists, there's no need to unwrap.

I can confirm this by removing Object.defineProperty(exports, "__esModule", {value: true}); from is-hotkey in my project, and watching it compile correctly.

Happy to create a PR if this is the case.

[FredKSchott]

Looking at this more closely, it definitely seems like unwrapExports was built to run only if __esModule exists. But, unwrapExports() is given the already unwrapped module. So, maybe this is no longer needed as of the latest release of Rollup?

[lukastaegert]

I started #481 to move all interop issues including this one forward.

[jbt]

I can't say for certain whether all of the different __esModule cases are fixed now, but v14 definitely fixes the bugs I was seeing when I opened this - great stuff! Happy for this issue to be closed unless there's anything else I've missed

[shellscape]

Thanks for letting us know!