Skip to content

Conversation

@ivogabe
Copy link
Contributor

@ivogabe ivogabe commented May 22, 2015

I've implemented a downlevel emit for block scoped variables that are used inside callbacks in a loop.
Example:

for (let i = 0; i < 9; i++) {
  setTimeout(i, () => console.log(i));
}

Is emitted as

var _a = function(i)  {
    setTimeout(i, function () { return console.log(i); });
};
for (var i = 0; i < 9; i++) _a(i);

Todo:

  • Formatting, a do ... while() loop is emitted ugly at the moment.
  • Downlevel emit of a for of loop.
  • Variables declared using destructuring in the head of a for loop aren't added to the closure yet.
  • Tests
  • Comments

I'm not sure whether the code in checker.ts is ok, should the variable blockScopedBindingInLoop be turned into a NodeFlag?

@vladima
Copy link
Contributor

vladima commented May 22, 2015

Some key aspects are still missing in this PR:

  • handling of control transfer statements in loops: return, break and continue (both labeled and non-labeled)
  • usage of arguments in loops
  • function scoped declarations defined in loops, i.e. var

Implementing these items properly is a non-trivial task though certainly doable. When we've originally started the work on downlevel support for let / const we had a design proposal that covered all these aspects. However even with that we still decided to exclude support for let/const that are declared inside loops and captured in closures. Main reason is: converting loop bodies into functions keeps proper semantic for let/const however it also changes runtime behavior and potentially introduces subtle perf/memory issues that will be difficult to discover, i.e. here

Saying that + the fact that we don't have lots of requests for this feature I'm going to close this PR for now. However if later this feature will have high demand we can always reconsider our decision

@vladima vladima closed this May 22, 2015
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants