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

[RFC] Move to Typescript #1035

Open
HenriBeck opened this issue Feb 15, 2019 · 89 comments
Open

[RFC] Move to Typescript #1035

HenriBeck opened this issue Feb 15, 2019 · 89 comments

Comments

@HenriBeck
Copy link
Member

HenriBeck commented Feb 15, 2019

With more OS projects now planning to move to Typescript (jest, Vue.js, etc.), I think it would be time to think about doing the same. Typescript has matured and now with eslint and babel both supporting Typescript as well, it should be future proof.

Additionally, I think the tooling around Typescript (speed, autocompletion), etc. is far head of flow currently.

I would start with "rewriting" smaller packages (like css-functions and theming) in the beginning.
Also, I started writing the new docs with Typescript.

Are you willing to implement it?
Yes

EDIT: I started rewriting the core in my fork in Typescript as an experiment.

@HenriBeck HenriBeck added the idea What if we turn JSS into styled-components? label Feb 15, 2019
@TrySound
Copy link
Member

Besides better tooling I saw a lot of unsoundness in TS which is "won't fix" when flow to gonna be and actually already safer than TS in some cases. For me it doesn't worth to switch.
I'm quite happy with flow in big project and open source.

@HenriBeck
Copy link
Member Author

Why would Typescript be less type-safe?

@TrySound
Copy link
Member

Here guys from flow team provides some examples
https://discord.gg/zGY9gW

For example TypeScript doesn't have exact types at all (microsoft/TypeScript#12936) when flow is gonna have them by default. Object spread is unsound (microsoft/TypeScript#15454) when flow is already more sound here and is gonna improve experience with exact by default.

image

@TrySound
Copy link
Member

Flow has some problems but flow team is quite active and open last few weeks. They are going to the right direction but their plan is time consuming. Now they have resources to work with community.

@kof
Copy link
Member

kof commented Feb 15, 2019

That's interesting, do you know how many people work on flow full time? I feel like flow with strict mode and better error descriptions would actually make the whole thing quite competitive. It's just a matter if companies are willing to invest enough to improve.

@TrySound
Copy link
Member

This is just now. There are more people there
image

@kof
Copy link
Member

kof commented Feb 15, 2019

Fulltime contributors? Where do you see it?

@TrySound
Copy link
Member

It's a chat with flow team (facebook guys). The link I put above.

@HenriBeck
Copy link
Member Author

I think flow error descriptions and reporting is quite better (especially when it comes to react props) than Typescripts.

What I now dislike the most about it is how slow the editor integration is and that types are an afterthought.

@TrySound
Copy link
Member

Which editor do you use?

@HenriBeck
Copy link
Member Author

Webstorm, and for me the autocompletion, auto-import etc. is really slow, I think in a recent release they improved the language server to handle request while they are recomputing all of the types though.

@TrySound
Copy link
Member

There is one representative from webstrom team in flow chat. They are discussing better integration. Yeah, main problem is checking too much. Ignoring things usually helps.

@HenriBeck
Copy link
Member Author

The reason why I proposed this change that a lot of quite large projects are moving into this direction.
And yeah, TS maybe is sometimes unsafer, but I think this only applies for certain type definitions/operators.

The reason I got to like Typescript so much is that the language was designed with types in mind and not just an afterthought/addition like flow.

@TrySound
Copy link
Member

Well, sorry, but herd instinct is a bad reason to refactor all the project. It looks like "flow is a shit and nobody will ever use, let's kill it". By doing this you just add fuel to the fire of hate which is really started by just one person.

By the way about big projects decisions. Here's another one
https://github.com/mdn/mdn/pull/53/files#diff-992ca475abeb0c101165f19ff62d37e7R14

@TrySound
Copy link
Member

And typescript was designed around the same javascript with all it's problem. It's not really different language. Reason/ocaml or haskell was designed with types.

@kof
Copy link
Member

kof commented Feb 16, 2019 via email

@HenriBeck
Copy link
Member Author

I never said that Typescript is a different language and has not the problems of javascript, but at the same time Typescript was designed around types (which in my opinion solves a few of them). The best comparison for that in my mind is how you import/export types in Typescript vs. flow.

@TrySound
Copy link
Member

Flow was also designed around types. It's type system.

Well, I dont see any sense in import way. It's just a choice without "not types in mind"

Flow coming types first arch name is quite descriptive.

@HenriBeck
Copy link
Member Author

