Variable is considered undefined in for-of loop when target is es6

[ivogabe]

The variable in a for-of loop contains undefined when the target is set to es6. For es5, everything works as expected.

TypeScript Version:

nightly (1.9.0-dev.20160428)

Code

for (const x of ["a", "b"]) {
    x.substring;
}

tsconfig.json:

{
    "compilerOptions": {
        "module": "commonjs",
        "target": "es6",
        "strictNullChecks": true
    }
}

Expected behavior:
Type of x is string in the loop body, no compile error.

Actual behavior:
Type of x is string | undefined, which gives an error.

@ahejlsberg

[ahejlsberg]

Thanks, good catch. There's an interesting challenge in fixing this. We currently have the following definitions:

interface IteratorResult<T> {
    done: boolean;
    value?: T;
}

interface Iterator<T> {
    next(value?: any): IteratorResult<T>;
    return?(value?: any): IteratorResult<T>;
    throw?(e?: any): IteratorResult<T>;
}

We derive the element type of an iterator from the type of the value property in the result of the next method. It is currently T | undefined because it is declared with the ? modifier. We could strip the undefined (corresponding to what we do with the ! postfix operator), but the problem is we can't tell if it came from the ? or was part of the type argument for Iterator<T>. Specifically, for an Iterator<string | undefined> we really want the element type to be string | undefined and not just string. We could cheat and look at the type argument, but then it wouldn't work for structural matches. Alternatively, we could remove the ? modifier from value with the assumption that anyone directly consuming the result of next would first check the done property.

[ahejlsberg]

@mhegazy Opinions on this one?

[ivogabe]

Thanks for the explanation! I think the proposed suggestions are good, but a little dangerous. A better fix would be, in my opinion, to change the type of IteratorResult to:

type IteratorResult<TYield, TReturn> = { done: true, value: TYield } | { done: false, value: TReturn }

A generator without a return statement could then be typed as IteratorResult<T, undefined>. Though this would require boolean literal types and member type guards (#6062) like if (result.done === true) {}. Given that string literal types already exist, I think (hope) this won't be too dificult.

[mhegazy]

as @ivogabe mentioned, the IteratorResult is not accurate, so my vote would be to make it a little bit less accurate by removing the ?, until we can model the correct type. The other option is subtle and would be hard to diagnose if you ever run into it.

[ivogabe]

@mhegazy Sounds like a good solution for the meantime.

[ivogabe]

I think that this also affects the spread operator in function calls:

function sum(...xs: number[]) {
    return xs.reduce((a,b) => a+b, 0);
}
const list = [1, 2, 3, 4, 5];
sum(...list);
//  ~~~~~~~ Argument of type 'number | undefined' is not assignable to parameter of type 'number'.
//      Type 'undefined' is not assignable to type 'number'.

[RReverser]

I'm wondering if it would be possible to use same solution as Array.from does - actual type parameter from Iterable<T> instead of Iterator / IteratorResult; this way Array.from produces correct result type even though it also operates on iterables.

[AdamWillden]

I still seem to be facing this issue with 1.9.0-dev.20160610-1.0, is it just me?

[mhegazy]

do you have your copy of lib.d.ts? can you provide more context?

Also, a code snippet that can be copied/pasted would be much easier to use than screenshots.

[AdamWillden]

Please excuse, I think it's a problem with my setup anyway.

I have an older JSPM install of typescript and even though I'm pointing VSCode to the correct NPM nightly libs it still seems to be picking up the JSPM install which is causing the errors shown.

Sorry for the lack of info

[evmar]

Treating this as a discriminated unions keyed on a bool would help, so it's not so bad to do something weaker for now if you think that solution will be coming soon:

interface IteratorResult<T> {
    done: true;
}
interface IteratorResult<T> {
    done: false;
    value: T;
}

[mhegazy]

@martine this is tracked by #8938. we are currentlly working on boolean and numeric literal types; once that is in, the change should be simple.

[geon]

@evmar : You can't have an IteratorResult without a value, since you can return from a generator. But perhaps that should be strongly discouraged?