Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adjust esbuild's handling of default exports and the __esModule marker #1849

Merged
merged 5 commits into from
Dec 14, 2021

Conversation

evanw
Copy link
Owner

@evanw evanw commented Dec 14, 2021

This change requires some background for context. Here's the history to the best of my understanding:

When the ECMAScript module import/export syntax was being developed, the CommonJS module format (used in Node.js) was already widely in use. Because of this the export name called default was given special a syntax. Instead of writing import { default as foo } from 'bar' you can just write import foo from 'bar'. The idea was that when ECMAScript modules (a.k.a. ES modules) were introduced, you could import existing CommonJS modules using the new import syntax for compatibility. Since CommonJS module exports are dynamic while ES module exports are static, it's not generally possible to determine a CommonJS module's export names at module instantiation time since the code hasn't been evaluated yet. So the value of module.exports is just exported as the default export and the special default import syntax gives you easy access to module.exports (i.e. const foo = require('bar') is the same as import foo from 'bar').

However, it took a while for ES module syntax to be supported natively by JavaScript runtimes, and people still wanted to start using ES module syntax in the meantime. The Babel JavaScript compiler let you do this. You could transform each ES module file into a CommonJS module file that behaved the same. However, this transformation has a problem: emulating the import syntax accurately as described above means that export default 0 and import foo from 'bar' will no longer line up when transformed to CommonJS. The code export default 0 turns into module.exports.default = 0 and the code import foo from 'bar' turns into const foo = require('bar'), meaning foo is 0 before the transformation but foo is { default: 0 } after the transformation.

To fix this, Babel sets the property __esModule to true as a signal to itself when it converts an ES module to a CommonJS module. Then, when importing a default export, it can know to use the value of module.exports.default instead of module.exports to make sure the behavior of the CommonJS modules correctly matches the behavior of the original ES modules. This fix has been widely adopted across the ecosystem and has made it into other tools such as TypeScript and even esbuild.

However, when Node.js finally released their ES module implementation, they went with the original implementation where the default export is always module.exports, which broke compatibility with the existing ecosystem of ES modules that had been cross-compiled into CommonJS modules by Babel. You now have to either add or remove an additional .default property depending on whether your code needs to run in a Node environment or in a Babel environment, which created an interoperability headache. In addition, JavaScript tools such as esbuild now need to guess whether you want Node-style or Babel-style default imports. There's no way for a tool to know with certainty which one a given file is expecting and if your tool guesses wrong, your code will break.

This release changes esbuild's heuristics around default exports and the __esModule marker to attempt to improve compatibility with Webpack and Node, which is what most packages are tuned for. The behavior changes are as follows:

Old behavior:

  • If an import statement is used to load a CommonJS file and a) module.exports is an object, b) module.exports.__esModule is truthy, and c) the property default exists in module.exports, then esbuild would set the default export to module.exports.default (like Babel). Otherwise the default export was set to module.exports (like Node).

  • If a require call is used to load an ES module file, the returned module namespace object had the __esModule property set to true. This behaved as if the ES module had been converted to CommonJS via a Babel-compatible transformation.

  • The __esModule marker could inconsistently appear on module namespace objects (i.e. import * as) when writing pure ESM code. Specifically, if a module namespace object was materialized then the __esModule marker was present, but if it was optimized away then the __esModule marker was absent.

  • It was not allowed to create an ES module export named __esModule. This avoided generating code that might break due to the inconsistency mentioned above, and also avoided issues with duplicate definitions of __esModule.

New behavior:

  • If an import statement is used to load a CommonJS file and a) module.exports is an object, b) module.exports.__esModule is truthy, and c) the file name does not end in either .mjs or .mts and the package.json file does not contain "type": "module", then esbuild will set the default export to module.exports.default (like Babel). Otherwise the default export is set to module.exports (like Node).

    Note that this means the default export may now be undefined in situations where it previously wasn't undefined. This matches Webpack's behavior so it should hopefully be more compatible.

    Also note that this means import behavior now depends on the file extension and on the contents of package.json. This also matches Webpack's behavior to hopefully improve compatibility.

  • If a require call is used to load an ES module file, the returned module namespace object has the __esModule property set to true. This behaves as if the ES module had been converted to CommonJS via a Babel-compatible transformation.

  • If an import statement or import() expression is used to load an ES module, the __esModule marker should now never be present on the module namespace object. This frees up the __esModule export name for use with ES modules.

  • It's now allowed to use __esModule as a normal export name in an ES module. This property will be accessible to other ES modules but will not be accessible to code that loads the ES module using require, where they will observe the property set to true instead.

These changes mean that esbuild now passes all of my test cases for this interoperability problem: https://github.com/evanw/bundler-esm-cjs-tests.

Fixes #1591
Fixes #1719
Closes #1622
See also #532

@lukeed
Copy link
Contributor

lukeed commented Dec 14, 2021

set the default export to module.exports.default (like Babel). Otherwise the default export is set to module.exports (like Node).

Note that this means the default export may now be undefined in situations where it previously wasn't undefined. This matches Webpack's behavior so it should hopefully be more compatible.

This is a little worrying tbh. Node should be followed only by default as it's the official implementor and team behind the spec. Babel and webpack implementations are carryovers from early days and while their outputs have been working in the wild, it's only thanks to the interop helper(s) that everything has included since to accommodate.

Defaulting to Node behaviors will still work in all webpack/Rollup/babel interops, because it already does.

Outputting or expecting exports.default (even sometimes) will not auto-resolve in Node environments correctly, which is problematic because now esbuild will sometimes output different/broken code, even if it should have run everywhere just fine:

// foo.cjs
module.exports = { foo: 123 };

// bar.mjs
import foo from "./foo.cjs";
console.log(foo);
// Node: { foo: 123 }
// Proposed: { default: { foo: 123 } }

@guybedford
Copy link
Contributor

Agree with @lukeed here that implementing different ESM -> CJS interop than Node.js will cause more issues in bundling Node.js apps in future, it's important to take into account future compatibility as well as backwards compatibility in the calculus here.

@guybedford
Copy link
Contributor

I haven't done a full review, but apart from the one concern brought up everything seems great to me - thank you for working on this!

@vovacodes
Copy link

vovacodes commented Dec 14, 2021

Agree with the folks above, I feel like the "just works" case should follow the Node standard. I understand where the desire to not break the old code comes from and totally appreciate it, but for the sake of the future health of the ecosystem I would not make the non-spec behavior work out of the box. Perhaps with a config flag acknowledging the "synthetic/compat" nature of that behavior.

@evanw
Copy link
Owner Author

evanw commented Dec 14, 2021

// foo.cjs
module.exports = { foo: 123 };

// bar.mjs
import foo from "./foo.cjs";
console.log(foo);
// Node: { foo: 123 }
// Proposed: { default: { foo: 123 } }

That's not right. I think I forgot a "not" in the bit about .mjs. I'll fix the description. The point of special-casing .mjs is that the file ending in .mjs is a good signal that we should use Node-compatible semantics no matter what. So if you're bundling ESM code intended for Node (i.e. the extension is .mjs or there is "type": "module"), then Node semantics will now be used unconditionally. This improves esbuild's Node compatibility (i.e. fixes #1719) and is also what Webpack does too.

FWIW Node, current esbuild, and proposed esbuild all return { foo: 123 } here. Nothing returns { default: { foo: 123 } }. The difference is between returning { foo: 123 } and returning undefined. With the proposed behavior, it would only return undefined if you change the file extension from .mjs to .js and use module.exports = { foo: 123, __esModule: true }. That's saying the file is a legacy Babel cross-compiled ESM-to-CJS module without a default export. Webpack returns undefined here too. This code wouldn't even run in Node because you can't use import in a .js file without "type": "module".

Agree with the folks above, I feel like the "just works" case should follow the Node standard. I understand where the desire to not break the old code comes from and totally appreciate it, but for the sake of the future health of the ecosystem I would not make the non-spec behavior work out of the box.

I disagree. I am trying to make esbuild compatible with the existing ecosystem by default and since esbuild is a bundler, that means Webpack. Webpack is by far the most widely used bundler and the vast majority of published packages intended for bundling are tuned for Webpack. It doesn't make sense to me to cause pain for esbuild's users by doing something incompatible with Webpack regarding default exports. That will just end up making esbuild harder to use. If you can convince Webpack to break compatibility with the existing package ecosystem and switch to using Node's behavior everywhere, then it could be reasonable to switch esbuild's behavior too, but I don't think esbuild should start doing that by itself.

@lukeed
Copy link
Contributor

lukeed commented Dec 14, 2021

Ah cool, thanks for the clarification and sorry for the confusion. I think this change is fine then, but as a general rule, I would be hesitant to conform to webpack behaviors. It may be most popular by the numbers, but it does have ESM & entry resolution issues that are specific to webpack that package authors frequently have to sidestep (including myself). Thankfully esbuild does not suffer from any of these missteps

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants