'instanceof' changes type outside of 'if' statement

[ajafff]

TypeScript Version: 3.4.*

Search Terms:

instanceof

Code

// @strictNullChecks: true
interface OnChanges {
    onChanges(changes: Record<string, unknown>): void
}
interface Validator {
    validate(): null | Record<string, unknown>;
}

class C {
    validate() {
        return {}
    }
}
function foo() {
    let v: Validator & Partial<OnChanges> = null as any;
    if (v instanceof C) {
    }
    if (v.onChanges) { // error here
        v.onChanges({});
    }
}

Expected behavior:

No error.

Actual behavior:

Property 'onChanges' does not exist on type 'C | (Validator & Partial<OnChanges>)'.
  Property 'onChanges' does not exist on type 'C'.

instanceof shouldn't change the type of the variable outside of the if statement.

This only happens with strictNullChecks enabled.

Playground Link: https://typescript-play.js.org/#code/JYOwLgpgTgZghgYwgAgPIgMIAs4gOYQDOyA3gFDKXID2mO+RAFAvQYQFzIBKEC1UAEwA8hMFFB4ANMgCuIANYhqAdxAA+AJScAbtWACyAXzKhIsRCgBqcADb64YfqQpVtt+5EZbkIGTZvIAD7cvPzCouL40nKKKuoA3EZkZAg2cITEGM5UyG52Ag4QXtk5VFAQYDJQIKTGOcbGMHIIYMC0yDDU1MXkOTYVuZzW+Q5OAGTIAApwUK22QujYuGxqyAC8Pn4B6ci4AJ6JOcAwyIzayKCiuEjUJxgaJZR1VMen2gB0tEsMhA+9pZQPl9WEwSIYNIcqA0gA

Related Issues:

[jack-williams]

Related to #30461.

The underlying issue here is that when narrowing using instanceof it checks if the candidate (C) is assignable to the source (Validator & Partial<OnChanges>) which it is, and therefore narrows to the candidate type C.

When combining the narrowing's at v.onChanges is combines the flows of C and Validator & Partial<OnChanges> in a union, which does not collapse as neither are a subtype of the other. Unfortunately, the union with C actually makes the type weaker than you started with.

Instead of narrowing directly to C, it should have probably narrowed using an intersection type - which is the final check of getNarrowedType.

// If the candidate type is a subtype of the target type, narrow to the candidate type.
// Otherwise, if the target type is assignable to the candidate type, keep the target type.
// Otherwise, if the candidate type is assignable to the target type, narrow to the candidate
// type. Otherwise, the types are completely unrelated, so narrow to an intersection of the
// two types.
return isTypeSubtypeOf(candidate, type) ? candidate :
             isTypeAssignableTo(type, candidate) ? type :
               isTypeAssignableTo(candidate, type) ? candidate :
                 getIntersectionType([type, candidate]);

I'm not quite sure what the correct fix is here. A heuristic is that when type is an object with optional properties you probably want to narrow using an intersection, rather than replacing with the candidate.

[weswigham]

The correct fix is to make an intersection even if one of type or candidate is assignable to the other - unions only do subtype reduction, so anything that narrows to a type by assignability (rather than subtype) will have this issue.

[jack-williams]

Want me to submit a PR removing the assignable cases?

[Skillz4Killz]

A similar thing happens with typeof when used outside the if statement with a variable.

This will run without any errors

if (typeof response === 'string' && CANCEL_OPTIONS.includes(response.toLowerCase())) {}

This throws an error

const isString = typeof response === 'string';

if (isString && CANCEL_OPTIONS.includes(response.toLowerCase())) {}

[orta]

This last example is a bit different @Skillz4Killz - you're not narrowing the scope of the variables in the context of that if statement, it's just checking a boolean and TypeScript can't infer that a union has been reduced.

@jack-williams - I think a PR sounds great, if not I'll take a look a this next week 👍

[orta]

This fix was reverted with #41849