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

Upgrade flow-bin #2930

Closed
Zalastax opened this issue Nov 15, 2017 · 14 comments
Closed

Upgrade flow-bin #2930

Zalastax opened this issue Nov 15, 2017 · 14 comments

Comments

@Zalastax
Copy link
Contributor

There has recently been a lot of improvements in the error reporting of flow so upgrading flow to the newest version should improve the development experience quite a lot. I hope that an upgrade will improve the type-checking speed as well as it's horrendously slow for me (maybe a Windows problem?).

@jquense
Copy link
Contributor

jquense commented Nov 15, 2017

upgrading flow is sort of a nightmare, i've tried casually a few times with no luck. We'd definitely be open to a PR tho if you are interested!

@Zalastax
Copy link
Contributor Author

I tested upgrading and most errors seem easy to fix. Unfortunately I hit an issue with the package resolving, related to lerna facebook/flow#5107. Adding an exclusion in Windows Defender speeds up checking a lot when running on the command line, but it's still terribly slow in VSCode. This is a long shot but would you consider TypeScript instead?

@KyleAMathews
Copy link
Contributor

I'd be totally open to moving to Typescript — as long as someone wants to really really own it. We'd be in a far worse position than we are now if someone got partway into converting things over and then abandoned the effort leaving us with part-flow, part-typescript, part-js codebase.

It'd be a several week project at minimum to convert things over.

@Zalastax if you've got that kind of spare time to work on things, would definitely strongly consider whatever proposal you wanted to make :-)

@Zalastax
Copy link
Contributor Author

That's really good to know! I don't have the time to do a conversion right now but I'll look into how TS is best added to a lerna+babel project. Looking at https://babeljs.io/blog/2017/09/12/planning-for-7.0 it seems like the workflow is similar to flow.

@ianseverance
Copy link

I think that moving from Flow to TypeScript would be a poor decision for multiple reasons:

  1. Either way, you're not writing standard JavaScript. But flow offers first-class support for React (obviously, but it is what Gatsby is built on), which is not something that should be overlooked.
  2. With Flow, you can incrementally convert your JS, without having to rewrite your code base on Day 1.
  3. Since it's built by Facebook and used on millions of lines of their own code, you know that the tool and the community will be maintained.
  4. Flow requires very little effort to implement, and in return provides a ton of value; without having to change every JS file to .ts and learn a new superset of JS.
  5. By changing the code base to TypeScript, you would be alienating any developer that doesn't know the language, who would have been able to provide value to the Gatsby community (whether it be plugins, bug fixes, etc.).

Like @KyleAMathews said

We'd be in a far worse position than we are now if someone got partway into converting things over and then abandoned the effort leaving us with part-flow, part-typescript, part-js codebase.

With Flow, you can have a part-flow code base, adopt it slowly, and reap the benefits while doing so.

I mean, the list goes on and on for me. But, just another opinion 👍

@Zalastax
Copy link
Contributor Author

Zalastax commented Dec 15, 2017

I'm not here to make choices for the maintainers of Gatsby but you're making some claims that are not true.

Either way, you're not writing standard JavaScript. But flow offers first-class support for React (obviously, but it is what Gatsby is built on), which is not something that should be overlooked.

Facebook and Microsoft both care a lot about the React+TypeScript experience. The definitions are well maintained and very easy to use. It really is a first-class support for React in both Flow and TypeScript. A side note is that looking at https://stateofjs.com/2017/connections/ we have twice as many using React+TypeScript compared to React+Flow.

With Flow, you can incrementally convert your JS, without having to rewrite your code base on Day 1.

This is possible with TypeScript as well. Just follow this guide: http://www.typescriptlang.org/docs/handbook/migrating-from-javascript.html. Just enable allowJs and you'll be fine.

Since it's built by Facebook and used on millions of lines of their own code, you know that the tool and the community will be maintained.

Both languages are well maintained and will be in use for a long time. There's no indication that should make you worried for any of the languages. Side note again, looking at https://stateofjs.com/2017/connections/ we see that four times as many respondents used TypeScript compared to Flow.

Flow requires very little effort to implement, and in return provides a ton of value; without having to change every JS file to .ts and learn a new superset of JS.

It's fairly new but you can use a // @ts-check comment in JavaScript files: https://github.com/Microsoft/TypeScript/wiki/Type-Checking-JavaScript-Files. The superset of JavaScript that TypeScript uses is a pragmatic version of ES6, basically like using babel + preset-es2017 + preset-stage-3. I see it as having the best plugins curated for you, and when features are mature enough they get added automatically when you upgrade the TypeScript version.It's a big feature to automatically know what constructs are available when you come to a TypeScript project. Compare that to a babel project where you have to check what plugins are enabled.

By changing the code base to TypeScript, you would be alienating any developer that doesn't know the language, who would have been able to provide value to the Gatsby community (whether it be plugins, bug fixes, etc.).

I get your concerns here but think the problem is the same for a project using Flow, since we should strive for a full Flow coverage as well. Having types of any kind would be a huge improvement for myself, since it lets me see the shape of the data going in and out. Whatever language gets used I think it's important to help newcomers make successful contributions. I'd like a roadmap like this: https://github.com/processing/p5.js/wiki/Development but we are already in a fairly good spot with the intro-guide and the first-good-issue tags.

I think both choices are valid but I've had a lot worse experience with Flow than with TypeScript. Failing to upgrade Flow to a new version when the old one is quite new is not a good sign. Flow v0.42 is extremely slow to typecheck for me, which makes the experience when editing very painful. I've stopped using gatsby though, so I won't push forward with this.

@ianseverance
Copy link

@Zalastax I'm not so sure that I am making false claims!

Facebook and Microsoft both care a lot about the React+TypeScript experience.

I don't doubt that. But it is not that often (read: almost never) that Facebook builds their own version of another tool, and then decides to not eat their own dog food. Both Flow and React are Facebook's. Typescript is Microsoft's. Would you continue supporting another company's tool for the sake of "caring", when you have your own to support? The divide will grow with time.

we have twice as many using React+TypeScript compared to React+Flow.

Unfair comparison, considering TypeScript has a two-year headstart on Flow!

Just follow this guide

The fact that you need a guide, proves my point when I said

With Flow, you can incrementally convert your JS, without having to rewrite your code base on Day 1.

Also, I read the guide and it's obviously doable for a sizable codebase to convert to TypeScript. But once again, you have to conform to TypeScript's way of doing things. Flow makes this obsolete.

Both languages are well maintained and will be in use for a long time. There's no indication that should make you worried for any of the languages.

Gatsby is built on React, not Angular. Flow's native support is for React.

The superset of JavaScript that TypeScript uses is a pragmatic version of ES6, basically like using babel + preset-es2017 + preset-stage-3.

Would you rather have the power to choose what versions/changes of JavaScript to use or would you rather the tool implement these decisions for you? Sounds like a restriction, not a feature, to me.

Compare that to a babel project where you have to check what plugins are enabled.

Why is searching through TypeScript's documentation for their supported versions of JavaScript (for each given release), easier than reading a config file? Also, any new coder should know what the underlying codebase is doing behind the scenes, instead of just taking advantage of the tool. That's one of the big issues with new coders entering the industry of today, is all of these frameworks not teaching what is actually going on. But I digress.

I get your concerns here but think the problem is the same for a project using Flow, since we should strive for a full Flow coverage as well.

I was stating that:

By changing the code base to TypeScript, you would be alienating any developer that doesn't know the language

Flow is comparable to syntactic-sugar on top of React. TypeScript is a whole other language.

I agree that typing is important, and would provide a lot of value to the platform. I just think that the decision to move from Flow to another type-checker should be thought through, and there's not many good reasons to (currently) do so.

@Zalastax
Copy link
Contributor Author

You're being unfair in your comparison to make a point that does not exist. They are both either whole other languages or none of them are. They both have native React support. They both will be supported for years to come.

The reason I wanted a switch is mainly because upgrading flow from a few months old release is breaking the code. If there is a need to do a lot of work anyway I'd personally rather get something that is stable and that I've had a lot less trouble with. A really big thing for me is that the type hint speed for TS is magnitudes faster compared to what I saw with flow in gatsby.

You shouldn't think of this as a migration since a large majority of the code is untyped and since both choices require a fair bit of work. See it as either picking latest flow or latest TS. To me, the choice is clear.

@jquense
Copy link
Contributor

jquense commented Dec 21, 2017

Folks, I'm not sure this is the best place to hash out this Flow vs TS argument. Both have merits and downsides, but the main point here is a bout upgrading or converting to typescript. On that front the project is fine with either option, that may deciding factor about which get's done really comes down to whether anyone wants to own it and make it happen, not whether one is better or worse suited for the project. (it's also fine if no one has time to take this on we understand). Thanks for taking an interest ya'll

@m-allanson
Copy link
Contributor

m-allanson commented Mar 6, 2018

@jquense what sort of issues did you run into with upgrading flow? Do you think it's something a flow newbie (like me) could do by grinding through it, or does it need someone with more experience of using flow?

@Zalastax
Copy link
Contributor Author

Zalastax commented Mar 6, 2018

@m-allanson if you know how gatsby is supposed to work and have the time, then it should be possible.
Without the knowledge of how it all fits together, making the right decisions can become quite hard and time consuming. Perhaps there is a way to make the types more ready for an upgrade before actually doing it?

@m-allanson
Copy link
Contributor

m-allanson commented Apr 13, 2018

@Zalastax finding the time is always the problem :)

Another possible approach - webpack recently added Typescript linting via JSDoc comments:

https://twitter.com/TheLarkInn/status/984479953927327744
webpack/webpack#6862

@Zalastax
Copy link
Contributor Author

@m-allanson yes, the allowJs approach is a good path. The tricky thing is to understand what the code should actually do, which I think requires lots of time or dedication from the authors. That is what stumped my attempt to upgrade the flow version. Especially tricky is that there might even be bugs in the current system, so a type error might be warranted.

@KyleAMathews
Copy link
Contributor

Due to the high volume of issues, we're closing out older ones without recent activity. Please open a new issue if you need help!

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

No branches or pull requests

5 participants