CommonJS module formatter: default export is assigned differently depending on whether there are any named exports #2047

Closed
UltCombo opened this Issue Jul 22, 2015 · 12 comments

Comments

Projects
None yet
4 participants
@UltCombo

This sample code:

export default 1;
export const x = 2;

Generates:

"use strict";

Object.defineProperty(exports, "__esModule", {
  value: true
});
exports["default"] = 1;
var x = 2;
exports.x = x;

Which is fine.
Now if we remove the named export:

export default 1;

Generates:

"use strict";

Object.defineProperty(exports, "__esModule", {
  value: true
});
exports["default"] = 1;
module.exports = exports["default"];

The module.exports is overwritten by the default export.
This still works fine if importing the module via import statements (due to the interopRequireDefault helper), but it is rather confusing and error-prone if importing the CJS module via require() calls.

That is, sometimes the default export is returned directly by require('module'), other times you will need to access the require('module').default property, all depending on an arbitrary factor that may change anytime (adding/removing named exports), which causes a breaking change for all consumers.

@loganfsmyth

This comment has been minimized.

Show comment
Hide comment
@loganfsmyth

loganfsmyth Jul 22, 2015

Member

This is expected behavior: http://babeljs.io/docs/usage/modules/#interop

As documented there:

If you don't want this behaviour then you can use the commonStrict module formatter.

Member

loganfsmyth commented Jul 22, 2015

This is expected behavior: http://babeljs.io/docs/usage/modules/#interop

As documented there:

If you don't want this behaviour then you can use the commonStrict module formatter.

@UltCombo

This comment has been minimized.

Show comment
Hide comment
@UltCombo

UltCombo Jul 22, 2015

