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

Add Flow or TypeScript typing to repo #1272

Closed
Daniel15 opened this issue Nov 12, 2017 · 7 comments
Closed

Add Flow or TypeScript typing to repo #1272

Daniel15 opened this issue Nov 12, 2017 · 7 comments
Labels
developer-experience Dev tooling, test framework, and CI

Comments

@Daniel15
Copy link
Member

It would be nice to have either Flow or TypeScript in the codebase, to better understand return types for some of the more complex functions.

@paulmelnikow paulmelnikow added the developer-experience Dev tooling, test framework, and CI label Nov 13, 2017
@paulmelnikow
Copy link
Member

I don't have much experience with either, though I'm happy to learn. I'll find an area in this project or another, to do a proof of concept. Any suggestions? The frontend perhaps?

It's really important that this project have a wide base of developers. There are lots and lots of package repositories and CI services for all sorts of different developer communities, and it's better to empower and enable all those developers to build out the features they use. This means lots of non-JS devs are hacking on this code.

So, I think we should decide whether to do this based on how likely it makes people to contribute. If we think we'll get more contributors by doing this, let's do it. If we think we'll get fewer, let's look at options involving ES6/ES7/ES8.

Another possibility is that we use a type system in the frontend and not in the server. React devs –
who are our target for frontend contributions – may be more likely knowledgeable of one of these type systems than a non-JS dev who is adding a service for their language.

@Daniel15
Copy link
Member Author

Daniel15 commented Nov 30, 2017

I was thinking it'd be good for the server, because there's several data structures in use that are mostly undocumented and would benefit from type checking (for example, the badgeData passed to sendBadge). Sure, we could write docs on the data structure, but it's nicer to have a strong type for it as the code then becomes somewhat self-documenting.

Maybe a good starting point is to add types to a few of the smaller files in the lib directory, to get a taste of it.

With Flow, you opt-in per-file by adding an @flow annotation to a docblock at the top of the file, so you don't need to convert the entire app at once ad can instead just gradually add types :)

@paulmelnikow
Copy link
Member

Cool, I'll take that on once #963 is merged.

@styfle
Copy link

styfle commented Dec 4, 2017

I vote for TypeScript! I could give some pointers or review a PR.

See my repo styfle/react-server-example-tsx for an example implementation.

@Daniel15
Copy link
Member Author

Daniel15 commented Dec 5, 2017

Yeah I think either one is fine. I'm more familiar with Flow (as it's what we use at Facebook) and it tends to handle React a bit better, but they're both good 😃

@paulmelnikow
Copy link
Member

I'm working on a PR for Danger, which uses TypeScript. So I'm getting some exposure!

@paulmelnikow
Copy link
Member

After working on that PR for Danger, my overall impression is that the types make working with code faster, and avoid common problems. It makes many things self-documenting, and forces you to think about how to model your data, which is awesome. It took me longer to make some changes, probably that is in part because I'm still learning the type system. The tooling is different and was initially unfamiliar, and that took me a good bit of time to adapt.

For a project like this, it's a difficult decision. I think it's really important that our backend code is welcoming to developers with all kinds of backgrounds. We've received many contributions in the last year by people who have never written JavaScript before. And I think that's really important for a project like this, which supports a really broad development community. Overall I would optimize for an easy learning curve. That suggests keeping with ES6.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
developer-experience Dev tooling, test framework, and CI
Projects
None yet
Development

No branches or pull requests

3 participants