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 block scoping transform for declarations in labeled statements #4669

Merged
merged 2 commits into from Oct 5, 2016

Conversation

motiz88
Copy link
Contributor

@motiz88 motiz88 commented Oct 4, 2016

Q A
Bug fix? yes
Breaking change? no
New feature? no
Deprecations? no
Spec compliancy? yes
Tests added/pass? yes
Fixed tickets #4122
License MIT
  • Reduce code duplication inside getLetReferences between block and switch handling;
  • Correctly visit LabeledStatement nodes when collecting block-scoped declarations to transform.

@codecov-io
Copy link

codecov-io commented Oct 4, 2016

Current coverage is 88.78% (diff: 100%)

Merging #4669 into master will increase coverage by <.01%

@@             master      #4669   diff @@
==========================================
  Files           195        195          
  Lines         13780      13782     +2   
  Methods        1425       1426     +1   
  Messages          0          0          
  Branches       3174       3173     -1   
==========================================
+ Hits          12235      12237     +2   
  Misses         1545       1545          
  Partials          0          0          

Powered by Codecov. Last update 12d2673...14baf0a

@motiz88 motiz88 added the PR: Bug Fix 🐛 A type of pull request used for our changelog categories label Oct 5, 2016
if (isBlockScoped(node)) {
convertBlockScopedToVar(path, node, block, this.scope);
}
declarators = declarators.concat(node.declarations || node);
Copy link
Member

Choose a reason for hiding this comment

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

If we are already deduplicating code, then we maybe can also deduplicate this if? as it basically twice in here I think.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah is this equiv?

    const addDeclarationsFromChild = (path, node) => {
      node = node || path.node;
      if (t.isLabeledStatement(node)) {
        node = node.body;
        path = path.get("body");
      }
      if (t.isClassDeclaration(node) || t.isFunctionDeclaration(node) || isBlockScoped(node)) {
        if (isBlockScoped(node)) {
          convertBlockScopedToVar(path, node, block, this.scope);
        }
        declarators = declarators.concat(node.declarations || node);
      }
    };

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 Did something similar in d276d57. CI is being fussy though: https://travis-ci.org/babel/babel/jobs/165258111#L161

Grumble grumble. Let me try rebasing to latest master.

@danez
Copy link
Member

danez commented Oct 5, 2016

👍

@danez danez merged commit 7a7704f into babel:master Oct 5, 2016
panagosg7 pushed a commit to panagosg7/babel that referenced this pull request Jan 17, 2017
…abel#4669)

* Fix block scoping transform for declarations in labeled statements (babel#4122)

* DRY block-scoping
@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 7, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Bug Fix 🐛 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants