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

Prevent labels from being renamed due to let/const transpilation. #236

Merged
merged 1 commit into from
Mar 26, 2020

Conversation

luiscubal
Copy link
Contributor

When a label has the same name as a variable that has been renamed due to let/const transpilation, the label may also be renamed.
This should not happen, since labels do not follow the same scope rules as variables.
This commit prevents labeled continues and label statements from being renamed, fixing the issue.
Labeled break were not buggy, as breaks do not initialise their children anyway, but this commit also adds breaks to the list of renames to skip, for consistency.

See the added tests for examples that were previously buggy.

When a label has the same name as a variable that has been renamed
due to let/const transpilation, the label may also be renamed.
This should not happen, since labels do not follow the same scope
rules as variables.
This commit prevents labeled continues and label statements from
being renamed, fixing the issue.
Labeled break were not buggy, as breaks do not initialise their
children anyway, but this commit also adds breaks to the list
of renames to skip, for consistency.
@luiscubal
Copy link
Contributor Author

luiscubal commented Dec 18, 2019

No clue why the tests for Node 6 are failing.

Copy link
Collaborator

@mourner mourner left a comment

Choose a reason for hiding this comment

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

This looks good to me, but I have no clue why the build failed in Node 6 too. We should probably start dropping old Node support, but the failure is still worth investigating just in case.

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.

3 participants