This plugin (and Rollup in extension) work as seamlessly as possible with output generated by itself and tools in the ecosystem.
There are a whole lot of interop issues, see details below. This is meant as an overview over how this plugin actually works and a more or less complete picture of all interop situations and the current behaviour. The idea is to use this as a base for discussion and to identify sub-issues that can be solved with reasonable effort. I will gladly try to take care of all changes to Rollup core but am very happy if I get support improving the plugin.
This list is VERY long, and I will try to update it based on suggestions. I will also try to add a compiled summary at the end soon.
As I strongly believe in testing over believing, I added actual output code samples for almost everything. To generate the pre-formatted code samples, I used the following
Rollup config
// "rollup.config.js"
import path from "path";
import cjs from "@rollup/plugin-commonjs";
const inputFiles = Object.create(null);
const transformedFiles = Object.create(null);
const formatFiles = (files) =>
Object.keys(files)
.map((id) => `// ${JSON.stringify(id)}\n${files[id].code}\n`)
.join("\n");
const formatId = (id) => {
const [, prefix, modulePath] = /(\0)?(.*)/.exec(id);
return `${prefix || ""}${path.relative(".", modulePath)}`;
};
export default {
input: "main",
plugins: [
{
name: "collect-input-files",
transform(code, id) {
if (id[0] !== "\0") {
inputFiles[formatId(id)] = { code: code.trim() };
}
},
},
cjs(),
{
name: "collect-output",
transform(code, id) {
// Never display the helpers file
if (id !== "\0commonjsHelpers.js") {
transformedFiles[formatId(id)] = { code: code.trim() };
}
},
generateBundle(options, bundle) {
console.log(`<details>
<summary>Input</summary>
\`\`\`js
${formatFiles(inputFiles)}
\`\`\`
</details>
<details>
<summary>Transformed</summary>
\`\`\`js
${formatFiles(transformedFiles)}
\`\`\`
</details>
<details>
<summary>Output</summary>
\`\`\`js
${formatFiles(bundle)}
\`\`\`
</details>`);
},
},
],
output: {
format: "es",
file: "bundle.js",
},
};
The plugin needs to take care that this works seamlessly, resolving require statements with module.exports of the required module. This is not really an interop problem, the goal of this section is rather to highlight how the plugin actually works and handles certain scenarios and where it could be improved before moving on to the actual interop situations.
module.exportsIn the importing file, we generate two imports: An empty imports of the original module while the actual binding is imported from a proxy module. The reason for the empty import of the original file is to trigger loading and transforming the original file so that we know if it is CJS or ESM when building the proxy file.
The actual module renders two exports: What is assigned to module.exports is exported as both default and __moduleExports. The proxy again exports __moduleExports as default (for situations where the proxy does slightly different things, look into the section where ESM is imported from CJS).
Input
// "main.js"
const dep = require("./dep.js");
console.log(dep);
// "dep.js"
module.exports = "foo";
Transformed
// "main.js"
import "./dep.js";
import dep from "./dep.js?commonjs-proxy";
console.log(dep);
// "dep.js"
var dep = "foo";
export default dep;
export { dep as __moduleExports };
// "\u0000dep.js?commonjs-proxy"
import { __moduleExports } from "/Users/lukastaegert/Github/rollup-playground/dep.js";
export default __moduleExports;
Output
// "main.js"
var dep = "foo";
console.log(dep);
exports or module.exportsIn this scenario, Rollup creates an artificial module.exports object that is created with all properties inline. This is very efficient as opposed to assigning the properties to an object one by one as the runtime engine can immediately optimize such an object for quick access. This object is again then exported as both default and __moduleExports. Additionally, all assigned properties are also exported as named exports.
Input
// "main.js"
const dep = require("./dep.js");
console.log(dep);
// "dep.js"
module.exports.foo = "foo";
exports.bar = "bar";
exports.default = "baz";
Transformed
// "main.js"
import "./dep.js";
import dep from "./dep.js?commonjs-proxy";
console.log(dep);
// "dep.js"
var foo = "foo";
var bar = "bar";
var _default = "baz";
var dep = {
foo: foo,
bar: bar,
default: _default,
};
export default dep;
export { dep as __moduleExports };
export { foo };
export { bar };
// "\u0000dep.js?commonjs-proxy"
import { __moduleExports } from "/Users/lukastaegert/Github/rollup-playground/dep.js";
export default __moduleExports;
Output
// "bundle.js"
var foo = "foo";
var bar = "bar";
var _default = "baz";
var dep = {
foo: foo,
bar: bar,
default: _default,
};
console.log(dep);
π Bug 1: Assigning to the same property twice will generate two exports of the same name, causing Rollup to throw a "Duplicate export
module.exports or exportsThere are a lot of cases where the plugin deoptimizes, e.g. when a property is read instead of assigned. In such situations, createCommonjsModule helper is used to create a wrapper to execute the module more or less like Node would execute it without detecting any named exports.
Input
// "main.js"
const dep = require("./dep.js");
console.log(dep);
// "dep.js"
if (true) {
exports.foo = "foo";
}
Transformed
// "main.js"
import "./dep.js";
import dep from "./dep.js?commonjs-proxy";
console.log(dep);
// "dep.js"
import * as commonjsHelpers from "commonjsHelpers.js";
var dep = commonjsHelpers.createCommonjsModule(function (module, exports) {
if (true) {
exports.foo = "foo";
}
});
export default dep;
export { dep as __moduleExports };
// "\u0000dep.js?commonjs-proxy"
import { __moduleExports } from "/Users/lukastaegert/Github/rollup-playground/dep.js";
export default __moduleExports;
Output
// "bundle.js"
function createCommonjsModule(fn, basedir, module) {
return (
(module = {
path: basedir,
exports: {},
require: function (path, base) {
return commonjsRequire(
path,
base === undefined || base === null ? module.path : base
);
},
}),
fn(module, module.exports),
module.exports
);
}
function commonjsRequire() {
throw new Error(
"Dynamic requires are not currently supported by @rollup/plugin-commonjs"
);
}
var dep = createCommonjsModule(function (module, exports) {
{
exports.foo = "foo";
}
});
console.log(dep);
require callsAt the moment, the plugin is NOT capable of maintaining exact execution order. Rather, even nested and conditionally executed require statements (unless they are written via an if-statement in a particular way) are always hoisted to the top. This is a separate situation that could be improved by drastic changes, i.e. wrapping modules in function enclosures and calling them when they are first used. This will however have a negative effect on the efficiency of the generated code as compared to the status quo so this should only happen when it is really necessary. Unfortunately, it is not possible to tell if a module is required in a non-standard way until the whole module graph is built so in the worst case, all modules might need to be wrapped. Rollup could help here a little by implementing some inlining algorithm, however this is very much future talk (say, up to a year in the future) and will likely only apply anyway if a module is used exactly in one place. Other approaches could be for the plugin to analyze the actual execution order to see if it can ensure that the first usage does not need wrapping so that it does not matter if there are dynamic requires later on but this feels complicated and error-prone.
Anyway, this is mostly listed for completeness here as it does not really touch on the subject of interop but warrants its own issue. Here is a sample to illustrate:
Input
// "main.js"
console.log("first");
require("./dep.js");
console.log("third");
false && require("./broken-conditional.js");
// There is special logic to handle this exact case, which is why
// "working-conditional.js" is not in the module graph, but it is not easily
// generalized.
if (false) {
require("./working-conditional.js");
}
// "dep.js"
console.log("second");
// "broken-conditional.js"
console.log("not executed");
Transformed
// "main.js"
import "./dep.js";
import "./broken-conditional.js";
import "./dep.js?commonjs-proxy";
import require$$1 from "./broken-conditional.js?commonjs-proxy";
console.log("first");
console.log("third");
false && require$$1;
// There is special logic to handle this exact case, which is why
// "working-conditional.js" is not in the module graph, but it is not easily
// generalized.
if (false) {
require("./working-conditional.js");
}
// "dep.js"
console.log("second");
// "\u0000dep.js?commonjs-proxy"
import * as dep from "/Users/lukastaegert/Github/rollup-playground/dep.js";
export default dep;
// "broken-conditional.js"
console.log("not executed");
// "\u0000broken-conditional.js?commonjs-proxy"
import * as brokenConditional from "/Users/lukastaegert/Github/rollup-playground/broken-conditional.js";
export default brokenConditional;
Output
// "bundle.js"
console.log("second");
console.log("not executed");
console.log("first");
console.log("third");
Now let us get to the actual interop patterns:
In Node, CJS modules only expose a default export that corresponds to module.exports. This important pattern should always work.
For the plugin, the main difference here is that instead of the proxy, the actual module is imported directly. Here just one example to illustrate, in general everything works similar to the CJS-to-CJS case.
Input
// "main.js"
import foo from "./dep.js";
console.log(foo);
// "dep.js"
module.exports = "foo";
Transformed
// "main.js"
import foo from "./dep.js";
console.log(foo);
// "dep.js"
var dep = "foo";
export default dep;
export { dep as __moduleExports };
Output
// "bundle.js"
var dep = "foo";
console.log(dep);
This is supported by Webpack but (previously in part but now fully) also by this plugin. In addition to the default import resolving to module.exports, named imports will resolve to properties on module.exports. Previously, this would only work for named exports that the plugin could auto-detect (and only if the module was not deoptimized to use createCommonjsModule) or which were listed by the user. The use of Rollup's syntheticNamedExports property in its current form now enables arbitrary named imports to be resolved without additional configuration while even maintaining live-bindings.
Note that Webpack currently either disallows or warns about this pattern when used from modules with the .mjs extension.
π§ TODO: Would be nice to confirm this
The intention here is that this extension signals we want to enter some sort of strict NodeJS interop mode. We could do something similar, but then I would love to have the ESM/CJS detection in @rollup/plugin-node-resolve and establish a communication channel to get this information from that plugin. Then we might add a switch to @rollup/plugin-commonjs to use "strict NodeJS interop" that
This could become an advanced feature we add after solving the more pressing issues we have at the moment.
Input
// "main.js"
import { foo } from "./dep.js";
console.log(foo);
// "dep.js"
exports.foo = "foo";
Transformed
// "main.js"
import { foo } from "./dep.js";
console.log(foo);
// "dep.js"
var foo = "foo";
var dep = {
foo: foo,
};
export default dep;
export { dep as __moduleExports };
export { foo };
Output
// "bundle.js"
var foo = "foo";
console.log(foo);
Here we just assign an object to module.exports. Note how we retain live-bindings by using property accesses: If the object at module.exports would be mutated later on, accessing our named variable would always provide the current value.
Input
// "main.js"
import { foo } from "./dep.js";
console.log(foo);
// "dep.js"
module.exports = { foo: "foo" };
Transformed
// "main.js"
import { foo } from "./dep.js";
console.log(foo);
// "dep.js"
var dep = { foo: "foo" };
export default dep;
export { dep as __moduleExports };
Output
// "bundle.js"
var dep = { foo: "foo" };
console.log(dep.foo);
This is the tricky one. The problem here is to maintain isomorphic behaviour between the original ES module and the CJS module. Named exports are handled mostly correctly when using the "NodeJS with named imports" pattern (except we do not throw for missing exports) however the default export should not be module.exports but module.exports.default. This is incompatible with the previously listed interop patterns.
At the moment most tools implement a runtime detection pattern for this by adding an __esModule property to module.exports to signify this is a transpiled ES module. Then the algorithm when getting the default import is
module.exports.default as the default exportmodule.exportsπ Bug 2: This is not working correctly as instead of the named export, it tries to return a property on the default export. The reason is that in this situation, the interop pattern in unwrapExports kind of correctly extracts the default export and exports it as default, but syntheticNamedExports should not use that to extract named exports.
Input
// "main.js"
import { foo } from "./dep.js";
console.log(foo);
// "dep.js"
Object.defineProperty(exports, "__esModule", { value: true });
exports.foo = "foo";
exports.default = "default";
Transformed
// "main.js"
import { foo } from "./dep.js";
console.log(foo);
// "dep.js"
import * as commonjsHelpers from "commonjsHelpers.js";
var dep = commonjsHelpers.createCommonjsModule(function (module, exports) {
Object.defineProperty(exports, "__esModule", { value: true });
exports.foo = "foo";
exports.default = "default";
});
export default /*@__PURE__*/ commonjsHelpers.unwrapExports(dep);
export { dep as __moduleExports };
Output
// "bundle.js"
function unwrapExports(x) {
return x && x.__esModule && Object.prototype.hasOwnProperty.call(x, "default")
? x["default"]
: x;
}
function createCommonjsModule(fn, basedir, module) {
return (
(module = {
path: basedir,
exports: {},
require: function (path, base) {
return commonjsRequire(
path,
base === undefined || base === null ? module.path : base
);
},
}),
fn(module, module.exports),
module.exports
);
}
function commonjsRequire() {
throw new Error(
"Dynamic requires are not currently supported by @rollup/plugin-commonjs"
);
}
var dep = createCommonjsModule(function (module, exports) {
Object.defineProperty(exports, "__esModule", { value: true });
exports.foo = "foo";
exports.default = "default";
});
var dep$1 = /*@__PURE__*/ unwrapExports(dep);
console.log(dep$1.foo);
This is quite difficult to fix, especially if we want to maintain live bindings for named exports. My first thought was to extend syntheticNamedExports to allow an additional value to indicate that the default export is also picked as default property from the actual default export. This would mean however that auto-detecting interop becomes slow and difficult and destroys likely all live-bindings for the non ESM case because we need to build a new object, i.e.
export default moduleExports && moduleExports.__esModule
? moduleExports
: Object.assign({}, moduleExports, { default: moduleExports });
π Improvement 1: A better idea I had is to allow specifying an arbitrary string as value of syntheticNamedExports, e.g. syntheticNamedExports: "__moduleExports". The meaning would be that missing named (and even the default) exports are no taken from the default export but from a named export of the given name. Then the interop would just be
export { __moduleExports };
export default __moduleExports.__esModule
? __moduleExports.default
: __moduleExports;
This is rather efficient, though it could still be put into an interop function getDefault if we want to save a few bytes. Of course we still do not get live-bindings for the default export in the transpiled ESM case, but even this is fixable if in a second step, we implement static detection for __esModule:
π Improvement 2: If we come across the Object.defineProperty(exports, "__esModule", { value: true }) line (or !0 instead of true for the minified case) on the top level of a module, then we can just mark this module as being transpiled and can even get rid of this line in the transformer, making the code more efficient and removing the need for any interop, i.e. above we do not add the export default at all in that case. There is also no longer a need to wrap the code in createCommonjsModule if this property definition is ignored.
π Bug 3: It is not really surprising that this case is not working correctly as it should actually throw either at build or at runtime. Otherwise at the very least the default export should be undefined while here it is actually the namespace.
Input
// "main.js"
import foo from "./dep.js";
console.log(foo);
// "dep.js"
Object.defineProperty(exports, "__esModule", { value: true });
exports.foo = "foo";
Transformed
// "main.js"
import foo from "./dep.js";
console.log(foo);
// "dep.js"
import * as commonjsHelpers from "commonjsHelpers.js";
var dep = commonjsHelpers.createCommonjsModule(function (module, exports) {
Object.defineProperty(exports, "__esModule", { value: true });
exports.foo = "foo";
});
export default /*@__PURE__*/ commonjsHelpers.unwrapExports(dep);
export { dep as __moduleExports };
Output
// "bundle.js"
function unwrapExports(x) {
return x && x.__esModule && Object.prototype.hasOwnProperty.call(x, "default")
? x["default"]
: x;
}
function createCommonjsModule(fn, basedir, module) {
return (
(module = {
path: basedir,
exports: {},
require: function (path, base) {
return commonjsRequire(
path,
base === undefined || base === null ? module.path : base
);
},
}),
fn(module, module.exports),
module.exports
);
}
function commonjsRequire() {
throw new Error(
"Dynamic requires are not currently supported by @rollup/plugin-commonjs"
);
}
var dep = createCommonjsModule(function (module, exports) {
Object.defineProperty(exports, "__esModule", { value: true });
exports.foo = "foo";
});
var foo = /*@__PURE__*/ unwrapExports(dep);
console.log(foo);
To fix this, I would like to reduce the interop pattern to only look for the presence of __esModule and nothing else. This would be covered by the suggested Improvement 2.
To my knowledge, there is no engine that supports this at the moment, so the question is what the "correct" return value should be. For NodeJS, I heard that there is actually some interest in supporting this eventually but the road blocks are technical here (mostly that the ESM loader is async + TLA handling and similar things). However if this is ever supported, a require of an ES module will likely return the namespace. Otherwise Webpack supports this of course, and to my knowledge here you always get the namespace as well. Looking at TypeScript it will always add the __esModule property on transpiled ESM modules and use the default as a property which in turn means that if you require an ES module and use CJS as output target, you always get the namespace. The same goes for Babel.
For Rollup, it is more complicated, and the reason is in part rooted in that fact that Rollup was first and foremost designed for libraries, and here you want to be able to create CJS output where everything, e.g. a function, is directly assigned to module.exports to be able to create a nice interface for "one-trick-pony" libraries. So Rollup core by default uses its "auto" mode when generating CJS output, which means
module.exports contains the namespace _unless_module.exportsNow of course this is about converting ESM to CJS but one idea is that in such a situation, the CJS and ESM versions should be interchangeable when I require them. So for internal imports, it could make sense to work like Rollup's auto mode in reverse, giving you the default export when there are no named exports and the namespace otherwise. But I understand that in the long term, we should likely align with the rest of the tooling world even if it generates less efficient code, so my suggestion on this front is:
require by defaultexternal option in Rollup works) to work like auto mode as outlined above to make existing mixed ESM CJS code-bases work. The reason I think we need this is that the requiring module can easily be a third party dependency itself and thus not under your direct control.This works as intended.
Input
// "main.js"
const foo = require("./dep.js");
console.log(foo);
// "dep.js"
export const foo = "foo";
Transformed
// "main.js"
import "./dep.js";
import foo from "./dep.js?commonjs-proxy";
console.log(foo);
// "dep.js"
export const foo = "foo";
// "\u0000dep.js?commonjs-proxy"
import * as dep from "/Users/lukastaegert/Github/rollup-playground/dep.js";
export default dep;
Output
// "bundle.js"
const foo = "foo";
var dep = /*#__PURE__*/ Object.freeze({
__proto__: null,
foo: foo,
});
console.log(dep);
This one is working correctly for auto mode but with the arguments above, it should likely be changed, see Improvement 3 and Improvement 4.
Input
// "main.js"
const foo = require("./dep.js");
console.log(foo);
// "dep.js"
export default "default";
Transformed
// "main.js"
import "./dep.js";
import foo from "./dep.js?commonjs-proxy";
console.log(foo);
// "dep.js"
export default "default";
// "\u0000dep.js?commonjs-proxy"
export { default } from "/Users/lukastaegert/Github/rollup-playground/dep.js";
Output
// "bundle.js"
var foo = "default";
console.log(foo);
π Bug 4: This one is brokenβit should definitely return the namespace here but instead it returns the default export.
Input
// "main.js"
const foo = require("./dep.js");
console.log(foo);
// "dep.js"
export const foo = "foo";
export default "default";
Transformed
// "main.js"
import "./dep.js";
import foo from "./dep.js?commonjs-proxy";
console.log(foo);
// "dep.js"
export const foo = "foo";
export default "default";
// "\u0000dep.js?commonjs-proxy"
export { default } from "/Users/lukastaegert/Github/rollup-playground/dep.js";
Output
// "bundle.js"
var foo = "default";
console.log(foo);
To view the full interop story here, one has to look both at what this plugin generates and what Rollup generates as CJS output.
π Bug 5: For external imports, this plugin will always require the default import.
π Improvement 6: With the arguments given above, this should actually be the namespace by default (import * as external from 'external') as this is technically equivalent to requiring an ES module.
π Improvement 4b: Again we should add an option to specify when an external require should just return the default export instead. It could even be the same option. So here is some room for discussion.
Input
// "main.js"
const foo = require("external");
console.log(foo);
Transformed
// "main.js"
import "external";
import foo from "external?commonjs-proxy";
console.log(foo);
// "\u0000external?commonjs-external"
import external from "external";
export default external;
Output
// "bundle.js"
import external from "external";
console.log(external);
Ideally, this should be converted to a simple require statement as
require would again become a simple require.And this is indeed the case:
Input
// "main.js"
import * as foo from "external";
console.log(foo);
Output
// "bundle.js"
"use strict";
var foo = require("external");
console.log(foo);
These should just be converted to properties on whatever require returns as that should be equivalent to a namespace. And that is again the case:
Input
// "main.js"
import { foo, bar } from "external";
console.log(foo, bar);
Output
// "bundle.js"
"use strict";
var external = require("external");
console.log(external.foo, external.bar);
π Bug 6: Several things are broken here:
default property instead of checking the __esModule property. I always thought there was a reason buried in some old issue but going back in history it seems it has always been that way. This should change as it has all sorts of adverse effects: If the file was not an ES module but assigns something to exports.default, it will be mistaken for a default export; if it was an ES module but does not have a default export, this will wrongly return the namespace instead.
Input
// "main.js"
import foo from "external";
console.log(foo);
Output
// "bundle.js"
"use strict";
function _interopDefault(ex) {
return ex && typeof ex === "object" && "default" in ex ? ex["default"] : ex;
}
var foo = _interopDefault(require("external"));
console.log(foo);
π Improvement 7: To fix this, I think I would try to go for how Babel does it.
module.exportsThis will be turned into a default export which is probably the best we can hope for.
Input
// "main.js"
module.exports = "foo";
Transformed
// "main.js"
var main = "foo";
export default main;
Output
// "bundle.js"
var main = "foo";
export default main;
exportsThis appears to be working sensibly.
Input
// "main.js"
module.exports.foo = "foo";
Transformed
// "main.js"
var foo = "foo";
var main = {
foo: foo,
};
export default main;
export { foo };
Output
// "bundle.js"
var foo = "foo";
var main = {
foo: foo,
};
export default main;
export { foo };
module.exports but requires itselfπ Bug 7: Basically it looks for a __moduleExports export that does not exist instead of giving the namespace. My suggestion above for how to rework syntheticNamedExports in Improvement 1 should also fix this from within Rollup. Similar problems arise when another module imports our entry point or when the module just adds properties to exports.
Input
// "main.js"
const x = require("./main.js");
console.log(x);
module.exports = "foo";
Transformed
// "main.js"
import "./main.js";
import x from "./main.js?commonjs-proxy";
console.log(x);
var main = "foo";
export default main;
// "\u0000main.js?commonjs-proxy"
import { __moduleExports } from "/Users/lukastaegert/Github/rollup-playground/main.js";
export default __moduleExports;
Output
// "bundle.js"
console.log(main.__moduleExports);
var main = "foo";
export default main;
exports.defaultπ Bug 8: Here no output is generated while it should likely default export the namespace unless the __esModule property is present.
Input
// "main.js"
module.exports.default = "foo";
Transformed
// "main.js"
var _default = "foo";
var main = {
default: _default,
};
Output
// "bundle.js"
Ideally, the output should have the same exports as the original entry. At the moment, this will not be the case as the Object.defineProperty call will always cause the createCommonjsModule wrapper to be used and no named exports will be detected. There are several ways to improve this:
__esModule property definition and do not treat it as a reason for deoptimization, see Improvement 2namedExports that specifically lists exposed exports for entries. We could also use this to activate or deactive the default export and decide if the default export should be whatever is assigned to exports.default or rather module.exports. This could be handled by simply creating a wrapper file that reexports these exports. If we do it that way, Rollup tree-shaking might even remove some unused export code.
Input
// "main.js"
Object.defineProperty(exports, "__esModule", { value: true });
exports.default = "foo";
exports.bar = "bar";
Transformed
// "main.js"
import * as commonjsHelpers from "commonjsHelpers.js";
var main = commonjsHelpers.createCommonjsModule(function (module, exports) {
Object.defineProperty(exports, "__esModule", { value: true });
exports.default = "foo";
exports.bar = "bar";
});
export default /*@__PURE__*/ commonjsHelpers.unwrapExports(main);
Output
// "bundle.js"
function unwrapExports(x) {
return x && x.__esModule && Object.prototype.hasOwnProperty.call(x, "default")
? x["default"]
: x;
}
function createCommonjsModule(fn, basedir, module) {
return (
(module = {
path: basedir,
exports: {},
require: function (path, base) {
return commonjsRequire(
path,
base === undefined || base === null ? module.path : base
);
},
}),
fn(module, module.exports),
module.exports
);
}
function commonjsRequire() {
throw new Error(
"Dynamic requires are not currently supported by @rollup/plugin-commonjs"
);
}
var main = createCommonjsModule(function (module, exports) {
Object.defineProperty(exports, "__esModule", { value: true });
exports.default = "foo";
exports.bar = "bar";
});
var main$1 = /*@__PURE__*/ unwrapExports(main);
export default main$1;
I will gladly take care of the necessary improvements here:
auto mode without specifying it explicitly and there is a package that has only a default export. Explain how this package will not be interchangeable with its ESM version in many tools and suggest to use named exports mode with all its consequences or better, do not use default exports.syntheticNamedExports should support receiving a string value that corresponds to an export name from which to pick missing exports. The value of true would correspond to "default" except that for entry points when using a string, the listed property name would not be part of the public interface. This will fix issues where suddenly __moduleExports is introduced into an interface.Implement Improvement 3, Improvement 4a, Improvement 4b and Improvement 6: Always return the namespace when requiring an ES module except when a particular option is set. What is returned is configured in the proxy module. At the moment, we only return the namespace if the module does not use a default export. There are many ways how such an option might be done, here is one suggestion:
requireReturnsDefaultsupported values:
false (the default): All ES modules return their namespace when required. External imports are rendered as import * as external from 'external' without any interop.true: All ES modules that have a default export should return their default when required. Only if there is no default export, the namespace is used. This would be like the current behaviour. For external imports, the following interop pattern is used:import * as external_namespace from 'external';
var external = 'default' in external_namespace ? external_namespace.default : external_namespace;
It might make sense to extract this into a helper function that is reused where possible.
"auto": All modules return their namespace if required unless they have only a default export, in which case that one is returned. This is the inverse of Rollup's auto mode. For external imports, the following interop pattern is used:import * as external_namespace from 'external';
var external = 'default' in external_namespace && Object.keys(external_namespace).length === 1 ? external_namespace.default : external_namespace;
Again it might make sense to extract this into a helper function.
this.resolve so that the user can just use the name of a node_module. In that case if the module is not an ES module or does not have a default export, the plugin should throw, explaining the error. External imports are just rendered as import external from 'external' without any interop.true or false|undefined. It should be ensured that this function is only called once per resolved module id. This is equivalent to specifying an array of all module ids for which we returned true. βΊ #507 β
syntheticNamedExports and add the default export corresponding to the suggested simplified interop pattern.Implement Improvement 2: If a property definition for __esModule is encountered, remove it, do not treat it as a cause for using the createCommonjsModule wrapper and do not add the interop default export to the module. Ideally, in this scenario, assignments to exports.default should be treated like assignments to exports.foo in that it generates an explicit export. So this:
Object.defineProperty(exports, "__esModule", { value: true });
exports.default = 'default';
exports.foo = 'foo';
should be converted to
// Note that we need to escape the variable name as `default` is not allowed
var _default = 'default';
var foo = 'foo';
var dep = {
default: _default,
foo: foo
};
export { dep as __moduleExports };
// This is a different way to generate a default export that even allows for live-bindings
export { foo, _default as default};
βΊ #537 βοΈ
Implement Improvement 8: Allow specifying exposed exports. My suggestion is:
exposedExportsvalue: An object where keys correspond to module ids. For convenience, the keys should be put through this.resolve by the plugin. The value is an array of named exports. Internally, this could just mean we resolve the module not to itself but rather to a reexport file, e.g.
// for exposedExports: {'my-module': ['foo', 'default']}
export { foo, default } from ' \0my-module?commonjsEntry'
The new ' \0my-module?commonjsEntry' would correspond to that we usually render here. Commonjs proxies should also import from that file.
I'm blown away by how thorough this is. Thank you for taking the time to outline!
I don't have much to add here, other than to just add some color / advocate for "Importing ESM from CJS" as something that should be considered supported (above, you mention "if we support this"). This is common in applications and anything bundling 3rd part code, where an ESM package depends on a CJS package. Its been a problem in Snowpack before that most users coming from Webpack don't expect.
Thanks, I changed the wording. I also explicitly numbered all improvement suggestions now and added a summary section that contains an "action plan" and some suggested order in which those could be tackled. I will gladly do the improvements in Rollup core but would be more than happy for support in improving the plugin. Would be especially nice to have @danielgindi on board here, but other contributors are heartily welcome as well.
Just had a very brief read, Lukas this is great work and looks like it will be a great alignment. //cc @sokra for visibility.
Both of my main points I've mentioned previously but will state them again.
The first concern I still have here is that Node.js does not implement the __esModule interop pattern for ESM importing CommonJS. So when importing a CommonJS module that exports __esModule the full module.exports value is provided as the default export only.
So RollupJS working differently for this interop may cause breaks between Node.js semantics and RollupJS semantics.
In addition the edge case for default here is (let's call this the "esmodule default override" case):
Object.defineProperty(exports, '__esModule', true)
exports.default = 'asdf'
where import foo from 'dep' gets 'asdf' under es module interop and the full exports value in Node.js.
Node.js does have a PR for named exports that would add __esModule interop just for the esmodule default override case above, the current consensus was to release this behind a flag initially for testing soon, then if there is greater user interest it could be unflagged. Basing that unflagging decision on user feedback and real package support is important though as it uses a heuristical analysis approach.
The second point is more of a suggestion I've mentioned before that it could be useful to have 'auto' only work for the entry points, since that is a common use case for this feature. Perhaps 'autoentry' or something. This is additive work certainly but may smooth the transition.
The first concern I still have here is that Node.js does not implement the __esModule interop pattern for ESM importing CommonJS
This is true and definitely a concern. Adding a "strictNodeSemantics" mode that does not implement this interop or allow named exports would be a prudent step forward, especially if it also bases ESM/CJS detection solely on Node semantics. This would be a larger undertaking, however, as it should ideally be based on ' @rollup/plugin-node-resolve` providing access to package files, so I would put it down for "lager enhancements".
But as I am sure you are aware, this interop pattern is implemented by most other major tools in the ecosystem (I am aware Babel, TypeScript, Webpack) so I think we should support it out of the box. Nice to hear there are considerations to port it to Node as this will finally bring alignment across the whole ecosystem.
The second point is more of a suggestion I've mentioned before that it could be useful to have 'auto' only work for the entry points
Are you referring to Rollup's output.exports: 'auto'? Of course this only applies to entry points, but I am not sure why this should matter? Other files are used only internally anyways and usually only have minified exports anyway.
Or are you referring to my suggestion for requireReturnsDefault? I do not see why entry points are that much different, also I wonder how likely it is that a package will require its own ESM entry from a CJS file when both are created by the same user together. The mostly likely candidates where you need it is older packages in your node_modules. But detecting this is beyond the scope of this plugin. Also, it is not possible to determine 100% at build time if a module is an entry because modules can be promoted to entry points by plugins any time until the graph is built. But of course, extensions are always possible if we they help at least some people and we can figure out how to implement them in a reasonable way.
Nice to hear there are considerations to port it to Node as this will finally bring alignment across the whole ecosystem.
The PR for named exports support would not bring __esModule interop to any case other than the case where a CommonJS module exports both a default export and a __esModule export. But when named exports are analyzed and gathered onto the module namespace like { default: module.exports, ...namedExportsDetected } one ends up with a pattern that then makes the __esModule cases work since the detected plucked exports (even though not live bindings), help the majority of __esModule cases although of course there can still be edge cases.
Yes interop is an art not a science though. That said, users will likely simply shy away from the messier parts naturally.
Are you referring to Rollup's output.exports: 'auto'? Of course this only applies to entry points, but I am not sure why this should matter? Other files are used only internally anyways and usually only have minified exports anyway.
Ah yes it sounds like I'm confusing that with your requireReturnsDefault 'auto' proposal. In that case this proposal sounds fine to me!
Taking my hat off. This is an intensively thorough work!
I'm putting this on my desktop, to re-read and start to discuss here in the following days. Having a pretty busy weekend so will probably start on Sunday.
The PR for named exports support would not bring __esModule interop to any case other than the case where a CommonJS module exports both a default export and a __esModule export. But when named exports are analyzed and gathered onto the module namespace like
{ default: module.exports, ...namedExportsDetected }one ends up with a pattern that then makes the __esModule cases work since the detected plucked exports (even though not live bindings), help the majority of __esModule cases although of course there can still be edge cases.
@guybedford Even with this change the following would happen:
// module.cjs
exports.__esModule = true;
exports.default = 42;
import x from "./module.cjs";
console.log(x);
// Node.js: { default: 42 }
// Everybody else: 42
I get why you are doing that (To not introduce an breaking change), but if I would be in your place I would argue that this is a bug and should be fixed even while it's a breaking change. ESM in Node.js is still experimental so nobody can complain about that...
I created a draft PR for extending syntheticNamedExports to support an arbitrary export as base in rollup/rollup#3657. Core functionality is working but some edge cases will still be refined.
Update: The PR is now ready for review and includes the necessary documentation.
@danielgindi
I'm putting this on my desktop, to re-read and start to discuss here in the following days
No problems. I will start working on the Rollup core side of things for now anyway, and will not merge PRs too soon.
I also created a PR for the new warning in rollup/rollup#3659
I'm very interested in some the cases which prevent deoptimization. As you know I'm reusing rollup plugins in es-dev-server (and in a new generation of tools at https://github.com/modernweb-dev/web). I'm only using the single file transform API, so I'm only able to rely on the generated named exports. Right now, any occurrence of __esModule generates only a default export.
Do we have a maintainer who is comfortable with the commonjs plugin to implement these changes? It's a pretty daunting piece of code, but I could try and dig into it.
Besides myself, @danielgindi is the one who did most work on the plugin recently, but I think he is a little busy elsewhere at the moment. I think any help here is welcome.
Right now, any occurrence of __esModule generates only a default export.
I noticed it as well and this is very disappointing. I think a goal should be that most ES modules converted to CJS by Rollup should be possible to convert back to ESM by this plugin, and this particular issue prevents that effectively.
Myself, I am still working on the third part of the Rollup core changes, which unfortunately turns out to be a little bigger than anticipated and might take a few more days. Once done, I would switch over to this plugin but I would like to complete improvement 1 first to validate if the changes in Rollup make sense. Then I would be available for other improvements.
If you would be willing to dig into improvement 2, that would be tremendous and should not cause conflicts with other work. If you need help, you can open a draft PR and we can continue discussion there.
@lukastaegert
I'm back ;-) Kind of... We are almost finished moving to a new apartment, and this is tedious. Yes, more than the commonjs plugin!
About bug 3: I'm not sure it's a bug. Of course in terms of ES6 imports, if there's no default exports, you can't import it. But if we've already decided that CJS' exports object is exported as default, then there will always be a default export as in CJS the module.exports is always initialized with {}.
Bug 4: Again, you said "definitely" but I'm not so sure about it :-) As if it was the other way around, exporting from CJS, you would treat the main exports object as the default export. So taking the default export as the default result from require looks natural.
Anyway, if anyone started working on this please share the branch so I could take a look. If not, I may pick up the gauntlet.
Hi, no worries, I can feel this. Hope the move is worth it!
With regard to your comments, Bug 3 is about being able to replace the CJS file with the ES6 file. You are right that it may not be considered a bug, but it also matches what other tools do, e.g. Babel: https://babeljs.io/repl/#?browsers=defaults%0A&build=&builtIns=false&spec=false&loose=false&code_lz=JYWwDg9gTgLgBAMwhRUIjgcgEYEMqYDcAUEA&debug=false&forceAllTransforms=false&shippedProposals=false&circleciRepo=&evaluate=false&fileSize=false&timeTravel=false&sourceType=module&lineWrap=true&presets=env%2Creact%2Cstage-2%2Cenv&prettier=false&targets=&version=7.10.4&externalPlugins=
The point is, if the default export existed but was false, null or undefined, it should still be returned as the default export and that is not working correctly at the moment.
I somewhat agree with your argument for Bug 4 but here, Rollup definitely stands alone, and the behaviour here is not even compatible with Rollup's default behaviour where having a default and named exports, Rollup core exports the namespace. In any case, all other tools (Babel, TypeScript, Webpack) expect that requiring an es module exports the namespace, and there is a chance that Node will also go this way if they should ever support it, so, albeit slightly grudgingly, I think it is the way to move forward.
Personally, I only worked on the Rollup core parts until now, but once this is done, I would like to start completing improvement 1 here to see if my changes in core work out before I release them. I will create a branch with a draft PR once I start.
I think we need to clarify migration path to commonjs 14 for such packages.
https://unpkg.com/browse/[email protected]/lib/index.js
That changelog entry was not obvious for me as support for __esModule was dropped, not added.
https://github.com/rollup/plugins/blob/master/packages/commonjs/CHANGELOG.md#v1301
If there is no migration path what was the point in releasing the change? Currently my test environment is broken and I don't know should I downgrade or add hacks with manual .default resolve (though I can't do this for my dependencies).
This was a stop-gap fix for a bigger error in favour of a smaller one. I would say the current path would be to speed up on releasing version 15 (yes, numbers on this plugin keep spinning), I have a pending fix for this situation, but it requires an update on Rollup that I need to release. I will link to the corresponding PRs once ready.
This was a stop-gap fix for a bigger error in favour of a smaller one. I would say the current path would be to speed up on releasing version 15 (yes, numbers on this plugin keep spinning), I have a pending fix for this situation, but it requires an update on Rollup that I need to release. I will link to the corresponding PRs once ready.
We will soon reach Chrome's versioning scheme π
I got here from the new warning:
Entry module "src\plugin.js" is implicitly using "default" export mode, which means for CommonJS output that its default export is assigned to "module.exports". For many tools, such CommonJS output will not be interchangeable with the original ES module. If this is intended, explicitly set "output.exports" to either "auto" or "default", otherwise you might want to consider changing the signature of "src\plugin.js" to use named exports only.
introduced in https://github.com/rollup/rollup/pull/3659. I revised all the relevant documentation and I couldn't understand what possible problems arise when there is only a default export used (i.e. no mixed default and named exports). The cjs format produces something like:
module.exports = /* the_default_export */;
The es format gives:
export default /* the_default_export */;
I get that mixed named and default exports require some special marker export symbol to work (https://stackoverflow.com/questions/40294870/module-exports-vs-export-default-in-node-js-and-es6) and that can be a problem.
A bit of background: my use-case is that I export a single function from a plugin library.
Is there something I missed? This is the quote that confuses me from the documentation:
In other words for those tools, you cannot create a package interface where const lib = require("your-lib") yields the same as import lib from "your-lib"
For my own use-case (single default export), I can't see how this can be the case.
Unfortunately this is exactly the situation this is warning about.
Assume that the ESM version of your library looks like this
// lib
export default 42;
Further assume someone bundles your library in a Webpack build where it is referenced from CommonJS via require like this
const lib = require("lib");
console log(lib);
then it will log
{default:42}
which is definitely not the same as when someone requires a CommonJS Version in Node that just assigns 42 to module.exports.
Hence the warning, and Webpack is not likely to change their stance here. Moreover, TypeScript and Babel decided to agree in so far as their output when generating CJS from ESM always corresponds to Rollupβs named export mode.
So you can easily disable the warning by explicitly specifying output.exports as auto, but you have been hereby informed about this problem.
Here is my lib:
export default function install(/* args */) { /* body */ }
When I require it like this:
const lib = require('vue-unique-id');
console.log(lib);
I get:
[Function: install]
(so not { default: Function }). In another project, I include it as an esm module through Webpack (import UniqueId from 'vue-unique-id';) and get the default exported function there as well. That's why I'm saying that my experience doesn't match the warning.
Is there something different from the example case you gave? By the way, I agree that what you show as a result is indeed not compatible between cjs and es modules. I just don't get how to get there unfortunately and I'm trying to understand so my library users don't get a bad user experience.
You have to run it through Webpack and your lib needs to use "main" and "module" in its package.json which is how you provide an ESM entry point at the moment. Here is an example demonstrating it. It will simulate such a lib and run first the original entry and then the bundled file through Node. https://repl.it/@lukastaegert/PoliteSuspiciousConversion
Note also that if Node will ever support requiring an ESM file from a CJS file, they will also very likely return a namespace.
Also in Webpack, importing is totally fine because this is not touching the generated CommonJS. It is using require that is broken. Maybe I should rework the warning message to make this clear: It is the case where a require is fulfilled with an ES module that will be broken, and these days, this case is most prominently present in bundlers.
Thanks for the explanation. I had forgotten about that module entry. That means that in my case (with both "main" and "module" in package.json) it's fine to use output.exports: "default".
For what it's worth, I understood the warning as applicable to a CommonJS consumer. I just didn't consider that I wasn't serving the same thing to CommonJS and ESM consumers. I think it's unfortunately impossible to make everything crystal clear in this situation because unless a user is intimately familiar with all this, there will likely be some confusion. Maybe there is a better wording, but I don't know what it would be.
Thanks for this great summary of all those kinds of interop cases.
Here, I run into a case about importing a namespace from a external commonjs but with __esModule true. Code as follow:
// index.js
import * as allIcons from './icons';
console.log(Object.keys(allIcons));
// icons.js
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
exports.foo = "foo";
exports.bar = "bar";
logout "foo", "bar", "default". Actually, only "foo", "bar" are expected...
output code with exports: 'auto':
var icons = createCommonjsModule(function (module, exports) {
Object.defineProperty(exports, "__esModule", { value: true });
exports.foo = "foo";
exports.bar = "bar";
});
var icons$1 = /*@__PURE__*/getDefaultExportFromCjs(icons);
var allIcons = /*#__PURE__*/Object.freeze(/*#__PURE__*/Object.assign(/*#__PURE__*/Object.create(null), icons, {
'default': icons$1
}));
semantically, the icons.js only exports "foo", "bar", but the default is added when transforming the code.
I am kind of not sure about whether default is mandatary included when importing a namespace from an external package(If so, it might be considered as bug?), or it's just an intentional design according to the cases here?
Yes, that is and will be with the coming updates an unfortunate side-effect of interop auto-detection. So you see, normal CJS modules always have an implicit default export. This is e.g. for compatibility with how NodeJS allows ESM to import only a default export from CJS. The __esModule flag should signal that this module is actually a transpiled ES module. However at the moment, this flag is not detected during compile time but only at runtime. At that point, we cannot remove the default export if it is not need because exports in ESM are static.
There is however an idea to scan for the __esModule definition during compile time to "fix" this. May take a while, though.
EDIT: Please disregard this message.
No, this issue is solely about handling imports and exports between different module formats. I would prefer to keep it separate.
Just updated the description with all relevant PRs. On Rollup side, all improvements are done and released as of v2.24.0. On plugin side, there is an open PR for improvement 2, otherwise only improvement 8 is missing. Everything else is already released.
Amazing. Lukas, thanks for taking such a thoughtful & thorough approach to lead this effort. This will be a huge improvement for all Snowpack users (and the entire ecosystem). Looking forward to integrating next week and will report back on how it goes!
Thanks, and really looking forward to your feedback. So far, only two bugs have popped up, which I think is ok considering the scale of some of the changes, and both of them are fixed now. If you find anything broken, please report so that I can have a look.
Nice! Will do. Have those fixes gone out in the latest releases? If not, can you share links to track the issues/fixes?
@lukastaegert Seems like now I won't have to submit so many interop patches, saved me some work ;-)
@lukastaegert I just want to say you've handled all the "personal" rant and grumbling of people much better than I would, and ended up with this much appreciated interop overhaul.
Thanks for the kind words. While it is not easy, it also goes to show the importance that this project has by now. Interop issues were a recurring theme, and what motivated me to finally try to straighten this through is not in a small part to get rid of these issues for good. And it feels better. Another things is that I see especially the commonjs plugin emerging as a go-to tool for converting CommonJS to ESM and I like to support this trend as it supports the Rollup ecosystem as a whole.
I'm not sure if this is the right place to ask this, but I'm running into a strange issue, might be similar to https://github.com/rollup/plugins/issues/481#issuecomment-662944889. I have a React library that I have created that consumes material-ui/icons. Since the changes from rollup/commonjs 13+ I keep running into failures consuming the library in a React/Webpack based client app when these icons are referenced.
The main differences I see in the output cjs bundle are:




In terms of the code the icons are being used as the following:
import ChevronRight from '@material-ui/icons/ChevronRight';
...
<Box className={classes.arrowContainer}>
<ChevronRight className={classes.arrow} />
</Box>
After updating the plugin I get the following errors in my consuming project:
Warning: React.createElement: type is invalid -- expected a string (for built-in components) or a class/function (for composite components) but got: object. and the page fails to render.
Seems something specific to importing it as a named export from the specific path, if I switch to importing it as
import { ChevronRight } from '@material-ui/icons';
but then I have some tree-shaking issues and the dev builds are much slower in the consuming project. I can work on resolving this, but just find this curious since it worked with version 12.
Any ideas what the problem here might be? I don't full understand the interop changes that were made and if this is a side-effect of that.
Thanks!
Make sure you are using both the latest version of this plugin as well as the latest version of Rollup. Beyond that, you could try creating a separate issue with a minimal reproduction as it is hard to tell from here what the issue is.
So as it turns out, there is one important case I overlooked when aggregating the interop scenarios, and this might actually be the scenario that @pachuka was looking at. Assume you have a CommonJS file that actually used to be an ES module by means of transpilation, and further assume this file imports the default from a module that was external to that module at transpilation time. Such a file could look like this:
// main.js
var lib = require('./lib.js');
function _interopDefault(e) {
return e && e.__esModule ? e : { default: e };
}
var lib__default = /*#__PURE__*/_interopDefault(lib);
console.log(lib__default['default']);
Now assume we are bundling this file together with an actual dependency lib.js that IS an ES module at bundling time. Let's assume this dependency looks like this:
// lib.js
export default 'foo';
export const bar = 'baz';
Now when bundling this together, the require('./lib.js') will be resolved with the namespace of lib.js. And that namespace does not contain an __esModule property! So what will be logged is not foo, it is { default: 'foo', bar: 'baz' }, i.e. the namespace object.
So to make this work, we need to inject the __esModule property, which is non-trivial as the namespace object is frozen (and when preserveModules is involved, it could even be an actual namespace). Moreover, we would not want to lose live-bindings here. So my solution would be to use a new helper in such a scenario that copies the namespace and adds the new property. I will try to set up a PR.
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. β
Reopening as there are some final points to be resolved
Hmm
@sokra has shared the rollup specific interop results from his comparison here: https://github.com/rollup/plugins/issues/654
Most helpful comment
I'm blown away by how thorough this is. Thank you for taking the time to outline!
I don't have much to add here, other than to just add some color / advocate for "Importing ESM from CJS" as something that should be considered supported (above, you mention "if we support this"). This is common in applications and anything bundling 3rd part code, where an ESM package depends on a CJS package. Its been a problem in Snowpack before that most users coming from Webpack don't expect.