React: Children.only can not be called on result of Children.map

[jimfb]

Children.only is intended to be called on an opaque Children object (eg. this.props.children). Best as I can tell, Children.map is intended to return an opaque Children object. But Children.only can't be called on the return value of Children.map, as per https://github.com/facebook/react/issues/4410#issuecomment-122199087

It seems very reasonable to want to map your only child to another element (clone element, for instance), and then grab the only child from the map and use it to render. It is also very surprising for the identity operation to be passed into map, but the thing that comes out to not be semantically equivalent.

Issue demonstrated here: https://jsfiddle.net/9Ldyq5jk/4/

[sebmarkbage]

The intention of Children.only is to be used with the result of JSX where the child is guaranteed to be a single child. The reason for it is as an upgrade safety if JSX would change this behavior in the future.

[jimfb]

@sebmarkbage But isn't the intent of map that it return a data structure that's semantically equivalent to this.props.children? Specifically, the opaque Children data structure? Otherwise, isn't the return value of map completely useless? / how do you use it? What's the contract for the return type of map?

[sebmarkbage]

type SingleChild = ReactElement;
type SetOfChildren = Array<OpaqueChildren> | Map<string, OpaqueChildren>;
type OpaqueChildren = SingleChild | SetOfChildren;

JSX produces a children property:

createElement(type, props) : { props: {} }
createElement(type, props, child : SingleChild) : { props: { children : SingleChild } }
createElement(type, props, child : OpaqueChildren) : { props: { children : OpaqueChildren } }
createElement(type, props, child1 : SingleChild, child2 : SingleChild) : { props: { children : OpaqueChildren } }

only(children : OpaqueChildren) : SingleChild is a downcast that "warns" for the wrong type.

map(children : OpaqueChildren, fn) : OpaqueChildren doesn't promise to return anything that is compatible SingleChild. However, it is always compatible as a child of a div for example because it is compatible with createElement.

[jimfb]

Well, the documentation states:

Return the only child in children. Throws otherwise.

And since the signature is only(children : OpaqueChildren) (as per your comment above), it means that at the very least the documentation is wrong, since the function is not returning the only child in children.

I think the downcast behavior is unnecessarily complicated and incorrectly documented. At the very least we should fix the documentation, but my preferred solution is to fix the implementation. Regardless of our solution, I think this is a valid bug.

[sebmarkbage]

The point is that the whole thing is unnecessarily complicated since it is purely a perf optimization and an upgrade path if the perf optimization becomes unnecessary. However, people are likely relying on this.props.children being an element anyway, since we don't enforce it through a static type system. So I think that this whole utility is probably unnecessary.

[gaearon]

Sometimes, when I want a component to have no DOM of its own (and only have some lifecycle logic), I write

render() {
  return React.Children.only(this.props.children);
}

to enforce the one-childness ASAP. Otherwise I assume the error's going to be more cryptic to the consumer if they put several elements as children of my “container component”.

Am I misusing React.Children.only? Is it meant for a different use case?

[jimfb]

@sebmarkbage Ok, well, I've made my points here. I still feel like there is some sort of action item required (deprecate, fix, or document).

@gaearon It is my understanding that you are correctly using the React.Children.only helper in your example.

[LegNeato]

FWIW I just hit this and was about to file an issue as it seemed pretty broken to me too :speak_no_evil:.

:heart: This code is straightforward and seems "correct" as far as the documentation is concerned (treating this.props.children as opaque and using React.Children to manipulate):

_renderInfo = (): any => {
  let infoChildren = React.Children.map(
    this.props.children,
    (child: ?Object): ?Object => {
      if (child.type.name === MyInfoComponent.name) {
        return child;
      }
    },
  );
  // If we have a MyInfoComponent child, render it. Throw if multiple, used wrong.
  if (React.Children.count(infoChildren) > 0) {
    return React.cloneElement(
      React.children.only(infoChildren),
      {
        hidden: this.state.infoHidden,
      },
    );
  }
};

It's a nice pattern to look for specific optional children and make sure there is only one of a certain type (so that siblings can be in any order). But doing it this way throws an error because of the one element array (hence this issue).

:hankey: This code looks worse to me as it is relying on the underlying implementation of this.props.children and has more lines...yet it works fine with no error:

_renderInfo = (): any => {
  let infoChildren = React.Children.map(
    this.props.children,
    (child: ?Object): ?Object => {
      if (child.type.name === MyInfoComponent.name) {
        return child;
      }
    },
  );
  // If we have a MyInfoComponent child, render it. Throw if multiple, used wrong.
 let numInfo = React.Children.count(infoChildren);
 if (numInfo > 1) {
   // throw
 } else if (numInfo === 1) {
    return React.cloneElement(
      infoChildren[0],
      {
        hidden: this.state.infoHidden,
      },
    );
  }
};

Note the replacement of React.children.only(infoChildren) with infoChildren[0] in cloneElement.

:beers: More than happy to put up a patch if there is consensus on what we should do (and if something should be done even).

[epsitec]

Shouldn't React.Children.only(x) be replaced by this function?

function universalChildrenOnly(children) {
  return (Array.isArray (children) && children.length === 1)
    ? children[0]
    : React.Children.only (children);
}

Still not sure what to do (see also https://github.com/facebook/react/issues/6156).

[sebmarkbage]

This is still helpful as a "safe" downcast for type systems so it's not great to deprecate it. However, it is also meant to effectively remove all runtime information. The runtime type check is not great to have for perf when you don't need it.

That runtime check is also not equivalent to React.Children.toArray(x)[0] because of how keys are preserved in nested arrays.

I think the fix is to fix the docs.

[adamhenson]

I'm seeing an outcome of this issue in react-router-bootstrap, when using React.createElement directly.

https://github.com/react-bootstrap/react-router-bootstrap/issues/195

If React.createElement expects an array data type as the children argument, then I don't see how it makes any sense why React.Children.only should be anything but what @epsitec suggested.

UPDATE: Based on the documentation, I was under the impression that the children argument received by React.createElement needs to be an array data type. Unfortunately, if there is a single child - in order to accommodate code that uses React.Children.only - this needs to be a React element object.

Yikes. So, I suppose I need to hack my code that uses React.createElement to accommodate this.

I'll need to do something like the following:

if (Array.isArray(children) && children.length === 1) {
    children = children[0];
}

return React.createElement(type, props, children);

[Thinkscape]

This function is a mess...

The docs say:

Returns the only child in children. Throws otherwise.

Which suggests that I can pass it the children object (like I do for every other React.Children.* method) and it will give me the first and only child.

The docblock for the function states:

/**
 * Returns the first child in a collection of children and verifies that there
 * is only one child in the collection.
 *

Which further confirms that's the expected behaviour...

But in reality it does none of those things. Instead, it just verifies that the arguments is a ReactElement 😕

[bvaughn]

The documentation and source code for reactjs.org now lives in a different repository: reactjs/reactjs.org. (For more info on why we made this move, see issue #11075.)

I've moved this issue to the new repo: reactjs/reactjs.org#87