-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Question regarding mixing default and named exports #1961
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
Comments
@transitive-bullshit I'm sure warning is quite descriptive and ask you to use explicit option to determine behavior. |
Yep, the option docs were just a bit confusing, but changing my output to use Thanks! |
The docs state:
I'd love to get some clarity on why this is considered a bad practice because empirically this is how many top-level libraries provide their exports. Thanks! |
It is not really a problem if you export to ESM (though some consider it bad style) but it definitely is a problem if you export to CJS and your consumer is not using a bundler. As CJS only has a single // only a default
// ESM input
export default 42;
// CJS output
var main = 42;
module.exports = main; // only named
// ESM input
export var a = 42;
// CJS output
Object.defineProperty(exports, '__esModule', { value: true });
var a = 42;
exports.a = a; Now if I would add a default export in the second case like I did in the first place, this would replace all named exports. Which is why the following happens if you add a default export: // mixed
// ESM input
export var a = 42;
export default 42;
// CJS output
Object.defineProperty(exports, '__esModule', { value: true });
var a = 42;
var main = 42;
exports.a = a;
exports.default = main; So your default export is added as a However if people use a bundler and ESM, they will not notice this because the As for the "bad style": There are many articles describing the disadvantages of default exports, e.g. https://blog.neufund.org/why-we-have-banned-default-exports-and-you-should-do-the-same-d51fdc2cf2ad |
Really appreciate the thorough answer @lukastaegert -- thanks so much! |
I even prefer to omit default exports at all. |
I think this is only true depending on the type of the default export. If it's a function or class (e.g. constructor function), no big. I have done both of these in CJS: module.exports = function () {};
module.exports.foo = 'bar'; class Stuff {
constructor() {}
static get foo() {
return 'bar';
}
}
module.exports = Stuff; Both are easy to wrangle with ESM. |
// mixed
// ESM input
export var a = 42;
export default () => {};
// CJS output
Object.defineProperty(exports, '__esModule', { value: true });
var a = 42;
var main = () => {};
exports.a = a;
exports.default = main; Is there a way in latest rollup to generate output without // desired CJS output
var a = 42;
var main = () => {};
exports = main;
exports.a = a; I found |
Technically this is error prone way. What should produce var a = 42;
export default function main() {}
main.a = a; |
I want to propose a ES version for module.exports = pathToRegexp
module.exports.parse = parse
module.exports.compile = compile
module.exports.tokensToFunction = tokensToFunction
module.exports.tokensToRegExp = tokensToRegExp But there are a requirement to keep it backward compatible. export default pathToRegexp
export { parse }
export { compile }
export { tokensToFunction }
export { tokensToRegExp } now I am trying to generate backward compatible version for // CJS output
Object.defineProperty(exports, '__esModule', { value: true });
var a = 42;
var main = () => {};
exports = main; // <= how to add this line?
exports.a = a;
exports.default = main; |
The best you can do here is to use different entry points and propose named exports later. export default pathToRegexp
pathToRegexp.parse = parse
pathToRegexp.compile = compile
pathToRegexp.tokensToFunction = tokensToFunction
pathToRegexp.tokensToRegExp = tokensToRegExp
export { parse, compile, tokensToFunction, tokensToRegExp } module.exports = pathToRegexp
pathToRegexp.default = pathToRegexp
pathToRegexp.parse = parse
pathToRegexp.compile = compile
pathToRegexp.tokensToFunction = tokensToFunction
pathToRegexp.tokensToRegExp = tokensToRegExp Though mixing named and default exports is still error prune. If commonjs module uses package with module field which has mixed named and default exports webpack produces broken output. So even interop does not help here.
So I recommend just do breaking change and migrate to named exports. They work fine. Default export was a mistake. |
I agree mixing esm and cjs is wrong, but why generation of cjs file from esm with such content is a bad idea? // CJS output
module.exports = main;
Object.defineProperty(module.exports, '__esModule', { value: true });
var a = 42;
var main = () => {};
module.exports.a = a;
module.exports.default = main; UPD: I found a hackish workaround by using // CJS output
exports = module.exports = main;
Object.defineProperty(exports, '__esModule', { value: true });
var a = 42;
var main = () => {};
exports.a = a;
exports.default = main; UPD2: See PR pillarjs/path-to-regexp#197 |
@TrySound do you have a link to an article or something why mixing named and default exports is problematic? Because from a purely semantic / aesthetic point of view I find being able to import something like quite legit. import Sblendid, { Peripheral } from "@sblendid/sblendid"; I don't want to exploit this issue for a discussion on this because this seems off topic and I could not find any proper articles on this, but maybe you know of something. I came across this issue while implementing my library and posted a related question on StackOverflow, which may also be interesting to people who stumble across this issue https://stackoverflow.com/questions/58246998/mixing-default-and-named-exports-with-rollup |
This kind of explanation needs to be added to the docs. I came here because the statement
without further explanation or references was very bothersome to read in the documentation. Users of rollup shouldn't have to trawl the Github issues to make sense of the documentation. |
Is this how React does it? |
Yes, for now. |
One thing I don't understand. If you look at React's How then are we able to import import React, { useEffect, useState } from "react"; And this when using commonJS: const React = require("react");
const { useEffect, useState } = require("react"); ? |
Rollup does not allow to generate such hacky exports. You should build them manually and then provide commonjs namedExports option because such exports cannot be detected.
In upcoming commonjs version named exports will be detected from usage and you will not need to specify namedExports option. |
But React uses Rollup, so I guess they achieve it somehow. How does the export default {
input: "src/index.js",
output: [
{
file: pkg.main,
format: "cjs",
sourcemap: true,
exports: "named" // <---- ???
},
{
file: pkg.module,
format: "es",
sourcemap: true,
},
], ? Thank you! |
React is distributed as CommonJS. There is no defined interface between CommonJS and ESM except possibly what NodeJS has implemented now which is that CommonJS modules only have a default export. Bundler creators in the past however have allowed to use named import syntax with CommonJS. But this is a feature of whoever is IMPORTING your stuff. Now React has a huge problem. Because people now want for quite a while that React is distributed as ESM. However with ESM this hacky syntax no longer works. Which is why they are now telling everyone to use |
@lukastaegert Sorry Lukas, one last question. I have rechecked the React source here: https://unpkg.com/[email protected]/cjs/react.development.js Their CommonJS build exports stuff in the following way: exports.Children = Children;
exports.Component = Component;
exports.Fragment = REACT_FRAGMENT_TYPE;
exports.Profiler = REACT_PROFILER_TYPE;
// And so ... When I import import React from "react";
console.log(React); // { Children: ..., Component: ..., Fragment: ... , Profiler: ..., ... }
React.memo(...);
React.Children;
React.Component;
// And so on... But, if I bundle my own library with Rollup using this code and Rollup config: // src/index.js
import Component1 from "./Component1";
import Component2 from "./Component2";
import Component3 from "./Component3";
export {
Component1,
Component2,
Component3
}; // rollup.config.js
import babel from "rollup-plugin-babel";
import commonjs from "@rollup/plugin-commonjs";
import external from "rollup-plugin-peer-deps-external";
import postcss from "rollup-plugin-postcss";
import resolve from "@rollup/plugin-node-resolve";
import url from "@rollup/plugin-url";
import svgr from "@svgr/rollup";
import inject from "@rollup/plugin-inject";
export default {
input: "src/index.js",
output: [
// Output only CommonJS module in dist/index.js, just like React does.
{
file: "dist/index.js",
format: "cjs",
sourcemap: true,
}
],
plugins: [
external(),
postcss({
modules: true,
}),
url(),
svgr(),
babel({
exclude: "node_modules/**",
runtimeHelpers: true,
plugins: [["@babel/transform-runtime"]],
}),
resolve(),
commonjs(),
inject({
moment: "moment"
}),
],
}; I end up with this 'use strict';
Object.defineProperty(exports, '__esModule', { value: true });
function _interopDefault (ex) { return (ex && (typeof ex === 'object') && 'default' in ex) ? ex['default'] : ex; }
...
exports.Component1 = Component1; // <--- Similar to React's CommonJS exports
exports.Component2 = Component2; // <--- "
exports.Component3 = Component3; // <--- "
//# sourceMappingURL=index.js.map // <--- " Then, when I import it this way in some ESM consuming code: import Components from "my-library";
console.log(Components); // undefined - But expected: Components.Component1, Components.Component2, ... How can I obtain the same behaviour as React? Thank you! |
The |
@lukastaegert That option did it, thank you for your reply, Lukas! |
When transpiling ESM to CJS, Rollup converts default exports to an explicitly exported variable named 'default'. Since this is undesireable, let's convert it to a named export. Incidentally, Rollup's documentation states that: > It is bad practice to mix default and named exports in the same module, > though it is allowed by the specification. More details on this here: - rollup/rollup#1961 (comment) - https://blog.neufund.org/why-we-have-banned-default-exports-and-you-should-do-the-same-d51fdc2cf2ad
hi, you have an example of the use of esModule, I have tried to do the same but it doesn't work for me |
Hi, I was able to achieve the functionality mentioned above with default setup of Rollup. Here is what i have in my setup: rollup.config.js export default {
input: pkg.source,
output: [
{
file: pkg.browser,
format: 'cjs',
exports: 'named'
},
{
file: pkg.module,
format: 'es',
name: pkg.name
}
]
} I hope it helps. |
faced this issue and it was related to using |
Hi, I am able to achieve the functionality requested above by @frenzzy and @LukasBombach by patching rollup. I know it sounds bad. But for those who really wants it this way, Here it is: https://github.com/avisek/rollup-patch-seamless-default-export // Default export
const lib = require('your-library')
lib('Hello') // <-- instead of using `.default` property
// Named exports
const { namedExport1, namedExport2 } = lib
// One liner syntex. This is also supported.
const { default: defaultExport, namedExport1, namedExport2 } = require('your-library') |
Hi, you can use @mnrendra/rollup-plugin-mixexport. It will automatically generate the mixexport named and default. |
I have a react module boilerplate which makes use of rollup, and some users are requesting the ability to export multiple components.
I'm bundling to
cjs
andes
formats (rollup.config.js), so including both named and default exports from my bundle should be fine as I understand it, but rollup prints a warning about the use of mixed default and named exports.For more context, see this thread.
In particular, I can see why this would be a problem for
umd
bundles (source), but I'm not sure why rollup is complaining fores
andcjs
bundles.Any idea how we could resolve this issue?
The text was updated successfully, but these errors were encountered: