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

Anders Hejlsberg (@ahejlsberg)

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.

Anders Hejlsberg (@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.

Mohamed Hegazy (@mhegazy)

Mohamed Hegazy (@mhegazy) Opinions on this one?

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.

Ivo Gabe de Wolff (@ivogabe)

as Ivo Gabe de Wolff (@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.

Mohamed Hegazy (@mhegazy)

Mohamed Hegazy (@mhegazy) Sounds like a good solution for the meantime.

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'.

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.

Fix #8357: Remove optionality for the definition of `IteratorResult`

Merge pull request #8450 from Microsoft/Fix8357

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

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.

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

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

martine

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.

Check in custom typings; fix for...of loops.

Evan Martin (@evmar)

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

Navigation Menu

Repository navigation

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

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

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

Description

Activity

ahejlsberg commented on Apr 28, 2016

ahejlsberg commented on Apr 28, 2016

ivogabe commented on Apr 28, 2016

mhegazy commented on Apr 28, 2016

ivogabe commented on Apr 28, 2016

ivogabe commented on May 2, 2016

RReverser commented on May 2, 2016

AdamWillden commented on Jun 10, 2016

mhegazy commented on Jun 10, 2016

AdamWillden commented on Jun 11, 2016

evmar commented on Jun 16, 2016

mhegazy commented on Jun 16, 2016

geon commented on Aug 28, 2017

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions