Skip to content
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

mixed default and named exports not working in cjs and umd #264

Closed
FezVrasta opened this issue Dec 4, 2018 · 11 comments · Fixed by #265
Closed

mixed default and named exports not working in cjs and umd #264

FezVrasta opened this issue Dec 4, 2018 · 11 comments · Fixed by #265

Comments

@FezVrasta
Copy link
Contributor

FezVrasta commented Dec 4, 2018

const a=1, b=2, c=3, d=4;
export { a, b, c};
export default d;

if you try to bundle this, the result is:

module.exports=4;
//# sourceMappingURL=test3.js.map

I'd expect:

module.exports.a=1;
module.exports.b=2;
module.exports.c=3;
module.exports.default=4;
//# sourceMappingURL=test3.js.map

it exports everything only on the esm build.

Is there any flag to turn on?

@marvinhagemeister
Copy link
Collaborator

Made PR #265 to fix this. There unfortunately isn't a workaround for keeping both named and default exports.

@FezVrasta
Copy link
Contributor Author

There unfortunately isn't a workaround for keeping both named and default exports.

What do you mean? AFAIK the workaround is to use the default key for the default export.

@marvinhagemeister
Copy link
Collaborator

@FezVrasta during the bundling process microbundle currently removes named exports and will only use the default export when both are present.

@FezVrasta
Copy link
Contributor Author

Sorry I don't think I understand :-(

@developit
Copy link
Owner

it's supposed to be an optimization for libraries that do this:

export { foo, bar, baz };
export default { foo, bar, baz };

Normally, that would result in the following commonjs output:

module.exports = { foo, bar, baz };
Object.defineProperty(module.exports, '__module', { value: true });
module.exports.default = { foo, bar, baz };

@ForsakenHarmony
Copy link
Collaborator

we really shouldn't handle that

@Andarist
Copy link
Collaborator

Andarist commented Dec 8, 2018

I also think this is not needed, why anyone is doing this and why we’d like to strip named exports?

If we plan to support this (given there is some reason to do so) then the heuristic should be tweaked - it should only happen when exported namespace has the same shape as object exported as default

@developit
Copy link
Owner

@Andarist agreed! Is there a way to hook into Rollup and actually see what the exports are? Perhaps an AST plugin that just looks for the duplicative exports on the entry module...

@ForsakenHarmony It's something from a fairly long time ago, maybe not needed really these days now that ES Modules usage is both more common and less Babel-specific. I'd still like a way to output the optimal commonjs/umd bundle while catering to the ES Modules hack, but I could also potentially do that using a second entry module.

@Andarist
Copy link
Collaborator

Andarist commented Dec 9, 2018

@Andarist agreed! Is there a way to hook into Rollup and actually see what the exports are? Perhaps an AST plugin that just looks for the duplicative exports on the entry module...

Not at the time we want it, thanks to static shape of ESM it should be really easy to implement this as custom plugin using transform hook and babel in it.

@ForsakenHarmony It's something from a fairly long time ago, maybe not needed really these days now that ES Modules usage is both more common and less Babel-specific. I'd still like a way to output the optimal commonjs/umd bundle while catering to the ES Modules hack, but I could also potentially do that using a second entry module.

IMHO it would be better to implement support for different entries per format (using convention like index.cjs.js or something) - that way it would be more explicit, current approach is implicit and might be surprising.

@developit
Copy link
Owner

I'm okay with removing the pruning I guess. For the couple cases I rely on it I can just use index.cjs.js. That gets easier too if we find a way to do multiple compiles in a single run (subpackages).

bors bot added a commit that referenced this issue Dec 18, 2018
265: Remove import pruning r=ForsakenHarmony a=marvinhagemeister

Fixes #264 

**EDIT:** PR #262 likely needs to be merged first, as that one fixes the failing CI.

Co-authored-by: Marvin Hagemeister <marvin@marvinhagemeister.de>
Co-authored-by: Jason Miller <developit@users.noreply.github.com>
bors bot added a commit that referenced this issue Dec 18, 2018
265: Remove import pruning r=ForsakenHarmony a=marvinhagemeister

Fixes #264 

**EDIT:** PR #262 likely needs to be merged first, as that one fixes the failing CI.

Co-authored-by: Marvin Hagemeister <marvin@marvinhagemeister.de>
Co-authored-by: Jason Miller <developit@users.noreply.github.com>
Co-authored-by: Leah <me@hrmny.sh>
@bors bors bot closed this as completed in #265 Dec 18, 2018
@itsmichaeldiego
Copy link

itsmichaeldiego commented Aug 31, 2020

Hey everyone! I am having the same issue, if I export like this:

import GoogleMap from './google_map';

export default GoogleMap;

It will build:

module.exports = GoogleMap;

However, when including named exports, it changes to:

exports.namedExport = namedExport;
...
exports.default = GoogleMap;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants