CommonJS Node.js Default Interop Case

[guybedford]

• Rollup Plugin Name: @rollup/plugin-commonjs
• Rollup Plugin Version: 15.0.0

Feature Use Case

This is an edge case of the Node.js ESM / CJS interop.

The Node.js ESM / CJS interop semantics are described in the following section of the documentation - https://nodejs.org/dist/latest-v15.x/docs/api/esm.html#esm_commonjs_namespaces.

Up until version 14, RollupJS actually exactly followed these Node.js semantics (unintentionally yes, but it did nonetheless!).

A good test example is the following:

main.js

import * as m from './dep.js';
console.log(m);

dep.js

exports.__esModule = true;
exports.default = 'asdf';

Running:

rollup main.js --plugin @rollup/plugin-commonjs > build.js
node build.js

For version 14 of the plugin we get the output:

[Object: null prototype] {
  __esModule: true,
  default: { __esModule: true, default: 'asdf' },
  __moduleExports: { __esModule: true, default: 'asdf' }
}

While for version 15 and up we get the output:

[Object: null prototype] { __esModule: true, default: 'asdf' }

That is, it was only as recently as this August that RollupJS started diverging from what is considered the Node.js interop at this point.

Feature Proposal

I would like to propose that RollupJS reconsider the default behaviour here and consider aligning with Node.js.

Webpack is specifically not going with Node.js alignment on this edge cases, but RollupJS still can.

I think this kind of thing is important to get good behaviours universally in the ecosystem moving between environments. And I don't think blindly following Webpack is the recipe for success necessarily even though it is the most tempting path due to the majority of the ecosystem expectations being built there.

I understand this is a difficult discussion though so the first thing to do is to remember that this is an edge case we are discussing. It already affects a minority of cases.

But these small edge cases are what define the development experience when working with JavaScript for many developers.

If we can improve these friction points and make the interops align then we do well by the ecosystem.

There is a decision here to be made in either aligning with Webpack or Node.js here. I'm pushing for the Node.js side, because I feel that as a platform it has a long future and that Node.js build workflows should be considered first class.

Since this behaviour has only been in the plugin for two months I don't think it unreasonable to consider making a breaking change to support this sort of thing, and it would be a step made by RollupJS as a project to improve future interop compatibiity.

Further discussion / questions / feedback on this topic very welcome.

//cc @LarsDenBakker @lukastaegert @shellscape

[guybedford]

/cc @weswigham it would be really interesting to hear your perspective here from the TypeScript team in terms of where Node.js interop meets bundlers and ways of tackling this default interop alignment.

[bmeck]

I would agree that maximal alignment would be great so that we can get a uniform ecosystem and not have people include feature detection code within libraries.

[robpalme]

The choice seems fairly clear cut - the purpose of @rollup/plugin-commonjs is to reflect Node behavior and so it ought to be updated to match.

I see standards compliance, or in this case "preservation of platform semantics", as one of the reasons why folk choose Rollup today. Maybe you could even consider it a differentiator. So let's retain that.

[jkrems]

I think this is a very valuable discussion to have. Also worth noting: Webpack is more likely to either pick other semantics in the future or have flags that change them. Node.js (because it needs to keep running existing code) is much more hesitant to break code that may depend on the current .default behavior. But as @guybedford mentioned above - the incentive created by webpack's current behavior makes it a tougher choice.

[lukastaegert]

While it is certainly prudent to align with Node, and this might even become a default, I am convinced we should keep the existing interop behaviour at least behind an option. TypeScripts esModuleInterop flag could serve as an example.

Talking about TypeScript, their behaviour without interop seems rather inconsistent to me:
https://www.typescriptlang.org/play?esModuleInterop=false&module=1#code/JYWwDg9gTgLgBAKjgQwM5wGYQpqERwDkWEhA3AFADGEAdqhADYCmAdIxAOYAUJAlJSA

