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

Improper let scoping #153

Closed
ShimShamSam opened this issue Sep 9, 2016 · 4 comments
Closed

Improper let scoping #153

ShimShamSam opened this issue Sep 9, 2016 · 4 comments
Labels
bug Confirmed bug

Comments

@ShimShamSam
Copy link

ShimShamSam commented Sep 9, 2016

let x = 0;

for(let i = 0; i < 10; ++i) {
    x++;
}

console.log(x);

is converted to:

for(let x=0,a=0;10>a;++a)x++;console.log(x);

x is being moved into the first part of the for loop which causes it to be scoped only to the for block. The call to console.log(x) is outside of the block and thus throws a ReferrenceError.

@hzoo hzoo added the bug Confirmed bug label Sep 9, 2016
@hzoo
Copy link
Member

hzoo commented Sep 9, 2016

Sounds like we don't want to combine variable declarations in the loop if the outer declaration is also a let/const and only if it's a var - or maybe only do it if not referenced outside?

@ShimShamSam
Copy link
Author

ShimShamSam commented Sep 9, 2016

maybe only do it if not referenced outside

That would work nicely. You could take it a step further and remove any surrounding blocks if all the variables within it are moved into the for loop. For example:

{
    let x = 0;

    for(let i = 0; i < 10; ++i) {
        x++;
    }
}

Should become:

for(let x=0,a=0;10>a;++a)x++;

Notice we can safely remove the surrounding block.

@loganfsmyth
Copy link
Member

Note that there is a slight difference in those two if the loop body had an async callback in it since mutating x from inside the callback would not work the same way as when it is declared outside the loop.

@ShimShamSam
Copy link
Author

ShimShamSam commented Sep 9, 2016

Good point. I guess it's safer to not do that kind of transformation at all.

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

No branches or pull requests

3 participants