Thanks for the quick reply.
I thought the strict module formatters were going to be removed (#587 (comment)), I assume this has changed?

I'll be using the commonStrict formatter again now, thanks. 😄

Thanks for the quick reply.
I thought the strict module formatters were going to be removed (#587 (comment)), I assume this has changed?

I'll be using the commonStrict formatter again now, thanks. 😄

@UltCombo

This comment has been minimized.

Show comment
Hide comment
@UltCombo

UltCombo Jul 22, 2015

So, looks like the commonStrict module formatter does not attempt any interop at all -- I had forgot this bit.

In my ideal use case, I'd like to keep the ES/CJS modules interop, but have the CJS module formatter expose the default export in a consistent way.

Well, I guess for this rare use case (where I need dynamic require()s, can't use ES Modules syntax), I can just as well import and apply the interop-require helper from the babel-runtime package manually.

So, looks like the commonStrict module formatter does not attempt any interop at all -- I had forgot this bit.

In my ideal use case, I'd like to keep the ES/CJS modules interop, but have the CJS module formatter expose the default export in a consistent way.

Well, I guess for this rare use case (where I need dynamic require()s, can't use ES Modules syntax), I can just as well import and apply the interop-require helper from the babel-runtime package manually.

@danielo515

This comment has been minimized.

Show comment
Hide comment
@danielo515

danielo515 Jan 30, 2017

I am also missing this feature. The reasoning behind removing it still unclear to me. Removing this feature almost breaks all the modules that trusted babel for transpile to regular CJS modules.

I am also missing this feature. The reasoning behind removing it still unclear to me. Removing this feature almost breaks all the modules that trusted babel for transpile to regular CJS modules.

@loganfsmyth

This comment has been minimized.

Show comment
Hide comment
Member

loganfsmyth commented Jan 31, 2017

You can get this behavior in Babel 6 via https://www.npmjs.com/package/babel-plugin-add-module-exports

@danielo515

This comment has been minimized.

Show comment
Hide comment
@danielo515

danielo515 Jan 31, 2017

Dear @loganfsmyth, I already found that plugin after a couple of hours of searching. So yes, why not add another plugin to the plethoria of plugins required for the most basic Babel functionality. I don't understand why you just give the option .

Dear @loganfsmyth, I already found that plugin after a couple of hours of searching. So yes, why not add another plugin to the plethoria of plugins required for the most basic Babel functionality. I don't understand why you just give the option .

@xtuc

This comment has been minimized.

Show comment
Hide comment
@xtuc

xtuc Jan 31, 2017

Member

@Daniel15 Would it help to indicate that on the website?

Member

xtuc commented Jan 31, 2017

@Daniel15 Would it help to indicate that on the website?

@danielo515

This comment has been minimized.

Show comment
Hide comment
@danielo515

danielo515 Jan 31, 2017

Dear @xtuc, not helpful to me but definetively helpful for newcomers .

Dear @xtuc, not helpful to me but definetively helpful for newcomers .

@UltCombo

This comment has been minimized.

Show comment
Hide comment
@UltCombo

UltCombo Feb 2, 2017

I don't understand why you just give the option .

This must already have been explained several times in other places, but to put it in simple terms:

Module a.js:

export default { foo: 1 };
export const foo = 2;

Module b.js:

console.log(require('./a').foo);

What do you expect this to print? The old behavior was inconsistent as the default export's properties sometimes could be referenced as named exports (when there are no named exports in the module), but the behavior would change as soon as a named export is added.

It is extremely dangerous that your existing imports would break when the exporter makes a completely unrelated change such as adding or removing named exports.

UltCombo commented Feb 2, 2017

I don't understand why you just give the option .

This must already have been explained several times in other places, but to put it in simple terms:

Module a.js:

export default { foo: 1 };
export const foo = 2;

Module b.js:

console.log(require('./a').foo);

What do you expect this to print? The old behavior was inconsistent as the default export's properties sometimes could be referenced as named exports (when there are no named exports in the module), but the behavior would change as soon as a named export is added.

It is extremely dangerous that your existing imports would break when the exporter makes a completely unrelated change such as adding or removing named exports.

@danielo515

This comment has been minimized.

Show comment
Hide comment
@danielo515

danielo515 Feb 2, 2017

So it is better to break in a backwards manner (breaking all the modules that relies on this functionality) than in forwards manner, even taking into account that some thousands will never export a second property.

In any case, what you said makes a lot of sense and it is logically argued, so I can not be against it. Making breaking changes to change things that makes non sense should happen more often, I say it several times, it is ridiculous that now I'm complaining about one of those.

Thanks and regards

So it is better to break in a backwards manner (breaking all the modules that relies on this functionality) than in forwards manner, even taking into account that some thousands will never export a second property.

In any case, what you said makes a lot of sense and it is logically argued, so I can not be against it. Making breaking changes to change things that makes non sense should happen more often, I say it several times, it is ridiculous that now I'm complaining about one of those.

Thanks and regards

@loganfsmyth

This comment has been minimized.

Show comment
Hide comment
@loganfsmyth

loganfsmyth Feb 2, 2017

Member

We can definitely do better on documentation on this, sorry about that.

The point mentioned above is indeed the problem we looked to solve. I've also elaborated a bunch on that here if you are curious: http://stackoverflow.com/questions/33505992/babel-6-changes-how-it-exports-default

We made this change on a Babel major version so that it would only affect people as they upgrade, and since it was a behavior that was easy to add back via a standalone plugin, we felt it was a good change to make.

This behavior will also likely become more common moving forward when ES6 module support is added to Node itself.

Member

loganfsmyth commented Feb 2, 2017

We can definitely do better on documentation on this, sorry about that.

The point mentioned above is indeed the problem we looked to solve. I've also elaborated a bunch on that here if you are curious: http://stackoverflow.com/questions/33505992/babel-6-changes-how-it-exports-default

We made this change on a Babel major version so that it would only affect people as they upgrade, and since it was a behavior that was easy to add back via a standalone plugin, we felt it was a good change to make.

This behavior will also likely become more common moving forward when ES6 module support is added to Node itself.

@danielo515

This comment has been minimized.

Show comment
Hide comment
@danielo515

danielo515 Feb 2, 2017

Thanks for the extra info. From your apologies I deduct that I'm not the only one confused about this. Just make sure that everybody knows about the issue and the solution.

Thanks for this great project

Thanks for the extra info. From your apologies I deduct that I'm not the only one confused about this. Just make sure that everybody knows about the issue and the solution.

Thanks for this great project

@lock lock bot added the outdated label May 5, 2018

@babel babel locked as resolved and limited conversation to collaborators May 5, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.