You would expect that a namespace import converted to CommonJS would return an object where the default property is the actual required file. However, the required file becomes the namespace. I guess this is something for them to sort out.

Babel, on the other hand, goes all-in on the __esModule interop pattern, and in contrast to Webpack, I would see Babel very much as a standard: https://babeljs.io/repl#?browsers=defaults%0A&build=&builtIns=false&spec=false&loose=false&code_lz=JYWwDg9gTgLgBAKjgQwM5wGYQpqERwDkWEhAUGQMYQB2qEANgKYB0DEA5gBQkCUA3GSA&debug=false&forceAllTransforms=false&shippedProposals=false&circleciRepo=&evaluate=false&fileSize=false&timeTravel=false&sourceType=module&lineWrap=true&presets=env%2Creact%2Cstage-2&prettier=false&targets=&version=7.12.3&externalPlugins=

I hope there is also discussion with them to align with the Node behaviour?

Also to be clear, a lot of the changes in the v15 of the CommonJS plugin were not triggered by incompatibilities with Webpack alone, but rather by inconsistencies with TypeScript.

At the moment, this is a whole ecosystem vs. the Node way, and while there are good arguments for adopting the Node way, we will just make the interop situation even worse if major players like Babel are not on board.

[guybedford]

Thanks for the thoughtful feedback here. Agreed an option might well make sense.

When talking about Babel, I think the important distinction to consider is that there is a difference between a CommonJS module with an __esModule export and an ES module. The term "faux module" has come to emerge to describe these.

So the difference is that if you are in a CommonJS "faux module" and you load another CommonJS "faux module", then I think it is expected that you get a different interop from when you are in an ES module (real module!), and loading a CommonJS "faux module". The desire to have this interop match the faux module interop is what is causing the difficulties here. Babel has nothing to do with this case though.

Certainly TypeScript alignment is important, and that's why I copied in some TypeScript people here. I do hope we could get more feedback on how they hope to handle these cases and I agree that RollupJS should not have to make these kinds of decisions as a project alone.

[lukastaegert]

Babel has nothing to do with this case though

When I create a project as ESM that has an external dependency "foo" which is faux ESM, and I convert my project to CommonJS via Babel, I will very much get the wrong behaviour. Not sure there is anything they can do to fix this, though.

In the end, converting a project with external dependencies via Babel will just never be compatible with Node I guess.

[guybedford]

Again reiterating the edge case aspect, named exports work out as expected here. Let's flesh out your example for the edge case that breaks.

Say we have a Babel faux module like:

exports.__esModule = true;
exports.default = 'default';

To import this module in Node.js, we need to do a double default access:

import m from 'cjs';
const val = m.default;
console.log(val);
// outputs: 'default'

We then convert this module into CommonJS:

const _cjs = require('cjs');
const m = (_cjs.__esModule ? _cjs : { default: _cjs }).default;
const val = m.default;
console.log(val);
// outputs: undefined

The value of the default export changes during this conversion process since the act of converting a real ES module loading a faux module into a faux module loading a faux module causes an interop change that breaks expectations.

Now I agree that ideally Node.js would have just used the __esModule signifier when importing CommonJS, but the problem we had was that we couldn't be sure we'd definitely be detecting all the named exports correctly. And if there was anything missing you would otherwise have no way at all to reach it since __esModule is a down-levelling. In addition we had backwards compatibility concerns by the time we were at a point we had shipped named exports.

In terms of the Babel output it is correct. There is no action for Babel to take here. The fix in this case is relatively straightforward for the user though. They control the code as they are the ones doing the transpilation here in the first place. So they are in a position to fix the problem.

You do some debug logging and you notice that the value isn't the default property anymore, so you correct the code with something like:

import m from 'cjs';
const val = (m.default || m);
console.log(val);
// outputs: 'default'

in order to get something to work in both faux ESM <-> fauxESM and ESM <-> faux ESM modes.

Given that this interop does already vary between environments, having the above is likely advised anyway for users if they want their code to work.

