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

Binding references count isn't always accurate #8533

Open
tbranyen opened this issue Aug 25, 2018 · 8 comments
Open

Binding references count isn't always accurate #8533

tbranyen opened this issue Aug 25, 2018 · 8 comments

Comments

@tbranyen
Copy link

Bug Report

Current Behavior

Using path.scope.getBinding(someIdentifier) doesn't always return an accurate count of someIdentifier usage.

Input Code

  • REPL or Repo link if applicable:
const core = require('@babel/core');
const traverse = require('@babel/traverse').default;

const code = `
  function test() {
  }

  export default test();
`;

const { ast, map } = core.transformSync(code, {
  sourceType: 'module',
  ast: true,
});

traverse(ast, {
  Program(path) {
    // Why does this equal 1?
    console.log(path.scope.getBinding('test').references);
  }
});

Expected behavior/code

I am expecting to see the number 2, so that it tracks the declaration and identifier usage in the call.

Environment

  • Babel version(s): 7.0.0-rc.1
  • Node/npm version: 10
  • OS: Linux

Possible Solution

Track all identifier usage across calls.

@babel-bot
Copy link
Collaborator

Hey @tbranyen! We really appreciate you taking the time to report an issue. The collaborators
on this project attempt to help as many people as possible, but we're a limited number of volunteers,
so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack
community that typically always has someone willing to help. You can sign-up here
for an invite.

@nicolo-ribaudo
Copy link
Member

I wouldn't expected it to be 2, since test() is a reference to the declaration, but function test() {} isn't referencing anything.

@tbranyen
Copy link
Author

tbranyen commented Aug 25, 2018

@nicolo-ribaudo what you would expect the following to equal?

/* ... */

const code = `
  export const c = false;
  console.log(c);
`;

/* ... */

traverse(ast, {
  Program(path) {
    console.log(path.scope.getBinding('c').references);
  }
});

It's two, it's absolutely inconsistent IMO. One is a declaration in this example and the other is a reference, and yet the count is two. You mentioned that function test() {} isn't referencing anything, which is true, but that's because it got hoisted and literally is the reference in the same way as a variable declaration (which obviously gets counted, per my above example).

This is inconsistent IMO.

@loganfsmyth
Copy link
Member

That's definitely inconsistent. export const c = false should not show up in the references.

@loganfsmyth
Copy link
Member

Removing this whole block fixes that export const c issue and doesn't break any tests 😬

@nicolo-ribaudo
Copy link
Member

I think that export declarations are counted because their binding is used (references -> 0 is for unused variables).

@loganfsmyth
Copy link
Member

Ohh that makes sense. I don't really know that I like that way of approaching it, but that does explain it.

@tbranyen
Copy link
Author

The phrasing "unused variables" sort of contradicts the original post then doesn't it? It seems we're being picky about which identifier usage counts, and which doesn't. This isn't really helpful for the purposes of dead code elimination.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants