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.

Sign up for GitHub

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

docs(README.md): note that Flow's Class<T> works with type parameter but TS typeof T doesn't #62

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

docs(README.md): note that Flow's Class<T> works with type parameter but TS typeof T doesn't #62

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 20 additions & 5 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,13 +291,28 @@ const instance = new Test();
type TestTypeFromInstance = Class<typeof instance>;
```

This works with type parameters:
```js
function newInstance<T>(c: Class<T>) {
...
}
```

### TypeScript

```ts
class Test {};
type TestType = typeof Test;
```

This unfortunately doesn't work with type parameters:
```ts
// error: 'T' only refers to a type, but is being used as a value here.(2693)
function newInstance<T>(c: typeof T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But you can:

declare function newInstance<T>(c: new (..._: any) => T): T;

var x = newInstance(class X { y = 1; });
x.y = 2

Copy link
Contributor Author

@jedwards1211 jedwards1211 Nov 18, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I could include that, but I bet it has some limitations like not having type information about the static members of the class, right? I'm 95% sure Class<T> in Flow includes the static member types

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, It's not clear how you want to use it. Flow has some limitations too, it's not like that in all scenarios you just get the statics part with this annotation. It's still a generic, (but the last Flow version I've used is 0.85). But, TS is anyway not the best in inference of static properties and even when it's suppose to have the information.

declare class Hi {
    static sss(): number;
    iii(): string;
}

var h = new Hi();

h.constructor.sss();
//            ^^^ <- Error in TS - Works in Flow

Copy link
Contributor Author

@jedwards1211 jedwards1211 Nov 21, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This case works in Flow at least:

declare class Hi {
    static sss(): number;
    iii(): string;
}

function foo<T: Hi>(cls: Class<T>) {
	cls.sss() // no error
        cls.aaa() // Cannot call `cls.aaa` because property `aaa` is missing in statics of `Hi`
}

I'm just saying if we include a note about new (..._: any) => T types, we should mention that it definitely doesn't capture static member types, but that Class<T> can.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the moment you add : in the generics it's become pointless. It's not a "real life usage". Because I would probably write it as:

declare class Hi {
    static sss(): number;
    iii(): string;
}


function foo(cls: typeof Hi) {
    cls.sss() // no error
    cls.aaa() // Cannot call `cls.aaa` because property `aaa` is missing in statics of `Hi`
}

I think that this demo is quite pointless. You better right about the fact that TS is really weak about static fields and you cannot get them through constructor, aka:

const h = new Hi();
h.constructor.sss(); // <- Error, doesn't know `sss` exists

But Class<T> utility isn't that different than typeof. The fact TS doesn't recognize static fields is the real different.
If you wouldn't use : in the demo, only if a function isn't exported it might work and than you wouldn't need any type annotations (this is where Flow shines).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • I didn't talk about TS vs Flow. I've gave examples where Flow is better than TS
  • Overtype, wasn't said about comparing to "losing needed type information", was about doing sophisticated types that aren't give any type safety, only use-less meta data
  • I don't say sequelize definitions are over-typed (By the way I guess in TS it would done very differently). I said giving overtype examples for non-real life senecrious isn't good a thing.

The main point, I think Class<T> vs typeof T isn't a good example, because it's misleading and unclear to the point, But showing TS can't handle static properties or "prototype chain" is way more relevant while Flow does it

Copy link
Contributor Author

@jedwards1211 jedwards1211 Nov 23, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I'm just having trouble understanding why you're against my proposed changes. Would you be in agreement with something like

The TypeScript equivalent of Class<T> is typeof T in cases where T is a value, and new (..._: any) => T in cases where T is a type; however, unlike Class<T>, new (..._: any) => T doesn't capture static class member type information.

