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

this.props.children and react/prop-types rule #7

Closed
remitbri opened this issue Mar 2, 2015 · 21 comments
Closed

this.props.children and react/prop-types rule #7

remitbri opened this issue Mar 2, 2015 · 21 comments
Assignees
Labels

Comments

@remitbri
Copy link

remitbri commented Mar 2, 2015

Using this.props.children triggers the rule ("'children' is missing in props validation").

Shouldn't it be an exception?

@yannickcr
Copy link
Member

Right, we should add an exception for this.props.children

@yannickcr yannickcr added the bug label Mar 2, 2015
@yannickcr yannickcr self-assigned this Mar 2, 2015
@AsaAyers
Copy link

Why should children be an exception? I think you should document when a component does accept or require children.

@yannickcr
Copy link
Member

You're right, but I rarely seen some propTypes for this.props.children so it dot not seems to be a common practice to me.

Maybe we can introduce an option to prop-types to re-include children in the props list to check?

@AsaAyers
Copy link

It seems to me like most components don't need this.props.children, so to me that makes it extra important to document children in the propTypes.

I think the option idea is fine. I'm interested in hearing why anyone would want to run this against all other prop types, but never against children. It seems not well thought out to me, but I kind of hope I'm just missing something.

@sebastienbarre
Copy link

All good points. [UPDATED]
I think @AsaAyers is onto something, considering the Single Child section in the React doc:

With React.PropTypes.element you can specify that only a single child can be passed to a component as children.

  propTypes: {
    children: React.PropTypes.element.isRequired
  },

In other cases when this.props.children might be used (or not), this below could probably do the trick:

  propTypes: {
    children: React.PropTypes.oneOfType([
      React.PropTypes.arrayOf(React.PropTypes.node),
      React.PropTypes.node
    ])
  },
  getDefaultProps: function() {
    return {
      children: null // or [] I guess
    };
  },

@remitbri
Copy link
Author

remitbri commented Apr 7, 2015

I ended up with React.PropTypes.node to solve my issues…

@AsaAyers
Copy link

AsaAyers commented Apr 7, 2015

It looks like I have a combination of element, node, a few strings, and an arrayOf(...shape(...)).

@sebastienbarre
Copy link

@remitbri thanks, I updated my comment above accordingly. What's your default prop value? null?

@AsaAyers
Copy link

AsaAyers commented Apr 7, 2015

React.PropTypes.any is also an option. If I really had a component that could support 0, 1, or more children I'd probably just stick with .any to document that children is used.

@remitbri
Copy link
Author

remitbri commented Apr 7, 2015

Hmmm, my component is a wrapper, which could stand on its own, so I didn't set a default prop value. Bad practice? I guess if I had to set one then it should be null

@yannickcr
Copy link
Member

Since 2.0.0 children is no longer ignored for props validation.

If you still want to ignore it you can use the ignore option.

@matthewwithanm
Copy link
Contributor

@AsaAyers Won't node handle those cases too?

@AsaAyers
Copy link

yes, I think node is a better option than any, I just hadn't used it much at the time I commented.

@matthewwithanm
Copy link
Contributor

👍 Thanks, just making sure.

@reggi
Copy link

reggi commented Sep 8, 2015

Just to clarify, children should be defined as type node, like this:

propTypes: {
  children: React.PropTypes.node
}

shorttompkins added a commit to shorttompkins/react_sandbox that referenced this issue Sep 23, 2015
The this.props.children warning details can be found here:
jsx-eslint/eslint-plugin-react#7 (comment)
@resistdesign
Copy link

I know this is closed, but wouldn't children be inherited from Component? Or is OOP not a thing anymore?

@AsaAyers
Copy link

Not all Components have children. In a couple of my projects I have an <Avatar user={userObject} />

@joaomilho
Copy link

@resistdesign actually yeah, OOP is not the main thing anymore, at least in React's world.

@lencioni
Copy link
Collaborator

Why do you need oneOfType here? The node propType is defined as "Anything that can be rendered: numbers, strings, elements or an array (or fragment) containing these types." https://facebook.github.io/react/docs/reusable-components.html

You should use children: PropTypes.node.

@jdanilov
Copy link

Justin case you're wondering how to do that in Flow, use React.Node as described here:
https://flow.org/en/docs/react/types/#toc-react-node

type Props = {
  children: React.Node,
};

@Denly
Copy link

Denly commented Mar 15, 2018

PropTypes.node can be an array, so no need the React.PropTypes.arrayOf(React.PropTypes.node) @sebastienbarre

// Anything that can be rendered: numbers, strings, elements or an array
  // (or fragment) containing these types.
  optionalNode: PropTypes.node,
 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests