search

reactjs / react.dev

The React documentation website
https://react.dev/
Creative Commons Attribution 4.0 International
10.97k stars 7.49k forks source link

Add Flow types to website source #24

Open bvaughn opened 6 years ago

bvaughn commented 6 years ago

Replace PropTypes with Flow types for our components in www (and make sure yarn flow checks these types to protect against regressions).

I got the ball rolling with commit 1de4c66

This Twitter thread also has some interesting discussion for auto-generating Flow types for Gatsby's graphql queries.

hd719 commented 6 years ago

Hey sorry @bvaughn is this now closed? Actually never mind, I looked at it wrong, the issue #10897 is closed. Sorry about that

bvaughn commented 6 years ago

No problem. 😄 Are you interested in helping out with this issue? Lots of work to be done here. Could be cool!

hd719 commented 6 years ago

Hey @bvaughn yes I am still kind of new to open source and still learning Flow Types, but yes I would like to help out on this particular issue.

bvaughn commented 6 years ago

Okay! Great.

This is a pretty big issue, so my advice is- let's do it a couple files at a time. That way it's easier to review and we don't risk conflicting with every other PR.

hd719 commented 6 years ago

Sounds good!

tricinel commented 6 years ago

I can help as well. Going through a couple of files at a time seems like a good idea.

bvaughn commented 6 years ago

Okay. I'll mark this issue in-progress.

Communicate here to avoid stepping on each other's toes. 😄

tricinel commented 6 years ago

Awesome. I've already opened a PR for it (but forgot to mention the issue number on the commit): https://github.com/reactjs/reactjs.org/pull/156. Sorry about that. Perhaps if it checks out, then it can be merged so that we keep on top of it and avoid conflicts as much as possible?

I'd like to continue working on it though. So I can continue on it next week if that's ok. I'll work through the src/components files first.

Do you think it's worth doing the utils as well? Adding flow typechecking there as well?

bvaughn commented 6 years ago

Do you think it's worth doing the utils as well? Adding flow typechecking there as well?

Yeah 😄

wuweiweiwu commented 6 years ago

@bvaughn Is this still in progress? I'd love to work on it

bvaughn commented 6 years ago

Please feel free to contribute anywhere you see missing types or types that could be improved, @wuweiweiwu. Probably worth checking through open pull requests first to make sure something hasn't already been done though! We usually let first-in win.

wuweiweiwu commented 6 years ago

@bvaughn Sounds good!

kanitsharma commented 6 years ago

Hey @bvaughn, I'd like to work on it too, is this still in progress?

bvaughn commented 6 years ago

I don't know. Looks like there's another open PR (#449) that got dropped because I just haven't had time to review this repo lately :( SOrry

tricinel commented 6 years ago

I think it's a rather old issue and PR...Last year some time I believe...I haven't looked at it in a while either. It might be worth just doing a "close and forget" (on the PR) - I'd be ok with that of course. It's your call @bvaughn! I think we all appreciate the effort needed to review these 👍

kanitsharma commented 6 years ago

No problem :), just looking for an issue to get started here.