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

Top level variables don't have references assigned #56

Closed
jbrumwell opened this issue Mar 31, 2015 · 5 comments
Closed

Top level variables don't have references assigned #56

jbrumwell opened this issue Mar 31, 2015 · 5 comments
Labels

Comments

@jbrumwell
Copy link

Give the following code and using http://mazurov.github.io/escope-demo/;

const a = 'b';
a = 'c';
testing(a);

You can see the variable a has an empty references property here, The references are available under scope.references just not attached to the variable.

@mysticatea
Copy link
Contributor

I got same bug.

var a = 0;
function foo() {
  a = 1;
}

In this case, references of a becomes an empty array.
I expect it to be 2 references; a = 0 and a = 1.

Each scope have a reference that its Reference.resolved property is null.

@michaelficarra
Copy link
Member

For top-level vars, this behaviour makes sense: you can't tell if that will be a reference or not because the scope is dynamic:

var a = 0;
delete global.a;
a;

Technically, we can be certain that the initialisation is a reference, so we could have either one or zero references. But I like the consistency of never tracking references for variables in dynamic scopes.

For let/const variables, though, the references should be tracked. That is a bug.

@mysticatea
Copy link
Contributor

Thanks for explain!
I got it.

@mysticatea
Copy link
Contributor

I'm trying to fix this issue: mysticatea@c644d82

Now, it became passing very simple test cases.
But I'm worried about whether the direction of modifications is correct.

I modified Scope.prototype.__close method.
If the scope is the top-level, checks whether each reference is let/const/class's.

I have not found the meaning that Variable's defs is an array yet.
I'm going to add more test cases.

I'm sorry, my poor English...
Thanks.

@Constellation
Copy link
Member

Great! Let's send pull request!

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

No branches or pull requests

4 participants