What I mean by "designed around types" is that flow tries to add types on top of javascript, and Typescript is a superset of javascript which compiles down to js.

@kof
Copy link
Member

kof commented Feb 16, 2019

That just means that flow is not going to add language features, just types, where typescript has done so and might do more. In that sense, ts has more power. At the same time, the more ts does so, the further away it is from js.

Most things other than that are basically implementation details and features which one has but another one doesn't and vise versa.

I think we need to clearly define the criteria for our choice first of all. Knowing we are working in a library context.

@TrySound
Copy link
Member

Its not true. Both add types and invalid syntax to javascript. Both are compiled to javascript. But flow team just is a bit more careful with JS semantics.

@kof
Copy link
Member

kof commented Feb 16, 2019

Where did flow add something that compiles to runtime javascript?

@TrySound
Copy link
Member

Well exact types is a good reason to stay with flow.

@TrySound
Copy link
Member

What babel compiles to JS from TS?

@kof
Copy link
Member

kof commented Feb 16, 2019

For example

enum Direction {
    Up = 1,
    Down,
    Left,
    Right,
}

@kof
Copy link
Member

kof commented Feb 16, 2019

Well exact types is a good reason to stay with flow.

Can you expand pls? Afaik both have this, but mb you know more than I do

@kof
Copy link
Member

kof commented Feb 16, 2019

TBH my biggest problem with both flow and TS is to maintain types for both. Ideally I want to do it only once.

@TrySound
Copy link
Member

Read the thread. TS doesnt have exact types.

@TrySound
Copy link
Member

AFAIK enums are not compiled via babel.

@hosseinmd
Copy link
Member

Tests can move to ts too. We should discuss it too.

@ITenthusiasm
Copy link
Member

Figured I'd finally add some thoughts. I might have more later; my time is limited.

I think it's good to recall attention to the question of "What most enhances the developer experience?" It's probably the most important one, though it may fight with the question of whether or not people are willing to maintain the code in first place.

To that extent, I do think that migrating to TS in the long term is a significantly better choice.

Popularity

I could be wrong, but I feel like on average there are going to be less developers familiar with flow. As HenriBeck (and jeremy-coleman) pointed out, many popular libraries have moved their codebases to TS.

Well, sorry, but herd instinct is a bad reason to refactor all the project.

But herd instinct isn't the only matter here. If popular libraries are not only used by several people but contributed to by several people, this theoretically implies that more contributors (especially the good ones) would be more familiar with TS than with flow (theoretically). If we want helpful contributors over the long haul, this is worth considering.

I don't consider this the strongest point.

Ease of Access

Flow honestly kind of feels like a hassle. The flow server is always crashing on my IDE. And even getting flow setup in something like VS Code is... a weird experience. I raised #1461 to try to help that problem, but the reality is that contributors shouldn't even have to deal with this in the first place.

TypeScript works straight out of the box, and it's fast. This is somewhat worth considering.

Maintainability and Tooling

Maintaining 2 type systems is always pain in the ass

If we use TypeScript, we wouldn't have to worry about this problem. We could write the codebase in TypeScript, and when we do the build, the type definitions could be automatically generated. This is huge because we don't have to worry about keeping JS (or JS + Flow) in sync with the TS types we expose. So the codebase becomes smaller, and the potential to run into problems like #1482 gets eliminated.

As far as I understand (correct me if I'm wrong), for users to get type definitions for a library, they need TypeScript [definition] files. This means we'll have to be working with TypeScript anyway to some extent or another. We can avoid duplicating our work by leveraging TS.

Miscellaneous

The benefit is avoiding having types mixed with js code, it's easier to read for people who are not using ts in every day work.

There's actually a kind of double-edged sword when it comes to this. If you have a small, simple repository that's very easy to contribute to, then there's a chance you don't need TypeScript. Someone can just read the code without any distractions, and life is simple. Some parts of the Testing Library family are an example of this.

However, as the codebase expands, having to deal with raw JS actually gets worse. You end up with a lot of functions and a lot of moving pieces that are hard to follow. Types help reduce some of the cognitive load here, and it seems to some extent we all agree that they're necessary. The benefit of using Flow or JSDocs is that people can technically get away with using raw JS and ignoring types.

However, the reality is that contributors probably shouldn't be adding code to a large project without [good] type information because it will cause more ambiguity/confusion for people further down the road. So the contributor will have to learn JSDocs/Flow/TypeScript anyway if we're looking at a code base that's more maintainable long term.

@ITenthusiasm
Copy link
Member

I really do think TS is the better play in the long haul.

I'm not sure about what the migration path should look like. And I don't know how quickly the work should be done.

Something gradual and non-disruptive would probably be best. (But that's assuming everyone decides migrating to TS is a good idea.)

@ITenthusiasm
Copy link
Member

I started rewriting the core in my fork in Typescript as an experiment.

@HenriBeck I know it's been a while, but what's the status of that fork?

@kof
Copy link
Member

kof commented Mar 30, 2021

@ITenthusiasm I would love to see flow types being generated from ts, but I don't believe this will work out. If it doesn't we will still have to maintain flow types, just from bindings. Other than that, I agree we should define a migration strategy to use TS.

One more idea though: when keeping types in separate files, a problem is to keep the interface in sync. I just though since we have/should have every interface tested, using types in tests will make sure that the interface is in sync with code.

@ITenthusiasm
Copy link
Member

I would love to see flow types being generated from ts, but I don't believe this will work out. If it doesn't we will still have to maintain flow types, just from bindings. Other than that, I agree we should define a migration strategy to use TS.

Ah, I see. I may have phrased things poorly. When I referred to types getting generated automatically, I was referring to TS type definitions (because that's what the end user will need to get intellisense if I understand correctly). So if we write the entire codebase in TS, we won't need separate type definitions files. Instead, during the build process, the TS type defs that the end users need would automatically be generated.

End users wouldn't need/use the flow types right? I didn't think flow had a separate type definition system like TS did (eg. index.d.ts, not index.ts).

I just though since we have/should have every interface tested, using types in tests will make sure that the interface is in sync with code.

If JS/TS absolutely had to be separated, then yeah we could add tests for each of the types in addition to the implementation details. But separation increases cognitive load, as well as the potential for visual overload when navigating a file system. If users will still be required to keep the API and the types in sync anyway, then they'll still be forced to learn/use TypeScript. In that case, I feel like using regular TS files all the way would be easier.

@kof
Copy link
Member

kof commented Apr 1, 2021

Ah, I see. I may have phrased things poorly. When I referred to types getting generated automatically, I was referring to TS type definitions (because that's what the end user will need to get intellisense if I understand correctly). So if we write the entire codebase in TS, we won't need separate type definitions files. Instead, during the build process, the TS type defs that the end users need would automatically be generated.

Yeah, this doesn't remove the need for all the flowtype users to have their types. Users using flowtype in their code base won't be able to use TS type defs to typecheck. VSCode can give them the intellisense, true.

@kof
Copy link
Member

kof commented Apr 1, 2021

If JS/TS absolutely had to be separated, then yeah we could add tests for each of the types in addition to the implementation details.

Wait that's exactly the point I am making, we don't have to. We have real tests. If those tests are writen in TS, they already use the interface and validate it.

@kof
Copy link
Member

kof commented Apr 1, 2021

But separation increases cognitive load

Isn't intellisense showing all the types when you hover over functions/classes? I mean this way we still see the types and we don't need to see them if we don't want to! That's ideal from both worlds. I am not sure if it's possible to link a typedef to the module specifically without making a reference? But I guess we could make the references in the code files

@kof
Copy link
Member

kof commented Apr 1, 2021

I tried to do that, so far this has worked:

https://codesandbox.io/s/admiring-goldwasser-27rki?file=/main.js

I have seen people saying on SO it's possible to automatically match type defs and code (so that we don't have to do @type import ... next to every function), do you know how? Didn't work for me.

@karlhorky
Copy link

If supporting Flow users is an absolute necessity, maybe it's worth taking a look at flowgen?

https://github.com/joarwilk/flowgen

@kof
Copy link
Member

kof commented Apr 1, 2021

@karlhorky we can try, but I wouldn't bet on it to work properly considered complexity of TS types we currently have, including imported CSS types.

@kof
Copy link
Member

kof commented Apr 1, 2021

Intellisense the way I used it here doesn't show a full type def for the function when asking it to show the type. Jumping to type definition works. This approach wouldn't be nice for a product team, but considered this is a lib, it's doable. I would love though to not have to add @type and to see a full type def on hover. To me this would be perfect.

https://codesandbox.io/s/admiring-goldwasser-27rki?file=/main.js

ts.d.mp4

@hosseinmd
Copy link
Member

hosseinmd commented Apr 1, 2021

jsdoc cannot Solve problem of flow. if we use ts/jsdoc user can't test real flow.

But about your example
I have used JsDoc before, I believe that typescript is more clear than Jsdoc. For example if your function have generic params, you should define them in jsdoc too.

Compare your code in js and ts version


Your example:

main.d.ts

export type Fn = () => string;

main.js

// @ts-check
/** @type {import("./main").Fn}*/
export const fn = () => 1;

ts version of that is:

export const fn = (): string => 1;

@hosseinmd
Copy link
Member

If we want to use jsdoc, we should code more without a good reason or benefit.

@kof
Copy link
Member

kof commented Apr 2, 2021

@hosseinmd do you know the answer to the question I posted?

@hosseinmd
Copy link
Member

@kof excuse me what's the question?

@hosseinmd
Copy link
Member

I have seen people saying on SO it's possible to automatically match type defs and code (so that we don't have to do @type import ... next to every function), do you know how? Didn't work for me.

I don't know

@kof
Copy link
Member

kof commented Apr 2, 2021

If we have to deal with more writing and limited syntax I wouldn't want to use it. I am trying to understand what happens if JS files are linked to type defs. Type defs have a full TS support if I understand correctly. If they can be linked in a nice way to code and vscode can show the types on hover - I don't see any problems, but there are 2 "ifs" here.

@ITenthusiasm
Copy link
Member

Yeah, this doesn't remove the need for all the flowtype users to have their types. Users using flowtype in their code base won't be able to use TS type defs to typecheck. VSCode can give them the intellisense, true.

Ah, okay that's super helpful! That was the disconnect for me. Yeah...that makes this a little hard... hm...

Wait that's exactly the point I am making, we don't have to. We have real tests. If those tests are writen in TS, they already use the interface and validate it.

Yes! Originally, it wasn't really my preference to have that separated out. However, now that I know that flow users still need flow type definitions, I'm not sure what's best right now.

I need to think more about the problem now. Do we compile flow type definitions in a separate file or do they remain with the code?

@kof
Copy link
Member

kof commented Apr 2, 2021

both options exist, we currently don't generate type defs, we generate a .flow file which is export * from '../src';, but we can as well generate type defs. I think it's the same thing as with TS

@HenriBeck
Copy link
Member Author

Hey Guys,

I haven't been active in the project for a long time but I hope to get back into it and find some time in the coming weeks.

My take is this by now on the whole Flow vs. Typescript topic:

At this point, it is clear and a fact that TS has the upper hand, and Flow will not catch up at this point (just looking at the installs on NPM and usage in the industry).
Considering this, I don't see an argument against migrating the codebase to 100% TS, it is more used in the community and the better-supported ecosystem/community.

I guess the only question is if we want to start out with providing Flow types or see if this is needed at all.

Overall the challenge will still be to provide a good CSS style definition due to the very dynamic nature of our plugin system.

@kof
Copy link
Member

kof commented Apr 9, 2021

I think moving public interfaces which are currently part of code to type defs file isn't too hard, much harder is to make flow types same way detailed, with CSS types and stuff. We can just wait if someone is interested to contribute and otherwise not waste the energy on improving them. Just keep them as they are and move them out of the js files.

@kof
Copy link
Member

kof commented May 5, 2021

I made the first step and moved flow types to a typedef file for one module #1509
It works incrementally, I could merge just this one. We could merge them all at once though, I think it's not too hard. Here is a document about declaration files https://flow.org/en/docs/declarations/

Anybody wants to take on some modules and do the same thing?

@hosseinmd
Copy link
Member

hosseinmd commented May 6, 2021

I like to move all react-jss to ts. can I do it?

@kof
Copy link
Member

kof commented May 6, 2021

I like to move all react-jss to ts. can I do it?

yes, though first we need to move flow definitions to .flow files, like I showed above :)

feel free to send a PR for moving flow definitions right onto that branch https://github.com/cssinjs/jss/tree/move-flow-to-typedefs

@kof
Copy link
Member

kof commented Jul 17, 2021

I finally finished moving all flow types in all packages, once this is merged, nothing stops us from using ts #1509

@kof
Copy link
Member

kof commented Sep 5, 2021

This is merged #1509

@hosseinmd
Copy link
Member

Could you mention a Road map for migrate to typescript?

ّWe need to pursue this more seriously

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