Transpile separate scope in each for loop iteration with let and ES5

[nycdotnet]

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);
    }
}

[danquirk]

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]

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]

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]

Much appreciated.

[jods4]

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]

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]

@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 let scope in for loops or the experimental async that 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]

@danquirk Any chance you'd take a PR for this for TS 1.7?

[mhegazy]

@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. @vladima can give you some context here.