React: Consider making <> pure unwrapped fragment

Created on 14 Dec 2017  路  7Comments  路  Source: facebook/react

Feature request

Currently if we pass Fragment with children to any custom component, this component gets that fragment as a child

const App = props => (
  <MyComponent>
    <Fragment>
      <div/>
      <div/>
      <div/>
    </Fragment>
  </MyComponent>
);

const MyComponent = props => {
  Children.count(props.children); // 1

  Children.map(props.children, item => {
    console.log(item.type === Fragment); // true
  });
};

That means in all components where children is being altered (cloned) we need to check first that each item is a fragment first, and if it is - take its children instead of itself.

This was a little unexpected, because after reading the documentation the impression was that Fragment is a pure abstraction on the caller side, that Fragment doesn't propagate down to components, but only its children.

In the same time it's understandable because Fragment might have more supported props in the future that receiving component might want to read.

But fragment shortcut <> will not have props, it will always mean pure abstraction (to group its children to show them by condition, for instance).
So maybe react could unwrap its children right away when elements are created and pass them down as set of children (merge with another children on the same level)?

React 16.2

Reconciler Discussion

Most helpful comment

I've stumbled across exactly the same issue.

For this to work I now need a render function like this:

render() {
  const {children} = this.props;

  const resolvedChildren =
    children.type === Fragment
      ? children.props.children
      : children;

  const enhancedChildren = Children.map(resolvedChildren, child =>
    React.cloneElement(child, {onClick: this.onClick})
  );

  render enhancedChildren;
}

I'm not sure if that children.type === Fragment check is safe, same about the access to children.props.children. I was also expecting that a fragment is resolved by using Children.map and friends, so I wouldn't have to do this check.

All 7 comments

I've stumbled across exactly the same issue.

For this to work I now need a render function like this:

render() {
  const {children} = this.props;

  const resolvedChildren =
    children.type === Fragment
      ? children.props.children
      : children;

  const enhancedChildren = Children.map(resolvedChildren, child =>
    React.cloneElement(child, {onClick: this.onClick})
  );

  render enhancedChildren;
}

I'm not sure if that children.type === Fragment check is safe, same about the access to children.props.children. I was also expecting that a fragment is resolved by using Children.map and friends, so I wouldn't have to do this check.

As far as I understand, this part should be patched to also consider fragments for this to work: https://github.com/facebook/react/blob/cc52e06b490e0dc2482b345aa5d0d65fae931095/packages/react/src/ReactChildren.js#L149-L160

If that's the case, can I submit a PR?

My understanding is this was an intentional decision. Paging @acdlite

Also @clemmy might have context on why this behavior was chosen.

As you mentioned, the children traversal semantics for <Fragment></Fragment> treats it as an element. The <></> syntax was created to be shorthand for <Fragment></Fragment>, so with that as the pretext, the same semantics exist for <></> since it鈥檚 simply de-sugaring to <Fragment></Fragment>. With your proposed changes to the semantics, switching from <></> to <Fragment></Fragment> will no longer be a non-breaking change. I think it鈥檚 an interesting idea, but the fact that it鈥檒l take a different component to implement (leading to more complexity), and breaks compatibility between <></> and <Fragment></Fragment> is not very compelling to me.

In response to @amannn, that patch will change the semantics for both <Fragment></Fragment> and <></>, which is a separate behaviour to what @klimashkin is requesting (only behavioural change for <></>).

I think changing behavior for both might be reasonable.
But we won't get much further in an issue here.
Please submit an RFC with your proposal: https://github.com/reactjs/rfcs

I could not find an existing RFC on this subject so I have created one myself

Was this page helpful?
0 / 5 - 0 ratings