Skip to content

Properties of instances of anonymous classes have type <any> after an instanceof guard. #17253

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
pbazant opened this issue Jul 17, 2017 · 7 comments
Labels
Bug A bug in TypeScript
Milestone

Comments

@pbazant
Copy link

pbazant commented Jul 17, 2017

TypeScript Version: 2.4.1

Code

function create_my_class<T>() {
    return class {
        value: T;
        constructor(
            value: T
        ) {
            this.value = value;
        }
    }
}
const MyClassWithNumber = create_my_class<number>();
const with_number = new MyClassWithNumber(234);

with_number.value = 10;
// with_number.value = "sdf"; // error, and rightly so

const something: any = with_number;

if(something instanceof MyClassWithNumber) {
    something.value = 20;
    something.value = "sdf"; // not an error, but should be one
}

Expected behavior:
The last assignment should be marked as a type error by TS.

Actual behavior:
The last assignment is not marked as error. This is somewhat surprising, as the line

with_number.value = "sdf";

is righteously reported as a type error by TS.

Not a huge issue, but prevented me from using a class factory.

@ikatyang
Copy link
Contributor

It seems working as intended to me (the behavior of instanceof), since something instanceof MyClassWithNumber can only ensure something is an instance of the Anonymous class, there is no types at runtime (instanceof), thus TS just narrow something to Anonymous class<any> (T -> any), which is the actual behavior, though it'll cause implicit any.

If you still considered this is not the expected behavior, you may have to use something like this:

function myInstanceOf<T>(obj: any, ctor: new (...args: any[]) => T): obj is T {
    return obj instanceof ctor;
}
if(myInstanceOf(something, MyClassWithNumber)) {
    something.value = 20;
    something.value = "sdf";
//  ^^^^^^^^^^^^^^^ [ts] Type '"sdf"' is not assignable to type 'number'
}

NOTE: TypeScript is a structural type system, which means

class A {
  v: string;
}
class B {
  v: string;
}

const a = new A();
const b: B = a; // passed, since they're in the same shape

For nominal type matching, see FAQ and #202.

@pbazant
Copy link
Author

pbazant commented Jul 18, 2017

Thank you for your reply. I still have the impression that it would not be an error if TS narrowed the type without the myInstanceOf wrapper.
Quoting from the docs regarding instanceof type guards:

The right side of the instanceof needs to be a constructor function, and TypeScript will narrow down to:

  1. the type of the function’s prototype property if its type is not any
  2. the union of types returned by that type’s construct signatures in that order.

(emphasis added)
N.B. that TS does narrow the type when the function producing the anonymous class is not generic:

function create_my_class() {
    return class {
        value: number;
        constructor(
            value: number
        ) {
            this.value = value;
        }
    }
}
const MyClassWithNumber = create_my_class();
const with_number = new MyClassWithNumber(234);

with_number.value = 10;
// with_number.value = "sdf"; // error, and rightly so

const something: any = with_number;

if(something instanceof MyClassWithNumber) {
    something.value = 20;
    // something.value = "sdf"; // an error, as expected
}

So the type of the property seems only to be forgotten (widened to any) when the class ctor is produced by a generic function and there is a "type roundtrip" involving a cast to any followed by an instanceof type guard.

The generic function myInstanceOf compiles to a function that trivially forwards the arguments to the JS instanceof operator, so i'd expect that TS could safely apply the narrowing logic on its own without the need to be taught it by means of myInstanceOf.

In any case, thank you for the myInstanceOf workaround, it works nicely.

@kevincox
Copy link

I ran into this and was incredibly surprised. This is a dangerous way for any to get into your code invisibly. instanceof is usually a very safe way to type check as it has a reliable runtime check but this makes it unsafe to use on any type with generic parameters. I would expect that instanceof only allows the minimum but instead it uses any.

class Generic<T> {
	constructor(public value: T) {}
}

class Specific<T extends number> {
	constructor(public value: T) {}
}

function example(v: unknown) {
	if (v instanceof Generic) {
		let _: undefined = v.value; // Allowed!
		// Expected v.value to be of type `unknown`.
	}
	if (v instanceof Specific) {
		let _: undefined = v.value; // Allowed!
		// Expected v.value to be of type `number`.
	}
};

Basically TypeScript is far too generous here. I would expect it to assume the minimum bound (unknown and number in these cases) but it assumes the upper bound!

This would be a breaking change so the default probably can't be change but I would definitely like to see a compiler option to do the safer thing in this case. (I looked for this option before finding this issue, I'm surprised it doesn't exist).

@joshuakb2
Copy link

I would also appreciate a compiler option to improve this behavior. Right now I'm working around the issue by using a type guard function instead of the instanceof keyword. For example, and to extend @kevincox's code example above:

const isGeneric = (x: unknown): x is Generic<unknown> => x instanceof Generic;
const isSpecific = (x: unknown): x is Specific<number> => x instanceof Specific;

function example2(v: unknown) {
	if (isGeneric(v)) {
		let _: undefined = v.value; // Errors now
	}
	if (isSpecific(v)) {
		let _: undefined = v.value; // Errors now
	}
};

@joeedh
Copy link

joeedh commented Nov 26, 2024

Is there any interest in a PR implementing a compiler option as discussed? Is it possible for a dev to chime in?

class A<param = something>
const DesiredA = A as {new: () => A};
const a: any = new A();

// we want this inference:
if (a instanceof A) { //infers to A<any>
}

//to equal this one:
if (a instancoef DesiredA) { //infers to A<something>
}

@joeedh
Copy link

joeedh commented Nov 26, 2024

I think this is the code that'd need to change, from 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.
        // It is an error to explicitly declare a static property member with the name 'prototype'.
        const classType = getDeclaredTypeOfSymbol(getParentOfSymbol(prototype)!) as InterfaceType;
        return classType.typeParameters ? createTypeReference(classType as GenericType, map(classType.typeParameters, _ => anyType)) : classType;
    }

Could be:

    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.
        // It is an error to explicitly declare a static property member with the name 'prototype'.
        const classType = getDeclaredTypeOfSymbol(getParentOfSymbol(prototype)!) as InterfaceType;
        return classType.typeParameters ? createTypeReference(classType as GenericType, map(classType.typeParameters, type => type.default ?? anyType)) : classType;
    }

@maximan3000
Copy link

maximan3000 commented Feb 9, 2025

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

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
Bug A bug in TypeScript
Projects
None yet
Development

No branches or pull requests

8 participants