[Bug] Type guarding fails.

[zheeeng]

TypeScript Version: 2.6.2

Code

declare const o: {
    records?: string[]
}

const newRecords = o.records &&
    o.records.map((record, i) => record + (o.records.length === i) ? '' : ',')

Expected behavior:

o.records is safe to using length property.

Actual behavior:

warning:

Object is possibly 'undefined'.
(property) records: string[] | undefined

Playground Link:
playground

Note: enable strict mode is required for reproducing.

Related Issues:

[aluanhaddad]

This is expected. It happens because the language doesn't know when the closure will be called.

There is a detailed explanation in #9998

[massimonewsuk]

This behaviour can be understood by looking at the following example:

declare const o: {
    records?: string[]
}

const newRecords = o.records &&
    o.records.map((record, i) => record + (o.records.length === i) ? '' : ',')

function foo(records: any[] | null) {
    if (records) {
        records.map((record, i) => record + (records.length === i) ? '' : ',')
    }
}

Then, keep in mind that properties on objects can change. The closure only has a reference to o, not records. You could have implemented o.records as a getter which deletes itself from o after the first time it has been evaluated. Then o.records would be undefined.

[massimonewsuk]

@RyanCavanaugh I don't think this is a bug. In the function foo, there is no way anything else could change records, so it's safe for the closure to rely on a type guard that executed before the closure was formed. In the author's original example, there are other things which could come into play, so it's not safe to rely on it.

[zheeeng]

@mhnewsuk If we have to consider mutation getter, many type analysis will be broken. Assuming o.records delete itself, the .map method is also unreliable like below:

playground

There is a runtime error.

[RyanCavanaugh]

The issue is not getters, the issue is that we don't know that the callback is immediately executed. An isomorphic example is this:

function mapLater(arr: any[], cb: (x: any) => void) {
    window.setTimeout(() => arr.map(cb), 100);
}

declare const o: {
    records?: string[]
}

if (o.records) {
    mapLater(o.records, () => o.records.length);
}
o.records = undefined;

which would crash

[massimonewsuk]

@zheeeng @RyanCavanaugh Yep I understand.

I was thinking more along the lines of:

if (x !== undefined) {
    // x is definitely not undefined
}

versus

if (x.a !== undefined) {
    // x.a might be undefined
}

It "feels logical" to me that a function expression would assume the types of closure variables based on what they are determined to be at the time of the function expression's creation, but as @RyanCavanaugh says this isn't always the case (the following example fails in TS):

function foo(records: any[] | null) {
    if (records) {
        records.map((record, i) => record + (records.length === i) ? '' : ',')
    }
    records = null;
}

[RyanCavanaugh]

If you captured o.records into a const then there'd be no error because we know it's not mutated later. See the linked issue above for much more details.