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
[NO JIRA] add 'hasChildrenOfType' to bpk-react-utils #407
[NO JIRA] add 'hasChildrenOfType' to bpk-react-utils #407
Conversation
Generated by 🚫 dangerJS |
|
||
## hasChildrenOfType.js | ||
|
||
A prop type checker that allows you to restrict a component's children to a specific type. |
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.
Can we explain how this is different to PropTypes.instanceOf
? That was my first question upon seeing this.
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.
Would adding 'Component' help? Totally open to suggestions if you have any ideas.
eg.
A prop type checker that allows you to restrict a component's children to a specific (Component) type.
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.
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.
PropTypes.instanceOf(...)
uses js's instanceof
operator to see if the prop value is an instance of the supplied class/type. It doesn't work with components (ie. jsx or React.createElement
etc.) since those can't be operated on with instanceof
.
This type checker validates that the children of a component are all "instances" of a certain component.
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.
See the tests - they should give some more clarity :)
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.
tbh im not sure why this isn't a part of prop-types
itself -- seems like a common enough requirement
update: looks like it may be soon.. facebook/prop-types#146
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.
And airbnb's prop types seem to already support something like this https://www.npmjs.com/package/airbnb-prop-types
@janeklb where do you see this being used in backpack? Im tempted to close this as it can safely exist in userland and will most likely be available in |
@matthewdavidson This is being used in It was inspired by |
@janeklb Lets keep it inlined within the data table component package then dude - we don't think |
sounds good! cheers |
Currently lives in bpk-component-datatable (added here: bf00596) but makes sense to move to bpk-react-utils.
This PR is just to add the functionality to bpk-react-utils, then will submit a separate one for bpk-component-datatable.