Fix getBindingIdentifiers in babel-types #5068

Merged
merged 3 commits into from Jan 9, 2017

Projects

None yet

4 participants

@rtsao
Contributor
rtsao commented Jan 5, 2017 edited
Q A
Patch: Bug Fix? yes
Major: Breaking Change? no
Minor: New Feature? no
Deprecations? no
Spec Compliancy? yes
Tests Added/Pass? yes
Fixed Tickets
License MIT
Doc PR no
Dependency Changes no

Summary of changes:

  • Added passing tests for t.getBindingIdentifiers (the first two tests)
  • Added a test which should pass, but fails without the fix in this PR (the third test)
  • Fixed logic in t.getBindingIdentifiers regarding handling of ExportDeclarations

The correct logic (which is added in this PR) is currently in reflected in the equivalent implementation of getBindingIdentifierPaths function in babel-traverse:

//
    if (id.isExportDeclaration()) {
      const declaration = id.get("declaration");
      if (declaration.isDeclaration()) {
        search.push(declaration);
      }
      continue;
    }
//
rtsao added some commits Jan 5, 2017
@rtsao rtsao Added getBindingIdentifier tests 952af10
@rtsao rtsao Added failing test for getBindingIdentifiers 2d40141
@rtsao rtsao Fix babel-types getBindingIdentifiers
ae87097
@codecov-io

Current coverage is 89.15% (diff: 100%)

Merging #5068 into master will not change coverage

@@             master      #5068   diff @@
==========================================
  Files           203        203          
  Lines          9821       9821          
  Methods        1072       1072          
  Messages          0          0          
  Branches       2614       2614          
==========================================
  Hits           8756       8756          
  Misses         1065       1065          
  Partials          0          0          

Powered by Codecov. Last update 796c6c0...ae87097

@danez
danez approved these changes Jan 5, 2017 View changes
@hzoo hzoo added the tag: bug fix label Jan 9, 2017
@hzoo
Member
hzoo commented Jan 9, 2017

Awesome work @rtsao (I see it's your first PR too)! Yeah we need to figure out how to separate babel-types/traverse/core logic better

@hzoo hzoo merged commit 39d1867 into babel:master Jan 9, 2017

3 checks passed

codecov/patch 100% of diff hit (target 89.15%)
Details
codecov/project 89.15% (+0.00%) compared to 796c6c0
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@panagosg7 panagosg7 added a commit to panagosg7/babel that referenced this pull request Jan 17, 2017
@rtsao @panagosg7 rtsao + panagosg7 Fix getBindingIdentifiers in babel-types (#5068)
* Added getBindingIdentifier tests

* Added failing test for getBindingIdentifiers

* Fix babel-types getBindingIdentifiers
19262eb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment