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

Disable __esModule interop handling #2

Closed
wants to merge 1 commit into from

Conversation

guybedford
Copy link
Collaborator

I don't think we should be making allowance for exports.default under __esModule interop at the cost of breaking the critical invariant that:

import cjs from 'cjs' will always correspond to the module instance in the CJS loader.

I do think this invariant is absolutely necessary to maintain in all cases.

@bmeck
Copy link
Owner

bmeck commented May 12, 2020

Cc @jkrems

@jkrems
Copy link

jkrems commented May 12, 2020

I think there's a fundamental question of "what are we trying to do here". If the goal is to create a safer transition story from transpilation of a package tree to native ESM, making default do different things would completely undo it in my opinion. Or put another way: I'm not sure what goal we are achieving if we don't let exports.default override the default binding under __esModule.[1]

I agree that having a simple, consistent rule is valuable ("default binding is the module.exports value"). But I'm not sure how to square that with the value proposition of incremental adoption here.

[1] There's a squishy "it may make it more convenient to use hand-written CJS libraries that happen to have easily parsable exports". But I'm not sure that's a) a reasonable expectation for the parser and b) a major problem. If that's the goal, parsing is much more fragile as a solution since we're not targeting generated code.

Incremental Adoption

My understanding was that supporting custom CJS bindings was supposed to address the issue of incremental adoption: Being able to have a CJS and an ESM build in the same package that interacts with files from its dependencies. Example:

// my-lib/default.mjs
import X from 'dep-x';
import {Y} from 'dep-y';
// dep-z/default.cjs
const X = require('dep-x').default; // w/ __esModule check
const Y = require('dep-y').Y; // w/ __esModule check

// dep-x/default.mjs
import {Y} from 'dep-y';
export default `${Y} - X`;
// dep-x/default.cjs
const _dep_y = require('dep-y'); // w/ __esModule check
exports.default = `${_dep_y.Y} - X`;
exports.__esModule = true;

// dep-y/default.mjs
export const Y = 'Y';
// dep-y/default.cjs
exports.Y = 'Y';
exports.__esModule = true;

These packages exist today and each is linking against the other's CJS version from node's perspective. In other words: my-lib/default.cjs loads dep-x/default.cjs and both load dep-y/default.cjs. Now I want to enable the exports field in my-lib - but I can't. All of my dependencies (and their dependencies transitively) need to change first. Otherwise my existing source ESM won't work. If I change the source ESM in my-lib to work against dep-x/default.cjs, it would break as soon as dep-x wants to add exports.

Having that level of forced coordination across a potentially non-trivial dependency tree is unfortunate. And that's without potential cycles which may move it from "unfortunate" to "real ugly".

(I'm more than happy to blame the code generation of TypeScript/babel/etc. for this issue but it's still an issue that users may face.)

If we want to make this work, it would require that default exports transpiled to CJS would be bound to default. Which would break the existing invariant.

@guybedford
Copy link
Collaborator Author

Superseded by #3.

@guybedford guybedford closed this May 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants