Repository navigation
Transpile separate scope in each for loop iteration with let and ES5 #3915
Description
Hi,
I was recently surprised to see that TypeScript 1.5 (using ntypescript, so recent master) doesn't support transpiling down to ES5 the separate scope functionality of ES6 when using a for loop with a let-initialized iterator.
For example:
function buttonsWithLet(count, targetElement) {
for (let i = 1; i <= count; i+= 1) { //error 4091: Loop_contains_block_scoped_variable_0_referenced_by_a_function_in_the_loop_This_is_only_supported_in_ECMAScript_6_or_higher
var button = makeButton("alert " + i);
button.onclick = function() {
alert("This is button " + i + ".");
};
targetElement.appendChild(button);
}
}This is where Error 4091 was implemented.
5f2588f
3b3a94c
Is the TypeScript team open to transpiling this code in ES5 mode? If so, would you accept a pull request? The above code is transpiled by Babel as such which is really clean:
function buttonsWithLet(count, targetElement) {
var _loop = function (i) {
button = makeButton("alert " + i);
button.onclick = function () {
alert("This is button " + i + ".");
};
targetElement.appendChild(button);
};
for (var i = 1; i <= count; i += 1) {
var button;
_loop(i);
}
}but a plausibly easier transpile might be:
function buttonsWithVarAndIIFE(count, targetElement) {
for (var i = 1; i <= count; i += 1) {
var button = makeButton("alert " + i);
button.onclick = (function (i) {
return function () {
alert("This is button " + i + ".");
};
})(i);
targetElement.appendChild(button);
}
}
Activity
[-]Transpile separate scope for each for loop iteration with let and ES5[/-][+]Transpile separate scope in each for loop iteration with let and ES5[/+]danquirk commentedon Jul 17, 2015
I believe the concern here was the surprising perf impact this creates in user code. Certainly an argument can be made that people should be allowed to take on that hit if they want and this sort of rule is perhaps better suited to a linter.
nycdotnet commentedon Jul 17, 2015
Thanks for considering this, Dan - I appreciate it.
Do you mean the performance hit of having a closure per iteration? If so, I don't see how that's avoidable whether TS can transpile it or not (since the dev would just write it themselves if going down this path) unless your expectation is that ES6 engines will have significant optimizations for this pattern that today's engines don't have?
danquirk commentedon Jul 17, 2015
Do you mean the performance hit of having a closure per iteration?
Correct. It's a non-trivial hit (especially in a tight loop) and not obvious.
If so, I don't see how that's avoidable whether TS can transpile it or not (since the dev would just write it themselves if going down this path)
Yeah, the 'people will do this anyway' argument is not irrelevant. I think the restriction was initially borne of trying to protect people from doing a simple var->let rename and finding large and surprising performance regressions. This particular error is definitely a bit of an outlier for the compiler today, I'm not sure we give any other performance related warnings.
nycdotnet commentedon Jul 17, 2015
Much appreciated.
jods4 commentedon Jul 17, 2015
The loop closure is a prime example of why you would use let rather than var. Not supporting it is quite unfortunate. The argument that there is no way to do it otherwise is quite strong, I've been writing those loop closures manually for quite some time now, and I would love to get some syntax help and stop.
MrHacky commentedon Aug 1, 2015
very unfortunate indeed, also the roadmap/release notes never mentioned this limitation so i was very disappointed when i found out this didn't work.
Having variables inside for loops just 'do what you want' even in the face of callbacks would be a great improvement for us.
jods4 commentedon Aug 1, 2015
Simon Sasburg (@MrHacky)
If you want better ES5 support today, you can configure TS to emit ES6 code and then pipe that into Babel (formerly 6to5). It will compile all your ES6/7 code into ES5.
Several people do so, because you get support for new features, such as
letscope inforloops or the experimentalasyncthat is already available (behind a flag) but only with ES6 target.You also get a more correct codegen, as babel is closer to the ES specs (e.g. on class inheritance) than TS. And you get the possibility to use additional Babel plugin for more build-related transforms.
This reminds me of the discussion I started here #1641...
nycdotnet commentedon Sep 21, 2015
Dan Quirk (@danquirk) Any chance you'd take a PR for this for TS 1.7?
mhegazy commentedon Sep 21, 2015
Steve Ognibene (@nycdotnet), the issue is marked "in discussion". we would be able to consider it. we have not had a chance to discuss this in the design meeting yet though.
Also keep in mind that the PR will also need to handle labeled statements and breaks within the function body. Vladimir Matveev (@vladima) can give you some context here.
2 remaining items
RyanCavanaugh commentedon Oct 5, 2015
Approved; tentatively assigning to Vladimir Matveev (@vladima) because I hear he has a working prototype of some sort.
Details:
breakcontinuebreak(too complex vs value)vars in the bodynycdotnet commentedon Oct 6, 2015
Cool.
nycdotnet commentedon Oct 27, 2015
Woo hoo! Awesome.
MrHacky commentedon Oct 27, 2015
supercool!
aluanhaddad commentedon Feb 3, 2016
Thanks for implementing this! I've wanted it for a long time. ❤️