I should research whether typeof T captures static class member type information either though (though if it doesn't already, I would expect typeof T to capture static members in the future, whereas I assume new (..._: any) => T will never capture static members.)
Or can you propose how you would describe the differences in typing classes, including typing static members?

Copy link
Contributor Author

@jedwards1211 jedwards1211 Nov 23, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay for reference with latest typescript playground:

class Foo {
  static bar: number = 2
}

const Typeof: typeof Foo = Foo
const a: string = Typeof.bar

const New: (new (..._: any) => Foo) = Foo
const b: string = New.bar
const a: string
Type 'number' is not assignable to type 'string'.(2322)

Property 'bar' does not exist on type 'new (..._: any) => Foo'.(2339)

So it's not correct to say that TS can't handle static properties; it certainly does with typeof, it just can't do so on a generic type, and this is a concrete difference from Flow I think it would benefit people to know.

So I think anything we document about differences in class typing must discuss both typeof T and new (..._: any) => T, the differences between each, and the fact that Flow can capture the class static member types even when the T in Class<T> is a type parameter.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TS don't deal statics as part of prototype object well. typeof SomeClass will work as much as typeof anyInstanceObject. But anyInstanceObject.prototype.constructor.* won't work in TS, Only in Flow.

I didn't say I'm against, I'm not a maintainer of this project and don't decide what goes in or not.

I think it's misleading to present it as "Class<T> vs typeof T". It's not the important part for my opinion and quite misleading. Talking about limitation to extract static properties from TS comparing to Flow is way more important and accurate

Copy link
Contributor Author

@jedwards1211 jedwards1211 Nov 26, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's very important for people migrating from Flow to TS to learn what to use instead of Class<T>. They need to know that they won't be able to fully replicate Class<T> in some situations.

I get that typeof T is not the only answer, my TS section needs to mention both typeof T and new (..._: any) => T to provide a complete answer. You may only use new (..._: any) => T but it's not necessarily sufficient for everyone's purposes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But you can:

declare function newInstance<T>(c: new (..._: any) => T): T;

var x = newInstance(class X { y = 1; });
x.y = 2

...
}
```

## Keys/Props Of Type

### Flow
Expand Down Expand Up @@ -405,7 +420,7 @@ These are functions that return a boolean, performing some logic to assert that

The implementations differ between Flow and TypeScript:

In TypeScript, it ensures the mapping between: `true` and `value is T`, versus in the case of Flow, it ensures the value is "checked" against the logic within the body of the function (i.e. things like `typeof`, `instanceof`, `value === undefined`).
In TypeScript, it ensures the mapping between: `true` and `value is T`, versus in the case of Flow, it ensures the value is "checked" against the logic within the body of the function (i.e. things like `typeof`, `instanceof`, `value === undefined`).

This means you cannot tell Flow that the tested parameter is of an arbitrary type, which closes the door to complex cases, e.g.:
- reusing logic from a different function
Expand Down Expand Up @@ -654,7 +669,7 @@ type C = Omit<A, B>;
// C is { b: number }
```

However, Flow implementation is stricter in this case, as B have a property that A does not have, it would rise an error. In Typescript, however, they would be ignored.
However, Flow implementation is stricter in this case, as B have a property that A does not have, it would rise an error. In Typescript, however, they would be ignored.

# Same syntax

Expand Down Expand Up @@ -683,8 +698,8 @@ This is supported by Flow. And we list out the different syntaxes here: [Try Flo

```js
type F = {
(): string,
[[call]]: (number) => string,
(): string,
[[call]]: (number) => string,
[[call]](string): string
}

Expand Down Expand Up @@ -719,7 +734,7 @@ Reference:

- [Callable Objects](https://flow.org/en/docs/types/functions/#callable-objects-)
- [immer.js](https://github.com/immerjs/immer/blob/master/src/immer.js.flow) uses it to overload the `produce` (default export) function which has multiple call signatures
- [Styled Components](https://github.com/flow-typed/flow-typed/blob/master/definitions/npm/styled-components_v4.x.x/flow_v0.75.x-/styled-components_v4.x.x.js#L242) uses it to separate cases of being called on a string and wrapping a component
- [Styled Components](https://github.com/flow-typed/flow-typed/blob/master/definitions/npm/styled-components_v4.x.x/flow_v0.75.x-/styled-components_v4.x.x.js#L242) uses it to separate cases of being called on a string and wrapping a component
- [Reselect Library Definition](https://github.com/flow-typed/flow-typed/blob/master/definitions/npm/re-reselect_v2.x.x/flow_v0.67.1-/re-reselect_v2.x.x.js) contains massive chunks of overloaded call properties

### TypeScript
Expand Down