Eslint-plugin-react: Clarify the documentation for `no-children-prop`

[vdh]

Children should always be actual children, not passed in as a prop.

When using JSX, the children should be nested between the opening and closing tags. When not using JSX, the children should be passed as additional arguments to React.createElement.

The current documentation only says _how_ to comply with the rule, but there hasn't been a clear rational provided for why the prop should be banned. The React docs haven't mentioned any issues with or preferences against providing the children prop via props, and the additional arguments to React.createElement are just automatically mixed into the props as a convenience.

Obviously these scenarios are needlessly messy and unintuitive:

<MyComponent children={<AnotherComponent />} />;
<MyComponent children={['Child 1', 'Child 2']} />;
<MyComponent children="Foo">Bar</MyComponent>;

But why are these notations:

<MyComponent baz="qux" children="Foo" />;
(
  <MyComponent>
    <AnotherComponent baz="qux" children="Foo" />
  </MyComponent>
);
React.createElement(MyComponent, { baz: "qux", children: "Foo" });
React.createElement(
  MyComponent,
  null,
  React.createElement(AnotherComponent, { baz: "qux", children: "Foo" })
);

less valid than these ones:

<MyComponent baz="qux">Foo</MyComponent>;
(
  <MyComponent>
    <AnotherComponent baz="qux">Foo</AnotherComponent>
  </MyComponent>
);
React.createElement(MyComponent, { baz: "qux" }, "Foo");
React.createElement(
  MyComponent,
  null,
  React.createElement(
    AnotherComponent,
    { baz: "qux" },
    "Foo"
  )
);

?

[ljharb]

JSX is like HTML. Children are a special form, that in jsx is provided using nesting, and in createElement is provided using the third argument.

Certainly you can provide the "children" prop, and it works the same - but that's more an artifact of React not wanting to decrease performance, and the implementation detail of "children are seen in the props object of the rendered component" rather than something that should be encouraged.

Additionally, if there exist _some_ scenarios that are messy and unintuitive (with the children prop), and there exists an alternate form that is always clean and intuitive (with nested children), it's strictly better to use a linter rule to force the alternate form. Things that are useful but sometimes confusing, should be avoided when a clearer alternative exists.

[vdh]

Your third point is a much clearer and more convincing rational for the rule than the first two.

The first two seem to contradict the opening paragraphs of the React docs "JSX in Depth":

JSX is a JavaScript syntax extension that looks similar to XML. You can use a simple JSX syntactic transform with React.

Why JSX?

You don't have to use JSX with React. You can just use plain JS. However, we recommend using JSX because it is a concise and familiar syntax for defining tree structures with attributes.

It's more familiar for casual developers such as designers.

XML has the benefit of balanced opening and closing tags. This helps make large trees easier to read than function calls or object literals.

It doesn't alter the semantics of JavaScript.

I find the phrasing of children being "special" to be unhelpful without better context, and referring to the arguments on React.createElement is also not very clear.

I'm fine with it being a style or code cleanliness choice. I understand there must be important reasons _why_ this rule was created and added as a default recommended rule, but the relatively-silent introduction and documentation are essentially a black box. I rely on the documentation of ESLint rules to better understand why I get errors and what I might be doing wrong, and it's been frustrating trying to get a clear explanation of the choices behind this rule.

[ljharb]

XML and HTML and JSX are all similar to each other.

I agree the React documentation does not indicate that "children" is special; I'm suggesting that it's special treatment in the API is part of what makes it so.

PRs are always welcome to improve documentation.

[piuccio]

Came here because as of eslint-plugin-react: 7.0.0 the rule no-children-prop is enabled by default.

I kinda agree with the general principle, however react router 4 uses (or abuses?) the children prop.

They provide the example

<Route path={to} children={({ match }) => (
  <li className={match ? 'active' : ''}>
    <Link to={to} {...rest}/>
  </li>
)}/>

which is nicer than

<Route path={to}>
  <Nested />
</Route>

const Nested = ({ match }) => (
  <li className={match ? 'active' : ''}>
    <Link to={to} {...rest}/>
  </li>
)};

note that you can't use the following because the props passed to the children are different from the one of the parent

<Route path={to}>
  <li className={match ? 'active' : ''}>
    <Link to={to} {...rest}/>
  </li>
<Route />

What's your thoughts about this? In the meantime I'm disabling the rule

[vdh]

@piuccio I'm (obviously) all for children props, but it looks like you're going against jsx-no-bind by re-declaring the same function on every render?

[piuccio]

I guess jsx-no-bind is not enabled by default because linting doesn't complain about that, by the way I'm using

"extends": [
    "airbnb-base",
    "plugin:react/recommended"
  ],

Again, the same principle applies, if most examples (from trusted sources) go against the recommended rules, maybe it's the rules that need updating?

[vdh]

@piuccio I would recommend moving from the airbnb-base preset to the main airbnb one. You're missing out on all the rules enabled by the main preset.

The jsx-no-bind rule is there to help you avoid performance issues caused by re-creating the same function over and over when you don't need to be. There's no logical _reason_ to go against js-no-bind. Example code should not be taken as gospel.

[vdh]

@piuccio In fact, after a second look, that documentation is both needlessly violating no-use-before-define for no good reason, and mixing JSX & JS next to each other without really clarifying between the two. I'm big fan of react-router, but that example code is _atrocious_.

[ljharb]

Passing non-elements as children is a really weird pattern.

[vdh]

@ljharb Agreed. I feel like someone with a tea light who set off the smoke alarm, seeing someone set up an indoor campfire next to me. It makes me start to reconsider if my tea light (strings as children props) is even worth the metaphorical fire hazard now…

[ljharb]

Either way, the following should work fine:

<Route path={to}>
  {({ match }) => (
    <li className={match ? 'active' : ''}>
      <Link to={to} {...rest}/>
    </li>
  )}}
</Route>

[vdh]

@ljharb But isn't that just a functional stateless component?

const SomeRoute = ({ match }) => (
  <li className={match ? 'active' : ''}>
    <Link to={to} {...rest} />
  </li>
);
// …
const SomeParent = () => (
  {/* … */}
  <Route path={to}>
    <SomeRoute />
  </Route>
  {/* … */}
);

The more I read those docs for Route the more confusing it gets. They seem to have three ways (component, render, children) for doing almost exactly same thing…?

[ljharb]

Right - it's a component, not an element. The equivalent would be {SomeRoute}, not <SomeRoute />.

[vdh]

I'm recanting this issue. I've seen the light that children props create way too much syntactic and logistical hassles than they're worth.

[cruzortiz99]

https://github.com/yannickcr/eslint-plugin-react/issues/891#issuecomment-300395219 this does work just fine