Skip to content

Infer constrained generic parameters after instanceof check #17473

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

Open
randombk opened this issue Jul 28, 2017 · 8 comments · May be fixed by #58028
Open

Infer constrained generic parameters after instanceof check #17473

randombk opened this issue Jul 28, 2017 · 8 comments · May be fixed by #58028
Labels
Fix Available A PR has been opened for this issue In Discussion Not yet reached consensus Suggestion An idea for TypeScript

Comments

@randombk
Copy link
Contributor

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.

@RyanCavanaugh RyanCavanaugh added the Bug A bug in TypeScript label Jul 28, 2017
@mhegazy mhegazy added Suggestion An idea for TypeScript In Discussion Not yet reached consensus and removed Bug A bug in TypeScript labels Nov 9, 2017
@thw0rted
Copy link

I started to file a new bug for this because I couldn't find the right search terms. I made a reproduction on Playground 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
Copy link
Contributor

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
Copy link
Contributor

jtbandes commented Apr 29, 2021

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
Copy link
Contributor

molisani commented Nov 3, 2021

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
Copy link

I think this is the same as #17253. However the other issue has a more accurate description of the problem.

@jcalz
Copy link
Contributor

jcalz commented Dec 18, 2024

Note that the constraint is only appropriate if it's a covariant type parameter. If it's contravariant then never is appropriate, and otherwise any is probably the best we can do (since we don't have existential types).

@OliverJAsh OliverJAsh marked this as a duplicate of #60810 Dec 18, 2024
@molisani
Copy link
Contributor

Note that the constraint is only appropriate if it's a covariant type parameter. If it's contravariant then never is appropriate, and otherwise any is probably the best we can do (since we don't have existential types).

My current proposed fix has the following logic:

  • If contravariant or bivariant, infer never
  • Otherwise, use constraint type
  • If no constraint type was specified, fallback to unknown

The PR also assumes that this logic will be gated behind a new compiler option (which may not be the case). If this logic is disabled then it will infer any for all of these cases.

@maximan3000
Copy link

Found temporary solution, which works fine for me - creating duplicated non-generic class with inheritance

For the topmost issue comment it looks like this

interface Foo {
    foo: string;
}

class Bar<T extends Foo> {
    constructor(readonly bar: T) {}
}
// creating a concrete class
class Temp extends Bar<Foo> {}
// taking type of that class to fix inference in generic class
// inside module - export BarProxy const instead of Bar class
const BarProxy = Bar as typeof Temp;

let a: unknown = new Bar({foo: 'foo'});
const barProxy: unknown = new BarProxy({foo: 'foo'});

console.log(a instanceof Bar);
console.log(a instanceof BarProxy);
console.log(barProxy instanceof Bar);
console.log(barProxy instanceof BarProxy);

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

Link to sandbox

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fix Available A PR has been opened for this issue In Discussion Not yet reached consensus Suggestion An idea for TypeScript
Projects
None yet