Skip to content

instanceof with class-like variable RHS erroneously narrows "else" #13918

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jeffreymorlan opened this issue Feb 7, 2017 · 5 comments
Closed
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@jeffreymorlan
Copy link
Contributor

jeffreymorlan commented Feb 7, 2017

TypeScript Version: 2.2-rc

Code

class Prop<T> {
    arg: T;
}

interface PropClass<T> {
    new(): Prop<T>;
    prototype: Prop<T>;
}

function check<T>(p: Prop<{}>, specialSubclass: PropClass<T>) {
    if (p instanceof specialSubclass) {
        p.arg; // ok, arg has type T
    } else {
        p.arg; // error here: Property 'arg' does not exist on type 'never'.
    }
}

Expected behavior:
p should have type Prop<{}> in the else block.

Actual behavior:
p has type never in the else block.

Because specialSubclass is a class-like object with instance type Prop, TypeScript is assuming that instanceof returning false implies the object is not a Prop. This is a bad assumption, because in this kind of code instanceof only returns true for members of some (unknown) subclass.

instanceof should narrow the false case only if the RHS is an actual class reference. If it's a variable of a class-like type, it should only narrow the true case.

@jeffreymorlan jeffreymorlan changed the title instanceof with non-class RHS erroneously narrows "else" instanceof with class-like variable RHS erroneously narrows "else" Feb 7, 2017
@aluanhaddad
Copy link
Contributor

This works in nightly.

@mhegazy
Copy link
Contributor

mhegazy commented Feb 9, 2017

This works in nightly.

I do not think so.

@mhegazy
Copy link
Contributor

mhegazy commented Feb 9, 2017

The Type system in TS is structural. so the checking for instanceof is rather an approximation to what happens at run time (which is nominal).

you could however, try and trick the compiler by using generic type parameters, these will not be narrowed to never since they are not assignable to the instance of your other argument. e.g.: function check<T, U extends Prop<{}>>(p: U, specialSubclass: PropClass<T>).

@mhegazy mhegazy added the Design Limitation Constraints of the existing architecture prevent this from being fixed label Feb 9, 2017
@jeffreymorlan
Copy link
Contributor Author

jeffreymorlan commented Feb 10, 2017

The Type system in TS is structural. so the checking for instanceof is rather an approximation to what happens at run time (which is nominal).

Right, I understand that due to structural typing {arg:string} is considered the same type as Prop<string>, and so TS acts as if instanceof Prop returns true on it. That's a tad unfortunate but it's actually not the issue here.

My issue is that a value that is (both structurally and nominally) of a class, is getting narrowed away due to instanceof on a subclass returning false. Even if class types were treated 100% nominally this would still be a problem.

If the RHS of instanceof is literally the class A, then it's correct to narrow A away when it returns false (other than the structural/nominal issue). But if it's a variable of type new() => A, it's generally some subclass of A, not A itself - so one can't reasonably assume anything about the object's type from instanceof returning false.

you could however, try and trick the compiler by using generic type parameters, these will not be narrowed to never since they are not assignable to the instance of your other argument. e.g.: function check<T, U extends Prop<{}>>(p: U, specialSubclass: PropClass<T>).

Prop<{}> is already unassignable to Prop<T>. Even though this issue has existed in some form since TS 1.8 (PR 5092), the non-assignability would prevent the false-side narrowing for this particular piece of code, until PR 13230. I don't think it makes any sense that Prop<{}> gets narrowed to Prop<T> on the true side and to never on the false side, or that one should have to make it "even more unassignable" to Prop<T> to prevent that.

@mhegazy mhegazy closed this as completed Apr 21, 2017
@jeffreymorlan
Copy link
Contributor Author

The bug is still present in 2.3rc and in master

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed
Projects
None yet
Development

No branches or pull requests

3 participants