It's important to remember that as soon as next year when Node.js 10 LTS ends it's no longer necessary to compile code into CommonJS. On the other hand we will still be importing CommonJS into real ES modules in the ecosystem for many many years! So in terms of the calculus, I do think the argument for Node.js interop outweights a probem of the order of a year that can be corrected.

[weswigham]

Personally, I don't think it's reasonable to write a module that's compatible with both being able to compile to the faux esm the tool chain is used to and run under node's real esm at present - there are enough gotchyas and edge case differences that attempting to do so requires a wealth of specialized knowledge the average user is unlikely to have, and you usually still have small differences in behavior that could end up affecting someone down the line. I, personally, am of the opinion that one should either be using node esm, or transpiled esm, but not both in the same codebase. Likewise, tools like roll-up should probably be able to figure out which was a package's source's intent (likely via export maps - if an esm module is found via node import condition, it must require node style interior to work, if it's found via module condition or module legacy field, it must be transpiled esm and need old style interop). At least that's where I'm at right now. I don't think inventing a "new UMD wrapper", and expecting people to use it to try to paper over esm format interpretation differences is a good idea. If we were to do that, I think we'd but justified in being accused of repeating the problems of the module systems of yore but with a new coat of paint.

[nicolo-ribaudo]

My 2 cents.

From an user perspective, CJS-ESM interop is hard. With the Node.js interop, when importing a dependency I have to check:

1. If it's written in CommonJS, import foo from "foo" probably works.
2. If it's written and published as ESM, import foo from "foo" probably works.
3. If it's written as ESM but published as transpiled CJS, I have to do something like import _foo from "foo"; const foo = _foo.default;

(1) and (2) make sense, (3) does not. I wish Node.js supported import foo from "foo"; for getting module.exports.default automatically, but I understand why it's not possible.
(3) is a rule that everyone needs to learn, since more and more people will start using native ESM (while still having a ton of transpiled dependencies).

Bundlers could try to make it more ergonomic, by checking if there is an __esModule property and extracting module.exports.default instead of module.exports. However, that's one more rule that developers need to be aware of: (3) would then become something like

3. If it's written as ESM, published as transpiled ESM and used on the server, import foo from "foo" probably works.
4. If it's written as ESM, published as transpiled ESM and bundled for the client, I have to use import _foo from "foo"; const foo = _foo.default;

That is, one more rule that I need to be aware of. Having an exception ( (3) ) already imposes a cognitive load, but having an exception that is only an exception sometimes is probably worse. Rollup should follow Node.js' interop strategy, even if it's sub-optimal in some cases.

[teohhanhui]

As a user, having to do .default is counter-intuitive and just unergonomic.

[shrinktofit]

I wonder if rollup(plugin-commonjs)could as well align its default CommonJS detection algorithm with NodeJS, without inspecting file content, at least for modules under node_modules:

1. .mjs files, or .js files having type:'module' in nearest package.jon, are ESM;

2. all other .js files are CommonJS.

[guybedford]

@shrinktofit discussion on this should really be a separate issue, but to mention briefly - "type": "module" can be useful to indicate JS files that don't use import or export syntax that should still be parsed as ES modules, but disallowing ".js" files that use import or export syntax within a non "type": "module" boundary is likely unnecessary friction as they wouldn't execute in Node.js so the subset of Node.js semantics that execute correctly is contained within the RollupJS semantics then fine.

[lukastaegert]

One thing to note:
At the moment, the only way to have everything "just work" is to never use default exports in ESM code that is converted to CommonJS.

However there is an "interop" pattern, or rather a conversion pattern, that allows to convert default exports to something that is compatible with Node, which is to convert

export default 'foo';

to

module.exports = 'foo';

This has some downsides:

• It does not support circular dependencies
• it does not support live-bindings of the default export
  (well, actually that is two variations of the same short-coming)
• it does not work if there are also named exports

As for the latter, I think everyone will agree by now that in such a situation, you should convert everything to named exports if you want to have any sane chance of working interop. But for library authors, having the opportunity to specify only a default export is important.

This interop pattern is actually what Rollup core has been using as default for years, and I was only now considering to change it to align more with the rest, i.e. Babel and Webpack.

But the recent discussions make me feel that we should actually double down on this old pattern. This would not be something for this plugin but mostly for Rollup core, which is very capable of finding out if there are potential circular dependencies or live-binding issues and can warn about this.

It's important to remember that as soon as next year when Node.js 10 LTS ends it's no longer necessary to compile code into CommonJS.

That is just not true, unless you want to maintain a mostly CommonJS code-base. Because the only "safe" solution for the dual-state hazard still remains to have an ESM wrapper around a CommonJS core (there is also the alternative of JSON modules, but there is still danger from assuming reference identities). So if you want to author your library as ESM only in the hope of eventually getting rid of the CommonJS parts, you need something like Rollup for the shared core.

A last point I want to add:

If a module contains the __esModule marker, then this means that the author of the module had the intention that a default import of the module would return exports.default and not module.exports. Not doing the interop means that we knowingly produce potentially broken code. I do not want to say we should not align with Node as default, but we should at least warn the user whenever such a marker is encountered in the code that they may encounter bugs, and also tell them what they can do against this, i.e. either activate interop or write their import in a certain way, or use one of the patterns @guybedford mentioned above.

Adding such a warning would not be a big issue, but I think it would be fundamentally important.

[guybedford]

However there is an "interop" pattern, or rather a conversion pattern, that allows to convert default exports to something that is compatible with Node, which is to convert export default 'foo'; to `module.exports = 'foo'``.

@nicolo-ribaudo perhaps Babel can bring this back as a custom option? I recall the reason for changing this was that adding named exports becomes an API break, but perhaps the UX here can be reconsidered a bit.

As for the latter, I think everyone will agree by now that in such a situation, you should convert everything to named exports if you want to have any sane chance of working interop. But for library authors, having the opportunity to specify only a default export is important.

I definitely agree that in general discouraging named exports and default exports together for ESM -> CJS outputs specifically seems like the best chance of glossing over interop issues. They are fine for internal dependencies etc etc, but just not for the outputted entrypoint module.

Similarly in the modules RollupJS emits. If we can get to a place where no __esModule CommonJS modules have a default export then that is a good place to be!

I do not want to say we should not align with Node as default, but we should at least warn the user whenever such a marker is encountered in the code that they may encounter bugs, and also tell them what they can do against this

Sensible warnings sounds like a great way to start to work through the user experience issues here before users hit the compatibility cases.

I would still like to push for the Node.js interop in due course if possible, and may even work on a PR at some point if you are not personally interested in this. Hopefully we can continue to iron out these cases in the mean time.

I brought up the same discussion to @evanw on esbuild today here - evanw/esbuild#532. Seems like compat discussions between bundlers will be quite important going forward.

[stale]

Hey folks. This issue hasn't received any traction for 60 days, so we're going to close this for housekeeping. If this is still an ongoing issue, please do consider contributing a Pull Request to resolve it. Further discussion is always welcome even with the issue closed. If anything actionable is posted in the comments, we'll consider reopening it. ⓘ

[nicolo-ribaudo]

I'm trying to add a new option to Babel which makes it reflect the Node.js behaviour: babel/babel#12838

Without that option, it's impossible to easily write ESM code which works both natively and when compiled: until that Babel option is merged, we have to rely on a custom plugin (babel/babel#12795).

I think this issue should be reopened, to consider a similar option for Rollup.

[guybedford]

Thanks for following up here @nicolo-ribaudo, reopening.

[nicolo-ribaudo]

Fyi, I opened #838 to continue this discussion with an actual implementation because we accidentally introduced a bug in Babel because of the difference between Node.js and Rollup.