'Property does not exist on type 'never'." after updating to TS 2.0

[aeschli]

After updating https://github.com/Microsoft/vscode-css-languageservice to TS 2.0, I get a lot of errors about accesses to a type never.
They always occur in happen after instanceof checks

export class Node {}
export class Nodelist extends Node {}

function finish<T extends Node>(node: T, error: any): T {
    if (!(node instanceof Nodelist)) {
        node.length = 0; // Property 'length' does not exist on type 'never'."
    }
    return node;
}

More workaround for the same error:
cssParser.ts#L135
cssCompletion.ts#L50
cssHover.ts#L41
selectorPrinting.ts#L311
selectorPrinting.ts#L311

[aluanhaddad]

Don't use empty classes. instanceof is a structural type check at compile time. Your if statement is conditional on the value not being a subtype of a type with no properties.

[decademoon]

Why is instanceof a structural type check at compile time? Take this example:

class A {}
class B extends A {}
class C extends A {}

function foo<T extends A>(t: T): void {
    if (!(t instanceof B)) {
        // t has type never here
        alert(t.constructor.name + ' is not a B');
    }
}

foo(new B());  // No alert displayed
foo(new C());  // Alert displayed

Irrespective of whether or not A, B and C are empty classes, we know that foo(new C()) will display the alert at runtime (and hence t can't be of type never). Why can't typescript deduce this at compile time?

[yortus]

An additional element here is that the compiler narrows unions differently to how it narrows non-unions. For example narrowing a union of two structurally identical types works as expected (fixed by #10216):

class Base {stuff:any}
class A {}
class B {}

function foo(x: A|B) {
    if (x instanceof A) {
        x // x is A here
    }
    else {
        x // x is B here
    }
}

But the OP example doesn't narrow from a union type, it narrows from a base class, which doesn't perform narrowing the same way:

function bar(x: Base) {
    if (x instanceof A) {
        x // x is Base here
    }
    else {
        x // x is never here
    }
}

Could you change your code to use a union type instead of a base class? Otherwise, it's a question for whether non-union type narrowing could be improved.

[aeschli]

Thanks for the tip on the properties. I changed my workaround to use artificial properties.
Adding a union type is not practical, I have a large type hierarchy of multiple levels. It's classic OO programming here, maybe a bit old fashioned, but nevertheless correctly running code.

[jamesandersen]

I'm seeing the same "does not exist on type 'never'" error when using a union type (unlike the OP):

export class SetSymbolAction { constructor(public symbolIndex: number) {} }
export class SetFilingAction { constructor(public filing: Filing) {} }
export class ClearTickersAction { constructor() { } }

export type Action = SetSymbolAction | SetFilingAction | ClearTickersAction;

if (action instanceof SetSymbolAction) {
    action // action is SetSymbolAction here
} else if (action instanceof ClearTickersAction) {
    action // action is ClearTickersAction here
} else if (action instanceof SetFilingAction) {
    action // **action is 'never' here  :-( **
}

Adding a dummy property to ClearTickersAction as follows fixes the issue:

export class ClearTickersAction { constructor(public foo: boolean) { } }

... but is a hack.

It's not entirely clear to me from looking at the referenced issues here whether this is already tracked as a bug; I assume it should be. Is this already tracked?

[mhegazy]

this should work:

if (action instanceof SetSymbolAction) {
..
} else if (action instanceof SetFilingAction) {
..
} else if (action instanceof ClearTickersAction) {
 ..
} 

[yortus]

@jamesandersen probably #202, since TS would need nominal instanceof narrowing to work the way you (reasonably) expect.

[mhegazy]

the reson is the narrowing removes constitents from the type. ClearTickersAction structurally is a super type of SetFilingAction. so narrowing ClearTickersAction out removes both ClearTickersAction and SetFilingAction leaving you with an empty union i.e. never

[jamesandersen]

@mhegazy interesting... that is a cleaner approach; at least until there's another Action variant that is meaningful but has no properties. Thanks for the explanation!