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

Properly deconflict ids of default exports that have been reassigned #2502

Merged
merged 1 commit into from Oct 10, 2018

Conversation

lukastaegert
Copy link
Member

This PR contains:

  • bugfix
  • feature
  • refactor
  • documentation
  • other

Are tests included?

  • yes (bugfixes and features will not be merged without tests)
  • no

Breaking Changes?

  • yes (breaking changes will not be merged unless absolutely necessary)
  • no

List any relevant issue numbers:
Resolves #2414

Description

See also #2414. The logic for handling default exports of function and class declarations did not properly take into account that those not only declare a local variable as well but that reassigning this local variable will reassign the default export. Cf. these examples:

// the default export will be 42 because it is the value of a that is exported
var a = 42;
export default a;
a = 43;

vs.

// the default export will return 43 because it is the binding of a that is exported
export default function a() {return 42;};
a = () => 43;

Previously, only the first case was handled properly while this PR also handles the second case right. This also cleans up some logic so that almost everything related to default exports with ids is now handled together in ExportDefaultVariable.

Copy link
Member

@TrySound TrySound left a comment

Choose a reason for hiding this comment

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

Great! Thank you

Copy link
Member

@TrySound TrySound left a comment

Choose a reason for hiding this comment

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

Great! Thank you

@lukastaegert lukastaegert merged commit df9ef28 into master Oct 10, 2018
@lukastaegert lukastaegert deleted the deconflict-reassigned-default-export-id branch October 10, 2018 14:45
@lukastaegert
Copy link
Member Author

I put this into 0.66.6 :)

@guybedford
Copy link
Contributor

Awesome work!

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 this pull request may close these issues.

Babel helper export is wrongly hoisted
3 participants