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

Using flow over propTypes #41

Closed
grabbou opened this issue Oct 25, 2016 · 6 comments
Closed

Using flow over propTypes #41

grabbou opened this issue Oct 25, 2016 · 6 comments

Comments

@grabbou
Copy link
Member

grabbou commented Oct 25, 2016

The contributing guide as well as the current files use and mention propTypes. Is that going to be our preferred approach or shall we favour Flow types instead?

@ahmedlhanafy
Copy link
Contributor

@satya164 I think you're more eligible to answer that

@satya164
Copy link
Member

Flow is certainly superior because it'll provide static type checking and autocompletion. It's also much easier to grasp than prop types in terms of documenting code. It can be used on the CI to prevent unintentional bugs.

However, we shouldn't get rid of PropTypes, because not everyone uses Flow, so runtime warnings will be useful for them.

I think we need to enforce both Flow and PropTypes for now. We should setup CI so that we don't break Flow on master.

@grabbou
Copy link
Member Author

grabbou commented Oct 27, 2016

However, we shouldn't get rid of PropTypes, because not everyone uses Flow, so runtime warnings will be useful for them.

Wouldn't a babel plugin to rewrite flow types to prop types solve that problem? That way, we could reduce the overheat of keeping two ways of describing the components API in favour of a single source of truth. The problem that I see here is that both of these provide a slightly different syntax for describing things like lists, arrays or optional values and it's rather easy to provide inconsistencies.

Here's a reference implementation: https://github.com/brigand/babel-plugin-flow-react-proptypes

@satya164
Copy link
Member

@grabbou I've been thinking about this and I think we'll need to setup a prepublish script which transpiles these flow annotations to propTypes before publishing

@ahmedlhanafy
Copy link
Contributor

Does this need to be open? @satya164

@satya164
Copy link
Member

We've not arrived at a conclusion, so yes.

@satya164 satya164 closed this as completed Feb 5, 2018
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

3 participants