Skip to content

Flow type definition #986

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

Closed
sindresorhus opened this issue Aug 2, 2016 · 17 comments
Closed

Flow type definition #986

sindresorhus opened this issue Aug 2, 2016 · 17 comments
Labels

Comments

@sindresorhus
Copy link
Member

sindresorhus commented Aug 2, 2016

We already have a great TypeScript definition. We should have one for Flow too! Both of these type checkers are awesome and we shouldn't play favorites. This would also be interesting comparison wise.

Anyone familiar with Flow willing to contribute this?

// @thejameskyle

The TypeScript definition is actually partly generated (#884) because it was too tedious to define the chainable test API manually. For example: test.cb.serial.skip(). The properties can be in any order. I wonder if that is needed for Flow too.

@ivogabe
Copy link
Contributor

ivogabe commented Aug 15, 2016

I guess that Flow would also need a generator script, since the parameter type depends on the chain and test.cb.cb should be blocked. I think that the current script can be rewritten to generate the two type definition files of both TypeScript and Flow, then both can be kept in sync. However, can't Flow read TS definition files?

@sindresorhus
Copy link
Member Author

I guess that Flow would also need a generator script, since the parameter type depends on the chain

That really depends on how good the Flow type system is. It might have ways to handle those cases without the TS amount of boilerplate. Would be good to have someone familiar with Flow to comment.

However, can't Flow read TS definition files?

Not yet, and it wouldn't be optimal as Flow is stricter than TypeScript.

@jamiebuilds
Copy link
Contributor

I apologize I did not respond the first time this ended up in my inbox.

I'm not sure what the limitations of TypeScript were that required writing a generation script. Hopefully the same problems do not exist.

As for Flow reading the TS definition files @sindresorhus is right, it would actually be much easier to generate TS definitions from Flow definitions as many of the semantics are intentionally more strict.

I'll look at the AVA API and see if I can start it off for you

@jamiebuilds
Copy link
Contributor

jamiebuilds commented Aug 16, 2016

Okay so I was planning on just playing around with it but you nerd sniped me and I ended up doing the entire type definition.

I was not prepared for how big the generated TypeScript .d.ts file is. I spent a long time trying to deduce what the API limitations were from that API. But then I started exploring the actual implementation in AVA and that was much easier to understand.

One of the things that I thought was unusual with the TypeScript definition you generate is that it reduces the API methods down to act like they can't chain forever despite option-chain supporting the ability to chain infinitely. I ignored that piece because it seemed unimportant and that it would only serve to greatly complicate the type definition, not to mention now it's matching what happens at the runtime.

It also adds support for .always since I noticed that was missing from the TypeScript definition.

Anyways here is the current state of the definition, I'm happy to say it's only like 200 lines compared to the 1,200 line TypeScript definition: https://gist.github.com/thejameskyle/6a0f4d2ef41ed3a7554ce1fa208ae291

I'm sure it has a handful of issues, I haven't really done that much testing of it, I've just been keeping a lot of it in my head.

@SamVerschueren
Copy link
Contributor

SamVerschueren commented Aug 16, 2016

Thanks for looking into this @thejameskyle! Nice to see how they compare. Although I don't think it's a 100% fair comparison :).

One of the things that I thought was unusual with the TypeScript definition you generate is that it reduces the API methods down to act like they can't chain forever despite option-chain supporting the ability to chain infinitely. I ignored that piece because it seemed unimportant and that it would only serve to greatly complicate the type definition, not to mention now it's matching what happens at the runtime.

This is actually the reason why the TypeScript definition is this big. Although test.serial.serial might work, we discussed this when writing the definition and decided to not allow this because it doesn't makes sense. (Almost) all the code from line 175 is to prevent doing things like test.serial.serial and other invalid modifier combinations. Thus TS is much stricter in the test modifiers resulting in such a huge TS file.

@ivogabe
Copy link
Contributor

ivogabe commented Aug 16, 2016

The TS definitions are indeed stricter; besides test.serial.serial it will also disallow things like test.after.before.

TS also supports intersection types and generics, so the definition of Flow can be ported to TS with almost the same amount of lines.

@jamiebuilds
Copy link
Contributor

youkeepusingthatword.jpg

This isn't a matter of strictness, you're effectively enforcing a lint rule with the TypeScript definition which is not the purpose of a type system and just making it impossible to update the definition.

Also test.serial.serial is not an invalid combination. option-chain is designed so that the latter properties in the chain take precedence. All you are doing is setting the same option twice.

@jamiebuilds
Copy link
Contributor

Also, people keep taking this as a comparison between Flow and TypeScript. I wasn't bragging about Flow, I was bragging about my own mad skillz trying to figure out that crazy type definition.

@SamVerschueren
Copy link
Contributor

Also, people keep taking this as a comparison between Flow and TypeScript. I wasn't bragging about Flow

I believe a lot of people assumed that you where bragging about Flow, looking at the questions you received on Twitter :).

After reading your comments, I must admit that I believe you might be right. Type definitions represent the internal/runtime structure of the code, which isn't the case for the TS definition at the moment. So I will open a new issue to discuss this.
Because you are the Flow community manager (is that the right title :p?), you probably know much more about it than us.

And when reading the runtime structure answer, something came in my mind. TS has a private modifier which actually does nothing like prefixing variables with underscores. It's just a way for the TS compiler to figure out that the property shouldn't be accessed from outside. This isn't entirely representing the runtime structure either, right? Because at runtime, that property is just accessible like all the rest. Actually, in my opinion they should prefix that with an underscore. I always end up doing private _foo which looks like doing the same thing twice...

@jamiebuilds
Copy link
Contributor

is that the right title?

lol idk

TS has a private modifier which actually does nothing like prefixing variables with underscores.

Could you give me an example of this?

@SamVerschueren
Copy link
Contributor

lol idk

Let's call it that ;)

Could you give me an example of this?

Sure, this is an example with private properties and methods.

class User {

    private firstName;
    private name;

    constructor(firstName: string, name: string) {
        this.setFirstName(firstName);
        this.setName(name);
    }

    private setFirstName(firstName: string) {
        this.firstName = firstName;
    }

    private setName(name: string) {
        this.name = name;
    }
}

Results in

var User = (function () {
    function User(firstName, name) {
        this.setFirstName(firstName);
        this.setName(name);
    }
    User.prototype.setFirstName = function (firstName) {
        this.firstName = firstName;
    };
    User.prototype.setName = function (name) {
        this.name = name;
    };
    return User;
}());

@jamiebuilds
Copy link
Contributor

Maybe this isn't the best place to discuss this particular issue.

One of the big differences between Flow and TypeScript is that in Flow if you strip the types you have plain JavaScript code and it behaves exactly as expected, we don't add any runtime features, and we don't modify any behavior whatsoever. We're actually unable to do that with our current setup. On the other hand TypeScript adds additional features to JavaScript that go beyond adding static typing, things like namespaces, enums, etc. which is fine until these things are added to JavaScript (enums is currently being proposed with different semantics than the TypeScript version).

I'm not sure why TypeScript has added private fields to JavaScript while also not making the runtime enforce them at all, it seems like a weird straddling of both design decisions and could cause problems if private class members are added to JavaScript. But the risk there is hard to judge.

Hopefully that answers your question.

@RyanCavanaugh
Copy link

Getting off-topic, but TypeScript has private members because people really really really really want private members. It's not enforced at runtime because there's not (in ES3 particularly) a way to do this that doesn't result in weird code mangling, has terrible performance implications, or usually both. Often people want private fields that are "sometimes" accessible (e.g. from unit tests, not that I endorse checking private members as part of a test suite) and having the fields be runtime accessible under the normal name while still getting type system enforcement is a very useful compromise.

If you read the TypeScript issue tracker you'll see over and over again us refusing to add trivial syntactic sugar because it could conflict with future JS features. The risk is taken quite seriously.

@DanielRosenwasser
Copy link

One of the big differences between Flow and TypeScript is that in Flow if you strip the types you have plain JavaScript code and it behaves exactly as expected, we don't add any runtime features, and we don't modify any behavior whatsoever.

TypeScript doesn't do any type-driven emit either. If you don't want to use namespaces or enums, you can feel free not to, the same way that you don't necessarily have to use certain features in Babel.

I'm not sure why TypeScript has added private fields to JavaScript while also not making the runtime enforce them at all

For the exact same TypeScript has types but doesn't do any dynamic runtime checking. Flow takes a page from that book too. The same way that types are a compile-time only thing, and that's how we view accessibility as well.

@DanielRosenwasser
Copy link

I disagree with the view that the pattern used here isn't the purpose of a type system. A type system can serve a lot of different purposes. I think it ultimately depends on what the authors wanted, so @sindresorhus and others should probably make that call.

For what it's worth, I managed to pretty quickly convert your declarations down to this .d.ts file as well.

@sindresorhus
Copy link
Member Author

I appreciate everyones input. We really want to have the best possible type definition for both TypeScript and Flow. Let's keep this thread Flow related and continue the TypeScript definition discussion in #1005.

@jamiebuilds
Copy link
Contributor

Sorry, I shouldn't have let this go off topic. But if @SamVerschueren @RyanCavanaugh and @DanielRosenwasser want to talk elsewhere I'm happy too. I don't think my words are being understood as intended.

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

No branches or pull requests

6 participants