Children prop gets recreated killing PureComponent optimizations

[davegri]

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

Report a possible bug

What is the current behavior?

When Component A renders Component B with a children prop that is a react component / JSX fragment any render of component A will recreate said children prop

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://jsfiddle.net or similar (template: https://jsfiddle.net/reactjs/69z2wepo/).

Example Code:

import  React from 'react';

class ComponentWithChildren extends React.PureComponent {
  render() {
    console.log("RENDER COMP")
    return <span>Hello{this.props.children}</span>
  }
}

class Children extends React.PureComponent {
  render() {
    return (
      <div>
        these are children
        <span>nested</span>
      </div>
    )
  }
}

class App extends React.PureComponent {

  update = () => this.setState({ count: this.state.count + 1 })

  state = {
    count: 0
  }

  render() {
    console.log("RENDER APP")
    return (
      <div>
        <button onClick={this.update}>Update</button>
        <ComponentWithChildren>
          <Children />
        </ComponentWithChildren>
      </div>
    )
  }
}

export default App;

What is the expected behavior?

I would expect ComponentWithChildren not to re-render because none of its props actually changed

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

React 15.4.1

[acdlite]

This is the expected behavior.

The reason ComponentWithChildren re-renders every time is because <Children /> === <Children /> always evaluates to false. This may seem unintuitive, but it's because <Children /> is sugar for React.createElement(Children), which returns a new object every time it is called.

To fix, you can cache the <Children /> element to ensure that the value is preserved across renders:

class App extends React.PureComponent {

  update = () => this.setState({ count: this.state.count + 1 })

  state = {
    count: 0
  }

  children = <Children />;

  render() {
    console.log("RENDER APP")
    return (
      <div>
        <button onClick={this.update}>Update</button>
        <ComponentWithChildren>
          {this.children}
        </ComponentWithChildren>
      </div>
    )
  }
}

[davegri]

Hey, thanks for the answer.
what would be the appropriate solution if <Children /> needed to receive props?

[gaearon]

If props are the same every time, you can hoist <Children prop={myProp} /> to a constant, just like in the example above.

If it receives different props every time then it’s not “the same” on updates. Therefore it makes sense for PureComponent to not skip rendering since otherwise you won’t see any changes below. However you are free to write a Component with a custom shouldComponentUpdate implementation if you want to use even more granular heuristics to determine whether to update.

[davegri]

@gaearon thanks for chiming in, were having a lot of trouble with optimizing these cases. One approach we have taken is passing a renderChildren function instead of passing children at all. However children is a very readable syntax and I don't want to be too quick throwing out the baby with the bathwater.

The problematic case is mainly when you have children that depend on props which are dynamic but that doesn't mean they always change, I don't want the children to be recreated when the props don't change.

I know this sounds a bit like overkill but out goal currently is not to have a single unnecessary render (render being just entering the render function)

[gaearon]

@davegri

You may be optimizing for the wrong thing. Running render() itself is not that expensive. The expensive part is recursively updating a large tree very often. If you can “cut off” fast updates at reasonable depth it’s fine for some components to always render.

It’s hard to offer more help without a specific example you’re struggling with.

[davegri]

@gaearon

I agree with you that our optimizations may be extreme, but assuming that we have decided we want this optimization. and we have code like:

<ComponentWithChildren>
  <span> this.props.text </span>
</ComponentWithChildren>

we don't want ComponentWithChildren's children prop to always change and cause a re-render, but only be recreated when this.props.text changes

as I said. one solution was

// on class
renderChildren = () => <span> this.props.text </span>

// in render
<ComponentWithChildren renderChildren={this.renderChildren} />

Like I said however, I dislike this approach because its much less readable.

I was wondering if you have any other solutions or suggestions?

[gaearon]

we don't want ComponentWithChildren's children prop to always change and cause a re-render, but only be recreated when this.props.text changes

If you just pass

<ComponentWithChildren>
  {this.props.text}
</ComponentWithChildren>

(with no span), that's exactly what would happen.

I don’t think the approach you suggested works:

// on class
renderChildren = () => <span> this.props.text </span>

// in render
<ComponentWithChildren renderChildren={this.renderChildren} />

In this case even if this.props.text changes, ComponentWithChildren doesn’t know, and forgets to re-render. It would re-render if you also happen to pass text itself as a prop:

<ComponentWithChildren renderChildren={this.renderChildren} text={this.props.text} />

But at this point you’re just duplicating information, and you might as well create <SpecificComponentWithChildren text={this.props.text} /> that is also a PureComponent and that renders <ComponentWithChildren><span>{this.props.text}</span></ComponentWithChildren>.

[davegri]

@gaearon

Your right about the function solution not working, thanks for that

That example was simple but the point is that I may have a more complex structure that can't be represented with a primitive. Is there no good way of solving this?

a real scenario from our code:

<SmallContactContainer
  userId={status === 'Reassigned' ? originalOwnerId : ownerId}
  retrieveQueue
  renderChildren={this.renderSmallContactContent}
>
  <div className={classes.dataItem}>
    {this.getStatusText()}
    {this.renderNewOwnerForReassigned()}
  </div>
</SmallContactContainer>

[gaearon]

Does what I suggested above in the last paragraph work for you? Just create a new component that encapsulates this render function, and make it a PureComponent.

[davegri]

I thought you were saying to encapsulate the children into a component, but yeah I guess thats a good solution, thanks :)

[ackvf]

@acdlite As you wrote, <Children/> === <Children/> evaluates to false as it's actually a new instance of Children and the render of parent component is called, but what happens with the <Children/> component's mounting? It doesn't get remounted, right?

[AlexGalays]

@ackvf

The instance reference changed but its type (and key) is still the same, so the underlying DOM Element needs no update and the same component remains mounted.

[ackvf]

So, the instance reference changes, but not a single lifecycle method is called on the new object, not even the constructor. What am I missing? (I haven't gone through the code yet - I find that difficult to start with.)