update tabs child validation #112
Conversation
There was a problem hiding this comment.
Did you find some docs on this? The propTypes docs have an instanceOf example that looks similar to the previous implementation, so I'm concerned that this might be working around a different issue
src/Tabs.jsx
Outdated
|
|
||
| React.Children.forEach(props[propName], child => { | ||
| if (typeof child === 'undefined' || child.type.name !== expectedName) { | ||
| error = new Error(`Children must be React elements of type ${expectedName}`); |
There was a problem hiding this comment.
just return the error immediately rather than assigning it to a let.
| @@ -63,9 +63,17 @@ export class Tabs extends React.Component { | |||
| } | |||
| } | |||
| Tabs.propTypes = { | |||
| children: React.PropTypes.arrayOf( | |||
There was a problem hiding this comment.
I think the problem might be that children is not a true array, so when it tries to check the type of children[0], it finds an object that is not the actual child.
That means that you can use React.Children.forEach in your custom function, but also use instanceof TabsTab
There was a problem hiding this comment.
simplified this to child.type !== TabsTab, which seems to work
src/Tabs.jsx
Outdated
| let error; | ||
| React.Children.forEach(props[propName], child => { | ||
| if (typeof child === 'undefined' || child.type !== TabsTab) { | ||
| error = new Error('Children must be React elements of type TabsTab'); |
There was a problem hiding this comment.
Ah, I see why you kept the let error setup since it is set inside a callback.
Just for fun, you can still avoid the let error (and mutable state) with:
const validChildren = React.Children.map(
children,
child => child instanceof TabsTab
).every(x => x); // check that all children are TabsTab
if (!validChildren) {
return new Error(...);
}
src/Tabs.jsx
Outdated
| children: (props, propName, componentName) => { | ||
| let error; | ||
| React.Children.forEach(props[propName], child => { | ||
| if (typeof child === 'undefined' || child.type !== TabsTab) { |
There was a problem hiding this comment.
I recommend separating out your two child validation conditions - use React.Children.count to return an Error about passing in no children, and use the forEach that you have here to check the type of the children.
The separation will make it a little more clear what is needed and simplify the code a bit
const children = props[propName];
if (!React.Children.count(children)) {
return new Error(...);
}There was a problem hiding this comment.
This is good stuff. I can actually use this to require that at least 2 TabsTab children are present.
|
bump, is there anything outstanding on this one? Would be nice to cleanup the unnecessary error messaging that this would fix |
Fun fact:
React.createClassreturns anObject, not aClass, which means you can't validate in propTypes usingReact.PropTypes.instanceOf(Component)This fixes the validation issue in
TabspropTypes.