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

Incorrect transpilation for closure in 'for' loop header referencing block-scoped loop variable #17632

Closed
kpreisser opened this issue Aug 5, 2017 · 3 comments
Assignees
Labels
Bug A bug in TypeScript ES6 Relates to the ES6 Spec

Comments

@kpreisser
Copy link
Contributor

kpreisser commented Aug 5, 2017

TypeScript Version: 2.5.0-dev.20170803

Hi, I noticed TypeScript incorrectly transpiles the following code to ES5/ES3 where a closure is used in a for loop "header" referencing the block-scoped loop variable. This probably doesn't occur in real-world code, however:

Code

let callback1;
let callbacks2 = new Array();
let callbacks3 = new Array();

for (
    let i = (callback1 = () => i, 0);
    callbacks2.push(() => i), i < 5;
    callbacks3.push(() => i), i++) {
    // loop body
    i++;
}

console.log(callback1());
console.log(callbacks2[0]());
console.log(callbacks3[0]());

Expected output (when running in Node, Chrome, Firefox or Edge):

0
1
3

Actual output (after transpiling to ES5):

6
6
6

Actual transpiled code:

var callback1;
var callbacks2 = new Array();
var callbacks3 = new Array();
var _loop_1 = function (i) {
    // loop body
    i++;
    out_i_1 = i;
};
var out_i_1;
for (var i = (callback1 = function () { return i; }, 0); callbacks2.push(function () { return i; }), i < 5; callbacks3.push(function () { return i; }), i++) {
    _loop_1(i);
    i = out_i_1;
}
console.log(callback1());
console.log(callbacks2[0]());
console.log(callbacks3[0]());

I'm not sure how the correct emitted code should ideally look like - maybe similar to this attempt (which produces the correct result):

var callback1;
var callbacks2 = new Array();
var callbacks3 = new Array();

var i_1;
var _loop_firstIteration_1 = true;
var _loop_init_1 = function () {
    var i = i_1;
    i = (callback1 = function () { return i; }, 0);
    i_1 = i;
}
var _loop_1 = function () {
    var i = i_1;

    if (_loop_firstIteration_1) {
        _loop_firstIteration_1 = false;
    }
    else {
        callbacks3.push(function () { return i; }), i++;
    }

    if (!(callbacks2.push(function () { return i; }), i < 5))
        return "break";

    // loop body
    i++;

    i_1 = i;
};

_loop_init_1();
while (true) {
    var state_1 = _loop_1();
    if (state_1 === "break")
        break;
}

console.log(callback1());
console.log(callbacks2[0]());
console.log(callbacks3[0]());

Thanks!

@ikatyang
Copy link
Contributor

ikatyang commented Aug 5, 2017

FYI, babel also outputs 5 5 5.

@jameswilddev
Copy link

jameswilddev commented Aug 6, 2017

I just found something similar in 2.4.2, might be the same issue:

const items = ["a", "b", "c"]
const callbacks = []
const collected: string[] = []

for (const index in items) {
	callbacks.push(() => {
		// index // uncomment to workaround bug
		collected[index] = "present"
	})
}

for (const callback of callbacks) callback()

console.log(collected)

Basically this example iterates over the indices of an array, capturing them in callbacks, which are then executed. These then use that index to populate a sparse array.

Without the workaround, this produces ", , present". With the workaround (just evaluating the index outside of an array accessor, not using it for anything) this produces "present, present, present".

I'm using ES3 but this applies to ES5 as well.

@aluanhaddad
Copy link
Contributor

aluanhaddad commented Aug 7, 2017

Looks like a regression as that works in the playground.

@mhegazy mhegazy added the Bug A bug in TypeScript label Aug 23, 2017
@mhegazy mhegazy added this to the TypeScript 2.6 milestone Aug 31, 2017
@mhegazy mhegazy added the ES6 Relates to the ES6 Spec label Oct 11, 2017
@mhegazy mhegazy modified the milestones: TypeScript 2.6.1, Future Oct 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript ES6 Relates to the ES6 Spec
Projects
None yet
Development

No branches or pull requests

6 participants