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

fix(es/transforms/cjs): Allow mixing named exports and export stars. #2583

Merged
merged 9 commits into from
Oct 31, 2021

Conversation

JeremyGrieshop
Copy link
Contributor

@JeremyGrieshop JeremyGrieshop commented Oct 29, 2021

This is my attempt at fixing #2548.



swc_ecma_transforms_module:

@CLAassistant
Copy link

CLAassistant commented Oct 29, 2021

CLA assistant check
All committers have signed the CLA.

@kdy1 kdy1 added this to the v1.2.106 milestone Oct 30, 2021
Copy link
Member

@kdy1 kdy1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!
Nice work, but I found some small issues.

@@ -155,6 +155,19 @@ where
ref specifiers,
..
})) => {
// handle: export {sym as alias1, alias2, ...}
for ExportNamedSpecifier { orig, exported, .. } in
specifiers.into_iter().map(|e| match e {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think filter_map with None for other cases is better.
(Although I don't have concrete input which breaks this)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've pushed a change to try to address this. I hope it's in line with what you were thinking - I had previously used a pattern that I saw was elsewhere in the code.

_ => unreachable!(),
})
{
if !exported.is_none() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use

if let Some(exported). = &exported {
    exports.push(exported.sym.clone());
}

@kdy1 kdy1 modified the milestones: v1.2.106, v1.2.107 Oct 30, 2021
Copy link
Member

@kdy1 kdy1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

@kdy1 kdy1 changed the title Fixing _exportNames when exporting aliases and (all) exports fix(es/transforms/cjs): Allow mixing named exports and export stars. Oct 31, 2021
@kdy1 kdy1 enabled auto-merge (squash) October 31, 2021 09:12
@kdy1 kdy1 merged commit 7e3fb0a into swc-project:master Oct 31, 2021
@swc-project swc-project locked as resolved and limited conversation to collaborators Nov 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

export {default as name} can create duplicate symbols
3 participants