Skip to content
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

Flow not checking new C when C is type Class<...> #2930

Open
karelbilek opened this issue Dec 1, 2016 · 4 comments
Open

Flow not checking new C when C is type Class<...> #2930

karelbilek opened this issue Dec 1, 2016 · 4 comments

Comments

@karelbilek
Copy link
Contributor

karelbilek commented Dec 1, 2016

I have this code and I would expect it to throw error; but it doesn't.

type Foo = {
    constructor(a: number): void;
};

function construct(FooClass: Class<Foo>) {
   const w = new FooClass('this is a string');
}

Similarly, this doesn't check errors

type Foo<T> = {
    constructor(): void;
};

function construct(FooClass: Class<Foo<number>>) {
   const w: Foo<string> = new FooClass();
}

I found no other way to say 'this variable is a class, you can call new() on it, only with the expected parameters, and it makes a new object of that type'. Maybe Class is not correct, but then what is; $Type and friends doesn't work either.

I am needing this, because I want to write at least somehow correct immutable-js Record typedefs; var ABRecord = Record({a: 1, b: 2}) doesn't create a new record, but a new class, on which you can call new ABRecord({}). I want to capture that.

@gcanti
Copy link

gcanti commented Dec 2, 2016

var ABRecord = Record({a: 1, b: 2}) doesn't create a new record, but a new class, on which you can call new ABRecord({}). I want to capture that

Maybe something like this

declare class RecordClass<T> {
  constructor(o: $Shape<T>): this;
}
declare function Record<T: Object>(o: T): Class<RecordClass<T>>;

const ABRecord = Record({a: 1, b: 2})
// or const ABRecord: Class<RecordClass<{a: number, b: number}>> = Record({a: 1, b: 2})
const a = new ABRecord({})

@karelbilek
Copy link
Contributor Author

karelbilek commented Dec 2, 2016

Hm. How come your example typechecks correctly and my example doesn't? :)

Oh, that's because I have type Foo and you have declare class! That must be it. Thanks!

@karelbilek
Copy link
Contributor Author

Once I edit the examples to use declare class instead of type, it works as expected

declare class Foo {
    constructor(a: number): void;
};

function construct(FooClass: Class<Foo>) {
   const w = new FooClass('this is a string');
}

Bam, it throws an error like it should.

I will close this.

@karelbilek karelbilek reopened this Dec 2, 2016
@karelbilek
Copy link
Contributor Author

Oh, the problem was because Record cannot extend T. So I cannot write

declare class Foo extends T {
    constructor(a: number): void;
};

So I tried to "hack" it by using types

declare type Foo = T & {
    constructor(a: number): void;
};

but then the Class<Foo> doesn't check the constructor.

In the end I will probably end up writing some typed wrapper around Records.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants