instanceof typeguard fails if classes have similar structure

[basarat]

TypeScript Version:

1.8.x and nightly

Code

class C1 { item: string }
class C2 { item: string[] }
class C3 { item: string }

function Foo(x: C1 | C2 | C3): string {
    if (x instanceof C1)
        return x.item;
    else if (x instanceof C2)
        return x.item[0];
    else if (x instanceof C3)
        return x.item; // Error property item does not exist on C2
}

Expected behavior:
Code should compile

Actual behavior:
Code has an error as shown

More

The following works i.e. if C1 and C3 differ in structural compatibility:

class C1 { item: string }
class C2 { item: string[] }
class C3 { items: string }

function Foo(x: C1 | C2 | C3): string {
    if (x instanceof C1)
        return x.item;
    else if (x instanceof C2)
        return x.item[0];
    else if (x instanceof C3)
        return x.items; // Ok
}

🌹

[ivogabe]

I'm afraid this is by design. I get a lot trouble with this too, so I hope this could be changed. Adding a brand to one of the classes fixes it, but I don't like that approach very much. I think x instanceof C1 should only remove C1 in the else branch, not both C1 and C3, or it shouldn't remove any types in the else branch. The current behavior doesn't match what happens at runtime, for instance this does not give any compile error:

function Foo(x: C1 | C2 | C3): string {
    if (x instanceof C1)
        return x.item;
    else
        return x.item[0];
}

The type of x is C2 in the else block, but at runtime it could also be C3.

[yortus]

I've also run into this problem, and wish that the compiler behaviour could be brought more into line with what happens at runtime. It's not specific to instanceof or classes, it's really just about the structural similarity of the types in the union. E.g., this works fine at runtime, but fails at compile time:

type UnaryFunction = (a: any) => any;
type BinaryFunction = (a: any, b: any) => any;
function isUnaryFunction(fn: Function) : fn is UnaryFunction {
    return fn.length === 1;
}
function isBinaryFunction(fn: Function) : fn is BinaryFunction {
    return fn.length === 2;
}
function foo(fn: UnaryFunction | BinaryFunction) {
    if (isBinaryFunction(fn)) {
        fn(1, 2);   // OK: fn is BinaryFunction here
    } else {
        fn(1);      // ERROR: Cannot invoke an expression whose type lacks a call signature
    }
}

I think the problem is similar to @basarat's, because from a structural typing viewpoint, a UnaryFunction is just a subtype of a BinaryFunction, triggering different narrowing behaviour than if the types were structurally independent. However there is no subtype relationship according to the runtime checks in isUnaryFunction and isBinaryFunction.

---

Here is another case that brings up this type guard behaviour because the compiler sees the types as structurally related. Consider a function that takes either (a) an options object, where all options are optional, or (b) a function that returns an options object:

interface OptionsObject {
    option1?: string;
    option2?: number;
}
interface OptionsFunction {
    (): OptionsObject;
}
function isOptionsObject(opts: OptionsObject | OptionsFunction) : opts is OptionsObject {
    return opts && typeof opts === 'object'; // definitely not a function
}
function bar(opts: OptionsObject | OptionsFunction) {
    let option1: string;
    if (isOptionsObject(opts)) {
        option1 = opts.option1 || 'none';   // ERROR: no option1 on OptionsObject|OptionsFunction
    } else {
        option1 = opts().option1 || 'none'; // OK
    }
}

This also works at runtime but fails at compile time. The compiler sees OptionsFunction as just a special case of OptionsObject, because structurally it is. But it is not a subtype according to the runtime check in the type guard.

---

I have learned how to spot and work around these cases now. But that involves taking valid runtime code, and rearranging it just right so the compiler won't complain. It's a (rare) case where the tool is fighting me rather than helping me. Probably also quite unintuitive for beginners.

[yortus]

@basarat here is a version of your example without instanceof or classes. It has the same error as your example, which is why I think this is about the structural type similarity and not related to the presence of instanceof or classes:

interface C1 { item: string }
interface C2 { item: string[] }
interface C3 { item: string }

function isC1(c: C1 | C2 | C3): c is C1 { return /*some test*/ }
function isC2(c: C1 | C2 | C3): c is C2 { return /*some test*/ }
function isC3(c: C1 | C2 | C3): c is C3 { return /*some test*/ }

function Foo(x: C1 | C2 | C3): string {
    if (isC1(x))
        return x.item;
    else if (isC2(x))
        return x.item[0];
    else if (isC3(x))
        return x.item; // ERROR
}

[yortus]

Another example, this time with typeof, that's maybe the same problem? Since Function is a subtype of Object. But isn't string also a special case of Object structurally? Not sure about this one...

function ok(x: string | Object) {
    if (typeof x === 'string') {
        x // string
    }
    else {
        x // Object
    }
    if (typeof x === 'object') {
        x // Object
    }
    else {
        x // string
    }
}


function fail(x: Function | Object) {
    if (typeof x === 'function') {
        x // Function | Object
    }
    else {
        x // Function | Object
    }
    if (typeof x === 'object') {
        x // Function | Object
    }
    else {
        x // Function | Object
    }
}

[falsandtru]

+1 this behavior is buggy.

[falsandtru]

from #6589 because it merged into this issue.

---

Type narrowing strategy for most closest type selection

Narrowing to the closest runtime type by instanceof operator:

class A<T> {
    prop: T;
}
class B<T> extends A<T> {
}
class C extends B<any> {
}

var x: A<string> | B<any> | C;
if (x instanceof A) {
    x; // closest type is A, now B
}
if (x instanceof B) {
    x; // closest type is B, now B
}
if (x instanceof C) {
    x; // closest type is C, now B
}
if (x instanceof Object) {
    x; // closest type is A, now B
}
if (x instanceof Array) {
    x; // no closest type, must be contextual type `A<string> | B<any> | C`
}

Motivation

Sometimes we must check the instance type instead of pattern matching. TypeScript should provide the way that select the most closest type for alternate method of pattern matching.

// maybe monad
  public bind<U>(f: (val: T) => Maybe<U>): Maybe<U> {
    return new Maybe<U>(() => {
      const m: Just<T> | Nothing | Maybe<T> = this.evaluate();
      if (m instanceof Just) {
        return f(m.extract());
      }
      if (m instanceof Nothing) {
        return m;
      }
      if (m instanceof Maybe) {
        return (<Maybe<T>>m).bind(f); // `m` is `Nothing | Maybe<T>`, should be `Maybe<T>`
      }
      throw new TypeError(`ArchStream: Maybe: Invalid monad value.\n\t${m}`);
    });
  }

Searching and showing of all derived types is useless and too complex when there are many derived types, and TypeScript should reduce those unnecessary costs for optimization.

class A<T> {
    prop: T;
}
class B<T> extends A<T> {
}
class C extends B<any> {
}

var x: A<string> | B<any> | C;
if (x instanceof A) {
    x; // this scope narrowed by A, B and C are useless, but x is B
}

In general, sets of types must narrow by operations, but instanceof operator widen types. Roles and effects of typings for abstraction is reducing of calculation size based on sets size.

class A<T> {
    a: T;
}
class B<T> extends A<T> {
    b: T;
}
class C extends A<any> {
    c: any;
}

var x: A<string> | B<string> | C;
if (x instanceof A) {
    x; // x should narrow to A of the most closest type from `instanceof` operator specified type.
    // if you want B or C, it should narrow by those types.
}

instanceof operater compare constructors in js engine, but TypeScript compiler compare structural types. It is mismatch behavior.

[mhegazy]

As noted in #8503, classes should be treated definitely for instanceof checks.

note this applies to instanceof type guards only, and not for user defined type guards; the later stays structural as we have no guarantees on how they will be implemented, where as instanceof is known to always be nominal.

[yortus]

Just retried all the examples on this page with the nightly, and they all work except for the UnaryFunction | BinaryFunction one (which I know was not addressed by #10216). Nice that as well as the OP example, the other non-instanceof examples all work now too. CFA is getting very good indeed.

[kitsonk]

I just wanted to mention that I came across a situation where TypeScript does treat classes nominally. We (Dojo 2) have a Promise shim which introduced a protected property named _state. While at run-time we load the same module, when we are developing we will often npm link in development branches of our packages and those packages then import in physically different locations of the .d.ts UMD file from the shim package. Once that occurs, TypeScript then starts to complain that Promise<T> is not assignable to Promise<T> because they are different class instances. We worked around the issue by npm linking in the package to all the other dependent packages, so the class Promise definition was coming from exactly the same physical file location, but I will admit it was surprising to see TypeScript treat it this way, especially with the "TypeScript is structural typing" booming in my head.

I guess the thought there will be quite a few other surprises like that, where for various reasons the typings appear to be nominally distinct but they are actually the same at run-time.

[mhegazy]

private and protected properties are nominal. treating them structurally defies the whole point of visibility modifiers.

[mkusher]

Is this a problem with structural classes or with wrong if-else instanceof?

class Foo {
    foo: string;
}

class Bar {
    bar: string;
}

function test(x: Foo | Bar) {
    if (x instanceof Foo) {
        x.foo.toUpperCase()
    } else { // somewhy x is Bar here
        x.bar.toUpperCase();
    }
}
test({ foo: 'foo' });

[yortus]

@mkusher at runtime, obviously {foo: 'foo'} is not an instance of the Foo class so you end up in the else block.

At compile time, the only questionable thing here is the last line, where the compiler allows the call to test with an argument that is not a Foo or a Bar. That's where structural typing comes into play. AFAIK TypeScript sees no difference in type between new Foo() and {foo: 'foo'} in this case.

[mkusher]

@yortus I understand why it happens so now, but the question is more about what needs to be fixed: type guard(if-else instanceof) or nominal classes

[kitsonk]

@mkusher the core team are looking at a solution for this under #202 (and this ticket). It will be some form of nominal support most likely, but something that doesn't break/incompatible with the rest of the structured typing system.

[mikelehen]

Why is this closed? I've spent the last 20 minutes trying to find the canonical (open) issue on this, but I've failed. There are lots of dupes, but the only open ones (at the moment) I can find are #11664 and #10934. I assume they'll get closed as dupes eventually.

This is the clearest issue I've found so far (best title and description). I see #202 has been referenced a couple times as covering this issue, but it seems to be much broader and it's not obvious if/how it will address the concrete issue reported here where the compiler has chosen to implement "instanceof" different from how it behaves at runtime.

I ask because our code is getting littered with hacks to work around this behavior and I'd like to reference an appropriate issue so that we can easily check if/when it gets addressed.

I'd also love to see a solution for this sooner than later, since the behavior is non-intuitive and the resulting errors are varied and often appear nonsensical.

[kitsonk]

This issue was closed by PR #10216 which the core team felt dealt partially with this issue, enough to close it. For broader nominal type, that is still #202 and is on the Future Roadmap

[mikelehen]

Thanks @kitsonk. I see that the specific issue here does seem to be solved. I've gone ahead and opened new issues for the instanceof cases I'm still hitting.

[drewnoakes]

See #19671 for a recent relevant merged PR.

[bennadel]

I think I just ran into this issue as well on TypeScript 2.6.2. I have a method that looks like this:

function ( target : Window | Element ) : string {
    if ( target instanceof Window ) {
        return( "__window__" );
    } else {
        return( target.tagName );
    }
}

I get Property 'tagName' does not exist on type 'Window'.. It doesn't matter if I change it to be else if ( target instanceof Element ). Seems like what other people are seeing.

[mikelehen]

It'd be useful to know what version you're on, since #19671 fixed a bunch of issues in this area...

[bennadel]

@mikelehen according to my npm package (I'm using TS through ts-loader in WebPack), I'm on versions:

"ts-loader": "3.3.0",
"typescript": "2.6.2",

[RyanCavanaugh]

The fixes are in TypeScript 2.7

[bennadel]

@RyanCavanaugh Ahhh, player. I didn't realize there was a new version. Time for a bit of the old npm outdated :)

[kitsonk]

TypeScript 2.7 is still only an RC at the moment:

$ npm view typescript
{ latest: '2.6.2',
     next: '2.7.0-dev.20180124',
     beta: '2.0.0',
     rc: '2.7.0-rc',
     insiders: '2.7.0-insiders.20180119' }