implications of shouldComponentUpdate on 'render callbacks'

[threepointone]

related to #4127 / https://discuss.reactjs.org/t/children-as-a-function-render-callbacks/626, this thread is to discuss and understand how shouldComponentUpdate should/could behave the render-callback pattern.

Again, an example, using react-state -

class Counters extends React.Component{
  render(){
    return <div id='A' onClick={() => this.setState({a: this.state.a + 1})}>
      <State initial={0}>{
        (val, set) =>
          <div id='B' onClick={() => set(val + 1)}>
            clicked A {this.state.a} times
            clicked B {val} times
          </div>
      }</State>  
    </div>;
  }
}

In the above example, when #B is clicked, everything updates as expected. However, when #A is clicked, and because State doesn't receive new props, then B risks having stale data, and won't reflect the changes in A's state. (This is technically correct, since the render callback here isn't 'pure')

There are 3 methods (that I can think of) to work around this.

• the render-callback-component (alternate name, please?) implements shouldComponentUpdate to always return true. This is my (current) preferred way, because the component is kinda equivalent to a stateful function wrapper on its 'children', and these children would have been rendered anyway. This is how react-state works (as also react-springs, etc)
• render callbacks must always be pure - dependent only on the arguments passed to them in the callback. this should also be fairly performant. the con is that the utility of this pattern diminishes a little.
• the render-callback-component accepts props that the render callback is 'dependent' on - this is sorta like method 2, except that these props don't have to be passed on to the callback itself, and are only used by the r-c-c to tell itself to update. Based on the above example, it could look like -

...
<State initial={0} depends={{a: this.state.a}}>{
  (val, set) =>
    <div id='B' onClick={() => set(val + 1)}>
      clicked A {this.state.a} times
      clicked B {val} times
    </div>
}</State> 
...

the con here is it's more fiddly. it also looks like ember's model dependency syntax, and that might not be too bad, the marking of explicit dependencies.

Thoughts / advice?

/cc @chenglou

[gaearon]

Another take on the third option:

...
<State initial={0}
       extraProps={{ a: this.state.a }}>{
  (val, set, extraProps) =>
    <div id='B' onClick={() => set(val + 1)}>
      clicked A {extraProps.a} times
      clicked B {val} times
    </div>
}</State> 
...

The difference is the third parameter that explicitly passes the data you say State depends on.

State would be able have a “normal” shouldComponentUpdate that shallowly compares props shallowly compares props and extraProps too. By default, it would still get different children on each render, but this is good because, if people don't know about that extraProps thing and casually use this.state inside the children callback, it should still “just work”.

When somebody has performance issues, they would be advised to turn the lambda into a class method. At this point they'll no longer have this, and will be forced to learn about the third extraProps parameter.

[threepointone]

very nice, I like this better too. also gives a way to transition a dumb scu => true component to a (possibly) more performant one by implementing the extraProps logic.

[sophiebits]

I don't think this is really a React issue. The discuss thread is a good place to keep talking about this.

[chenglou]

My bad about this, I've talked to @threepointone and suggested to create an issue for it. The initial goal is to turn this into something more actionable because it might require react-level support (assuming this pattern makes sense, I think it does). But I guess we can solve the problem fine without it. The same pattern @gaearon suggests can also work with scu with normal children passed to the wrapper.

[threepointone]

Sounds right. Thanks for the responses, folks.

[threepointone]

[for future readers]

The goal of #4127 / #4136 was to document potential patterns/problems with the 'children as a function' pattern. If you have any thoughts, please share on the react-discuss thread

[nkbt]

@threepointone just in case - these changes to my code let animations go smoothly without re-rendering content. So it is a combination of shouldComponentUpdate => false and moving things that should not change out of updated component.

[scottcheng]

Super late followup, but I don't quite understand why in the original example:

when #A is clicked, and because State doesn't receive new props, then B risks having stale data, and won't reflect the changes in A's state.

I think when #A is clicked, A rerenders given the new state, and State will also rerender because its children prop has changed, because children prop is an inline function ((val, set) => ...) and inline functions and creates a new identity each time it is evaluated.

Am I missing something? Or is this trying to solve for the generic case where the render callback may not be an inline function?

[sophiebits]

@scottcheng My understanding: @threepointone has shouldComponentUpdate always return true so that it does properly rerender but would prefer a more restrictive shouldComponentUpdate method such as by following one of the methods in the original bulleted list (but can't due to the inability to introspect closures).

[scottcheng]

@spicyj thanks for the reply. Sorry I wasn't being clear enough -- I was assuming State is a pure component, which shallow compares props and state in shouldComponentUpdate. Because props.children is an inline arrow function, which create new identities every time ((() => {}) === (() => {}) is false), State.shouldComponentUpdate will always return true. So in short I think even if State is a pure component, it should still work.

[benadamstyles]

Sorry to resurrect a very old thread but I came across this issue from Google, trying to understand how to make my render callback component more performant. I got very interested in @gaearon's suggestion, and had a go at implementing it. I've just published it to npm, just leaving a link to it here for anyone else who might come across this issue like I did, I'd also very much welcome any feedback/criticism: PureRenderCallbackComponent. Thanks!