-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
Kill CommonJS default export behaviour #2212
Comments
What's the planned behavior going forward? |
@cesarandreu To always export a default to |
👍 |
@jmm Not really. It's the fact that people are (wrongly) doing: a.js import { foo, bar } from "./b"; b.js export default {
foo: "foo",
bar: "bar"
}; And Babel is letting them. |
@sebmck That's just another symptom of bad implicit magic, imho. If you add a named export to |
@sebmck Oh ok, I wasn't aware of that behavior. 👍 to killing that off (obviously). |
Hi! first, thanks for all the hard work and excellent tool. is the idea that commonJS consumers would need to |
@jquense or you can tell the consumers to prefer the ES2015 module syntax over the CJS one. 😉 |
@UltCombo, I really can't tell anyone building a node app to also use Babel to transpile their app to avoid some non idiomatic import with my module. they just won't use my module instead |
@jquense I don't really see a problem as long as the API usage is properly documented for CJS and ES Modules. You may as well use a named export to give a more meaningful property name than |
It's just not idiomatic, it breaks expectations and is a hassle (albeit a small one) for consumers. for packages that provide a lot of modules, asking ppl to remember the name of a bunch of single export module names is an unnecessary burden. ultimately I am not going to over complicate an API surface just so I can use the es6 module syntax. I appreciate that others are willing to make that tradeoff, I just wanted to provide feedback that I would not, hoping that it provides a helpful perspective :) |
If you really want to use ES Modules syntax without having to ask your CJS consumers to add module.exports = require('./transpiled/index').default; |
@UltCombo Hmm, I guess you're probably right that I didn't think that through and misunderstood the motivation, not the actual change. What a tricky little piece of interop. I somewhat agree that documenting the API of a module as var Readable = require('stream').Readable; That being said, I think most of the time, especially when the default export is a function (and even when there are other exports), I'd be pretty happy with output like: exports["default"] = function () {};
module.exports = exports["default"];
module.exports["default"] = module.exports; (Not saying that should be the default output, but I'd probably use it...unless someone points out bad problems with it I haven't thought of.) Then you could do |
since esModules are being tagged with I agree that sticking to idioms isn't something that needs to be done dogmatically. However, getting folks onboard and using es modules over deeply established and working commonjs modules, is something transpilers can be very helpful in (and something that is needed if they are going to get used). However, I wouldn't underestimate folks not wanting to switch when it means adding a bunch of unnecessary api noise. the reexport approach works fine if you are only exporting a single module, but its not weird to have many export modules, like lodash: |
I believe Babel has opted out of this way in the past due to conflicts in these cases: export default { x: 1 };
export const x = 2; Also cases such as iterating over the default export's properties, etc. |
Yeah, that all makes sense. What I'm talking about is perhaps too opinionated as a default, but I think I'd like to be able to opt-in to doing what I suggested when the default export can be statically determined to be a function. And I'm guessing that's probably the 80% case for npm modules?
Not compile-time -- each file is transformed in isolation. Maybe at run-time (with obvious, though maybe negligible, performance overhead)? How...exporting another property like
As you can see from my comments, I actually like the idea of supporting the common case better, at least in opt-in fashion, if there's a reasonable way to do it. |
I'd suggest removing the default export behavior, then allow opting-in to it through |
So per discussion on Slack, apparently module formatters will be able to be implemented as plugins in |
can we at least support it with a --flag of some sort? |
Killed it. I'm not willing to support this in Babel, even as an option, as it's proved to be very dangerous for refactoring and semantics. Just to be clear, this just changes the export interop. Not the import. |
This change also affects the common import-and-call pattern using require:
I can see the argument against letting people shoot themselves in the foot with named imports, but this change definitely shits in the commonjs interop punchbowl.
It also requires any coffee-script files that import ES6 modules to be refactored, fwiw. (for default exports at least)
Came across this plugin if anyone needs to restore the babel 5 behavior: https://www.npmjs.com/package/babel-plugin-add-module-exports |
No description provided.
The text was updated successfully, but these errors were encountered: