microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
101.11k stars 12.5k forks source link

Infer constrained generic parameters after instanceof check #17473

Open randombk opened 7 years ago

randombk commented 7 years ago

TypeScript Version: 2.4.0

When a variable passes an instanceof check generic type with constraints on the type parameter, apply to constraints to the variable. There is currently no clean way to handle the following without an explicit cast:

Code

interface Foo {
    foo: string;
}

class Bar<T extends Foo> {
    constructor(readonly bar: T) {}
}

let a: any;
if (a instanceof Bar) {
    a.bar; // <-- a.bar should be 'Foo' instead of 'any'
}

Expected behavior:

a.bar is inferred as an instance of Foo.

Actual behavior:

a.bar is any

This change would only impact type checking, and will not affect compilation output.

thw0rted commented 5 years ago

I started to file a new bug for this because I couldn't find the right search terms. I made a [reproduction on Playground](https://www.typescriptlang.org/play/#src=class%20Bar%20%7B%0D%0A%20%20%20%20public%20num%3A%20number%3B%0D%0A%7D%0D%0A%0D%0Aclass%20Foo%3CT%20extends%20Bar%3E%20%7B%0D%0A%20%20%20%20public%20bar%3A%20T%3B%0D%0A%20%20%20%20public%20num%3A%20number%3B%0D%0A%7D%0D%0A%0D%0Alet%20f1%20%3D%20new%20Foo%3CBar%3E()%3B%0D%0Af1.num%20%3D%20%22%22%3B%0D%0Af1.bar.num%20%3D%20%22%22%0D%0A%0D%0Alet%20f2%3A%20any%20%3D%20f1%3B%0D%0Aif%20(f2%20instanceof%20Foo)%20%7B%0D%0A%20%20%20%20f2.num%20%3D%20%22%22%0D%0A%20%20%20%20f2.bar.num%20%3D%20%22%22%0D%0A%7D%0D%0A) to show that this is still an issue -- note that 3 of the 4 assignments to .num flag as errors, correctly, but the last assignment is not flagged because the type of f2.bar is any.

I think Ryan was right, and it is in fact a Bug, not a Suggestion. This tripped me up today while refactoring, when I changed

class A {
  public b: B;
}

to

class A<T extends B = B> {
  public b: T;
}

This should be a drop-in replacement, but it's not. Consider

if (a instanceof A) {
  a.b.c().then(x => ... );
}

With the original definition of A, a.b is of type B, so I know that a.b.c() returns Promise<C>, so I know that the argument x is of type C. After my refactor, a.b is of type any, even though the definition of A says a.b must extend B, so a.b.c is of type any and returns any, meaning I'm calling an any when I write then(), so I have no call signature to help the compiler infer the argument type for x. This results in a compiler error if I have strict any-checking turned on. So now my "drop-in replacement" just generated a compiler error -- unless I misunderstand something, that's a bug.

HolgerJeromin commented 5 years ago

This is IMO a bug or design limitation.

declare class myClass<ST = any>{
  public read<T = ST>(): T;
}

let aClass = new myClass<string>();
if (aClass instanceof myClass) {
  let result1 = aClass.read(); // any and not string
} else {
  aClass; // never
}
jtbandes commented 3 years ago

I ran into this today as well. The three.js typings have:

export class Mesh<
    TGeometry extends BufferGeometry = BufferGeometry,
    TMaterial extends Material | Material[] = Material | Material[]
> extends Object3D {
    ...
    material: TMaterial;
}

My code is:

if (obj instanceof THREE.Mesh) {
  if (Array.isArray(obj.material)) {
    // obj.material should be Material[], but it's any[]
  }
}
molisani commented 3 years ago

excerpt from narrowTypeByInstanceof in src/compiler/checker.ts

const prototypeProperty = getPropertyOfType(rightType, "prototype" as __String);
if (prototypeProperty) {
// Target type is type of the prototype property
const prototypePropertyType = getTypeOfSymbol(prototypeProperty);
if (!isTypeAny(prototypePropertyType)) {
targetType = prototypePropertyType;
}
}

The implementation of narrowTypeByInstanceof narrows to the type of the prototype property of the right hand side. It appears that for class MyClass<T extends number> {} the type of MyClass.prototype is set to MyClass<any> by default.

excerpt from getTypeOfPrototypeProperty in src/compiler/checker.ts

function getTypeOfPrototypeProperty(prototype: Symbol): Type {
// TypeScript 1.0 spec (April 2014): 8.4
// Every class automatically contains a static property member named 'prototype',
// the type of which is an instantiation of the class type with type Any supplied as a type argument for each type parameter.

Given that this is appears to be part of the TypeScript v1.0 spec, it feels unlikely that this can be changed without causing other issues. That being said, it does look like a compiler change to match the requested behavior would be fairly easy to implement. (Would be more than happy to contribute such a change, if it makes sense)

kevincox commented 1 year ago

I think this is the same as https://github.com/microsoft/TypeScript/issues/17253. However the other issue has a more accurate description of the problem.