-
-
Notifications
You must be signed in to change notification settings - Fork 788
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
Make itemData generic in grids and lists #66
Conversation
In this PR I am trying to achieve more sound implementation of itemData props in all grids and lists. To prevent `empty` type itemData is made optional with defaultProps. `flow` script is replaced with `flow:check` to allow running `yarn flow status` to view all errors and warnings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really great! Thanks!
@@ -5,5 +5,5 @@ before_script: | |||
- yarn | |||
script: | |||
- yarn lint | |||
- yarn flow | |||
- yarn flow:check |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why?
I prefer yarn flow
since it's less to type and what I use in most of my other projects. 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I run yarn flow
often. The script with the same name overrides bin.
@@ -0,0 +1,154 @@ | |||
// @flow |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand why this file exists, but I'm not a fan of the name. It's the only snake_case filename in the project. 😄 I think we should rename it to test.js.flow
so it will be still be Flow-checked, but it will be excluded from the NPM package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I name it like this to not conflict with jest. But yeah, .flow
should solve the problem too.
Published in 1.2.1 |
Thanks! |
In this PR I am trying to achieve more sound implementation of itemData
props in all grids and lists.
To prevent
empty
type itemData is made optional with defaultProps.flow
script is replaced withcheck
to allow runningyarn flow status
to view all errors and warnings.