React: this.props.children

[andrevenancio]

Do you want to request a feature or report a bug?
FEATURE?

What is the current behavior?
AMBIGUOUS.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem. Your bug will get fixed much faster if we can run your code and it doesn't have dependencies other than React. Paste the link to your JSFiddle (https://jsfiddle.net/Luktwrdm/) or CodeSandbox (https://codesandbox.io/s/new) example below:

What is the expected behavior?

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
EVERYTHING

Hi all.

Maybe this isn't a feature request, but more an API choice which I would like to put it the open and maybe try to understand the decision that led to the implementation. So apologies in advance if I just don't get it :)

Why does this.props.children either render a Child or an array of children? Why can't this be an array of children all the time?

Take this as an example of a Parent class:

import React, { PureComponent } from 'react';
class ParentExample extends PureComponent {
    render() {
        return (
            <div>
                { this.props.children.map((Child, i) => (
                    <Child key={i} />
                ))
                }
            </div>
        );
    }
}

Now if you do something like:

return (
    <ParentExample>
        <SomeComponent />
        <SomeOtherComponent />
    </ParentExample>
);

You have no problems, cause your children prop inside the ParentExample is an array. However if you decide that you just want to pass 1 child into it like so:

return (
    <ParentExample>
        <SomeComponent />
    </ParentExample>
);

You'll get an error because the children prop inside the ParentExample is now a direct reference to the SomeComponent instead of an array of children.

Doesn't it make more sense to always return an array?

Having to do this sanitation all the time before rendering the children is a bit silly... Now I might totally be missing the point here, but shouldn't the name itself be a clue? this.props.children children... an array of children. Can be 0, can be 1 can be N...

Opinions?

[NE-SmallTown]

Doesn't it make more sense to always return an array?

IMO, no, it's intended. It provides more flexible to us. Both render children or new context api(and many other things) relay on this.

[andrevenancio]

Hi @NE-SmallTown 👋, I understand that changing it right now will break a lot of things, my conversation is more on why this decision was made and not as much as why we can't change it now. IMHO Relying on something doesn't necessarily mean they are in "concept" correct.

[gaearon]

You want to use React.Children.map(this.props.children, fn) instead.

The initial reason for this is to avoid the extra array allocation for every single child (which is the most common case). Small costs like this add up in large hierarchies.

Another reason is that we'd generally like you to treat this.props.children as an opaque data structure. We might to further optimize how it's being stored later, and don't intend its specific form to be considered a public API.

Finally just making it an array won't solve your problem. For example you could have nested children arrays, and you'd probably handle this case incorrectly if you just used map(). This is exactly why React.Children.map() exists that handles all these cases and does the right thing.

[mcjazzyfunky]

@gaearon Thanks for your detailed answer. Even if this issue is closed, please allow me to ask some further questions regarding the term "opaque data structure" (I've asked something like that in some older issue already, but the answers was not completely satisfying there - so I hope it's not annoying to ask again - btw. your answer will help to close an open issue at the reactjs.org repo):

"Opaque data structure" means IMHO that you must not make any assumptions about the concrete nature of the data structure (you've wirtten above: "[...] don't intend its specific form to be considered a public API"), but....

1. In this two lines Props.children seems to have a very concrete type - is this an error (or at least "suboptimal")?:

   https://github.com/DefinitelyTyped/DefinitelyTyped/blob/5f7be1fd628aaa36a1414d6ba59df8ff03ca3f1e/types/react/index.d.ts#L768

   https://github.com/DefinitelyTyped/DefinitelyTyped/blob/5f7be1fd628aaa36a1414d6ba59df8ff03ca3f1e/types/react/index.d.ts#L152

2. If the data structure of props.children is opaque, shouldn't something like the following (here some "function as child" pattern) considered as bad code as it makes assumptions about the data structure of props.children?:

    function SomeComponent(props) {
      [...]
        <div>{props.children(...someArgs)}</div>
        // shouldn't this be something like:
        // <div>{React.Children.only(props.children)(...someArgs)}</div>
      [...]
    }

    SomeComponent.propTypes = {
      [...]
      children: PropTypes.func.isRequired
      // shouldn't this be something like:
      // PropTypes.childOf(PropTypes.func).isRequired
      // (of course there's no "PropTypes.childOf" function)
      [...]
    }

Thanks a lot in advance for the clarification.

[andrevenancio]

I do agree with @mcjazzyfunky. Then calling it children it's semantically incorrect in my opinion. if a property can be a child or a set of children or something else in the future.

I understand that making it an array can lead to more memory usage, however, are there any tests that prove that initializing an array that holds children is, in fact, a measurable performance decrease? Many libraries using scene graph's like three.js or pixi.js are organized in this way.

If this is not meant to be a public API, maybe it should be cleared on the documentation and across examples where {this.props.children} is vastly used and mentioned. You could also arguably move the this.props.childen out of the "public API" to a more "hidden" _reactInternalFiber location. And let users only render children through React.Children.map(this.props.children, fn) method as suggested.

Anyway, appreciate the clarification earlier, and the tremendous amount of work put into the library.
I do understand it can be a petty low priority issue, but without clarification on the documentation can lead to people like me coming to github complaining about life :)

[gaearon]

In this two lines Props.children seems to have a very concrete type - is this an error (or at least "suboptimal")

I'll go with "suboptimal". :-) Yes, ideally it would be nice for those to be something that you can only deal with via React.Children.

If the data structure of props.children is opaque, shouldn't something like the following (here some "function as child" pattern) considered as bad code as it makes assumptions about the data structure of props.children?

This is a fair point and I think you're right this would conceptually be better.

That ship has somewhat sailed. But there is some ongoing work (e.g. compilation experiments) that might make this more important again. If this happens we'll reevaluate the importance of this and maybe start enforcing it (or maybe not).