cssinjs / jss

JSS is an authoring tool for CSS which uses JavaScript as a host language.
https://cssinjs.org
MIT License
7.07k stars 398 forks source link

[RFC] Move to Typescript #1035

Open HenriBeck opened 5 years ago

HenriBeck commented 5 years ago

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.

karlhorky commented 5 years ago

Checking types using jsdoc types using ts compiler.

Yeah, this is a good way to do an incremental migration by building up the types and soundness in your existing JS files.

hosseinmd commented 3 years ago

By Jsdoc we can't define some complex types, But we could use Jsdoc to documentation.

hosseinmd commented 3 years ago

Migration to typescript could save us from some bugs in compile time or when we're coding.

kof commented 3 years ago

Complex types can be defined in type files and used inside comments, right?

hosseinmd commented 3 years ago

Yes but what is the benefit of that, why we don't use typescript?

hosseinmd commented 3 years ago

I found some type which is different in .d.ts vs source, like InsertionPoint.

hosseinmd commented 3 years ago

RuleListOptions is very different

kof commented 3 years ago

Yes but what is the benefit of that, why we don't use typescript?

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. A side effect of that is also js code can still be parsed directly by any parser or even run directly in the browser as is if needed. It's just javascript in this case. I am sure webpack maintainers have more to say about this approach since they use it for years. cc @bebraw

hosseinmd commented 3 years ago

This is not clear, through this way developers should understand jsdoc and typescript both.

karlhorky commented 3 years ago

After starting an increasingly large project with JSDoc, I would recommend staying away from JSDoc. It's a different, less ergonomic syntax (then you need to learn both if you want to have the complex types in TS files as well), you can't do everything you can in TypeScript, other downsides too - TypeScript is so much simpler and more natural. The downsides of a learning curve for TS beginners are far outweighed by the downsides of JSDoc, in my opinion.

hosseinmd commented 3 years ago

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

ITenthusiasm commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

https://github.com/joarwilk/flowgen

kof commented 3 years ago

@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 commented 3 years ago

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

https://user-images.githubusercontent.com/52824/113268821-1baf5a00-92d8-11eb-9e2e-786087775087.mp4

hosseinmd commented 3 years ago

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 commented 3 years ago

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

kof commented 3 years ago

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

hosseinmd commented 3 years ago

@kof excuse me what's the question?

hosseinmd commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

I made the first step and moved flow types to a typedef file for one module https://github.com/cssinjs/jss/pull/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 commented 3 years ago

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

kof commented 3 years ago

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 commented 3 years ago

I finally finished moving all flow types in all packages, once this is merged, nothing stops us from using ts https://github.com/cssinjs/jss/pull/1509

kof commented 3 years ago

This is merged https://github.com/cssinjs/jss/pull/1509

hosseinmd commented 2 years ago

Could you mention a Road map for migrate to typescript?

ّWe need to pursue this more seriously