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

Type for constructor function #1409

Closed
vkurchatkin opened this issue Feb 15, 2016 · 3 comments
Closed

Type for constructor function #1409

vkurchatkin opened this issue Feb 15, 2016 · 3 comments

Comments

@vkurchatkin
Copy link
Contributor

It seems that Class type is not covariant:

Given

class A {
  a: string;
  b: string;

  constructor() {
    this.a = 'a';
    this.b = 'b';
  }
}

type AType = {
  a: string
};

This fails: const Clazz: Class<AType> = A;

 18: const Clazz: Class<AType> = A;
                                 ^ property `b` of A. Property not found in
 18: const Clazz: Class<AType> = A;
                        ^^^^^ object type
@jeffmo
Copy link
Contributor

jeffmo commented Feb 15, 2016

Unfortunately this isn't actually intended to work -- but we're giving a poor error message for it :(

Because classes are typed nominally in Flow, this error message should really say that it is invalid to use Class< > on any type that doesn't represent the instance of a class. In other words, because AType is an object type, using it with Class< > would mean generating a type for a specific class-object that produces instances of AType -- which would then be useless (since it exists only in the type system and not at runtime).

Sorry for the confusion here -- I don't think we actually have a way to generically specify structural class types. Maybe you can go into your use case a bit so we can understand it better?

@vkurchatkin
Copy link
Contributor Author

@jeffmo I see, I probably misunderstood what Class actually does. There is probably a problem with the docs, then, as they state the following:

Class<T> Describes the type of the class object that would instantiate an instance of T

It should probably say "instance of class T"

Although it's weird, cause if I add b:string to AType this code actually type-checks.

Maybe you can go into your use case a bit so we can understand it better?

It's simple: I want to have a map of constructors that produce objects of type T, so that I can implement a factory that picks a constructor, calls it and returns value of type T.

To be precise, I don't need class type, I need a constructor type, i.e. a function that can be called with new, but I don't see a way to declare those as well.

I started with the following:

const map: { [key: string]: Class<MyType> } = { foo: MyConstructor }

Since it didn't work, I ended up wrapping constructors with functions:

const map: { [key: string]: (arg: any) => MyType } = { foo: arg => new MyConstructor(arg) }

It kind of makes sense, since Class<T> doesn't indicate in any way what arguments should be passed to constructor.

@jeffmo
Copy link
Contributor

jeffmo commented Feb 15, 2016

It should probably say "instance of class T"

Good point, I'll fix this wording. Thanks

Although it's weird, cause if I add b:string to AType this code actually type-checks.

Yes, this is actually a bug in Flow. We've known about it for a bit, but it's been so obscure that we haven't gotten to it yet.

I want to have a map of constructors that produce objects of type T, so that I can implement a factory that picks a constructor, calls it and returns value of type T.

This makes a lot of sense, thanks. I'll log this as a feature request

@vkurchatkin vkurchatkin changed the title Class is not covariant Type for constructor function Jun 17, 2016
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Mar 23, 2022
…eturn

Here's Flow's built-in definition for the `Intl` object [1]:

  declare var Intl: {
    Collator: Class<Intl$Collator>,
    DateTimeFormat: Class<Intl$DateTimeFormat>,
    NumberFormat: Class<Intl$NumberFormat>,
    PluralRules: ?Class<Intl$PluralRules>,
    getCanonicalLocales?: (locales?: Intl$Locales) => Intl$Locale[],
    ...
  }

(It uses Flow's built-in `Class` type:
https://flow.org/en/docs/types/utilities/#toc-class )

So, for Intl, the names with $ stand for instance types, and the
names with . stand for class types.

It looks like the touched lines of code really want to say something
like

   getDateTimeFormat: ConstructorOf<Intl.DateTimeFormat>

, where `ConstructorOf` is a name I just made up, and Flow doesn't
yet have a tool that would fit that name; see
  facebook/flow#1409 (comment)

Without that, it's at least easy to preserve the return value of
each of these functions. As with any constructor function, the
return value is an instance object, not the class object, which is
why we make this change.

[1] https://github.com/facebook/flow/blob/v0.141.0/lib/intl.js#L8-L15
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Mar 25, 2022
…eturn

Here's Flow's built-in definition for the `Intl` object [1]:

  declare var Intl: {
    Collator: Class<Intl$Collator>,
    DateTimeFormat: Class<Intl$DateTimeFormat>,
    NumberFormat: Class<Intl$NumberFormat>,
    PluralRules: ?Class<Intl$PluralRules>,
    getCanonicalLocales?: (locales?: Intl$Locales) => Intl$Locale[],
    ...
  }

(It uses Flow's built-in `Class` type:
https://flow.org/en/docs/types/utilities/#toc-class )

So, for Intl, the names with $ stand for instance types, and the
names with . stand for class types.

It looks like the touched lines of code really want to say something
like

   getDateTimeFormat: ConstructorOf<Intl.DateTimeFormat>

, where `ConstructorOf` is a name I just made up, and Flow doesn't
yet have a tool that would fit that name; see
  facebook/flow#1409 (comment)

Without that, it's at least easy to preserve the return value of
each of these functions. As with any constructor function, the
return value is an instance object, not the class object, which is
why we make this change.

[1] https://github.com/facebook/flow/blob/v0.141.0/lib/intl.js#L8-L15
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