Incorrect CJS import with `esmoduleInterop: true`

[beeequeue]

Repro: https://github.com/BeeeQueue/esbuild-cjs-no-default

TL;DR: import core from "@actions/core" generates invalid code which fails when running, presumably because it does something funky with the exports somewhere.

I tried to look around a bit to see if I could find what they're doing weird but couldn't find anything myself.

It can be worked around with import * as, but if that is the intended behavior there should probably be a warning about it.

Maybe related to #1079?

[hyrious]

This behavior is aligned to babel since 0.14.5. Which means: if a cjs module defined exports.__esModule, then its default export in esm context is always exports.default regardless of whether this value exists (it will be undefined if not exist). This is how front-end ecosystem currently works.

You can still switch to the node behavior by renaming your entry file to end with .mjs or .mts, which means it will consider if exports.default exists.

[beeequeue]

Then I guess the TS types are incorrect, since we should presumably get an error there if the default export doesn't exist?

But shouldn't esbuild be creating the default export for us, considering we're using esmoduleInterop ?

[hyrious]

I think you are mis-understanding what esModuleInterop does. As a summary, this setting only affects what import * as behaves so that with any cjs style as input, it give you a seemingly same esm namespace, which is already handled by esbuild.

[beeequeue]

But from my experience it also changes how default imports work - if it's false you have to use * as, while if it's true it allows using the default import as a namespace import (which we can see in the repro!)

My main concern is that there should be some error somewhere along the line here to indicate that something has gone awry, and will crash in runtime - but I have no idea where it would go

[hyrious]

while if it's true it allows using the default import as a namespace import

I can't reproduce this using tsc:

// a.ts
import core from "@actions/core"
console.log(core.info) // expect: [Function: info]

tsc a.ts --esModuleInterop

// a.js
"use strict";
var __importDefault = (this && this.__importDefault) || function (mod) {
    return (mod && mod.__esModule) ? mod : { "default": mod };
};
exports.__esModule = true;
var core_1 = __importDefault(require("@actions/core"));
console.log(core_1["default"].info); // expect: [Function: info]

node a.js

TypeError: Cannot read properties of undefined (reading 'info')

Notice the error message actually matches the babel behavior: the default export is always exports.default, which is undefined here.

[beeequeue]

Sorry, I meant the type checking changes to allow it. I did not know what the code output would so I didn't mean to make it sound like that was what I was referring to.

But if that's where the error is then I should probably open a ticket there then.

It would be nice with a warning if you imported default from a package where it is undefined, though

[hyrious]

Well, the type checking is not totally wrong. As I said before this is correct if you are running this code in esm format in node. (e.g. You name it with '.mjs' extension or add 'type: module' in package.json and run it with node directly). This is definitely some sort of historical issue when the idea of 'es modules' firstly introduced in typescript.

My suggestion is, platform matters how they excutes. You can and have to follow one of the cjs-to-esm rule in esbuild. Other bundlers may not give you such choice.

[evanw]

I wrote up some more documentation about esbuild's specific behavior regarding default imports of CommonJS code here: https://esbuild.github.io/content-types/#default-interop. This is the TL;DR at the end:

By default, esbuild will use the Babel interpretation. If you want esbuild to use the Node interpretation instead, you need to either put your code in a file ending in .mts or .mjs, or you need to add "type": "module" to your package.json file.

[guybedford]

Great writeup on the problem, glad this is being so clearly communicated. I just wanted to share a brief note here on a hack that eg JSPM uses for this -

When importing an ES module from a CJS module:

const dep = require('dep');
// this is the particular interop pattern that causes the issue
const depDefault = dep.__esModule ? dep.default : dep;

is transformed into:

import * as depNs from 'dep';
// try fetch the nested default if it exists (for support with Node.js interop), falling back to "babel interop"
const dep = depNs.default || depNs;
// this pattern now works for both CJS + ESM imports / Node.js or Babel interop
const depDefault = dep.__esModule ? dep.default : dep;

This does in turn have another tradeoff that an ES module with a default and named exports cannot properly be imported into CommonJS. But I would argue the problem is not so much the default export as it is combining default with named exports. And recommending against using the default export doesn't seem ideal either.

I hope it's the same right interop discussion on the above, but let me know if I've missed any details too.