Skip to content

sCU doesn't prevent child rerender when legacy context and batched setStates are involved #13920

Closed
@errendir

Description

@errendir

Do you want to request a feature or report a bug?
This is a bug report. I think this bug and one already reported (#13856) have the same root cause.

What is the current behavior?
A component instance preventing its own rerender, by returning false from shouldComponentUpdate may under certain condition still get its children rerendered, even if no direct update to said children happened.

This behaviour is triggered by a very specific stack of components. The simplest example I managed to create (https://codesandbox.io/s/9zymvq479r) involves five components, each rendering the next one:

  • UpdatingRoot responsible for triggering the top-level update, by calling setState in the componentDidMount lifecycle method.
  • LegacyContextInjector responsible for creating the child context object according to the legacy context API.
  • SCUBarrier with shouldComponentUpdate function always returning false. Any children of this component instance should not be rerendered as long as they themselves are not sheduled for an update. This is the behaviour I expect. This bug prevents it from happening.
  • IntermediateComponent responsible for rendering UpdatingChild few levels down in the component instance tree.
  • UpdatingChild responsible for calling its own setState in the componentDidMount lifecycle method.

Each of the created component instances will be rendered only one or two times. Both UpdatingRoot and UpdatingChild are expected to be rendered twice, and this does happen. Each of the IntermediateComponent instances is expected to render only once. This expectation is based on the fact that the SCUBarrier#shouldComponentUpdate always returns false, and no child IntermediateComponent instance updates itself. The legacy context API should not pierce through shouldComponentUpdate returning false (https://reactjs.org/docs/legacy-context.html#updating-context).

Despite those expectation, all IntermediateComponent instances are rendered twice.

It's important that the UpdatingRoot#setState and UpdatingChild#setState function are batched together. You can try "unbatching" them, by swapping:

this.setState({ weAreHome: "my friend" });

for

setTimeout(() => this.setState({ weAreHome: "my friend" }),0)

When this is done the IntermediateComponent instances are correctly rendered only once.

My example code does the batching by calling UpdatingRoot#setState and UpdatingChild#setState functions directly in the componentDidMount lifecycles of their respective components. I believe this generally is an anti-pattern, but this approach was chosen for simplicity. The same issue can be replicated by calling both setState in ReactDOM.unstable_batchedUpdates(() => { ... })

For a more practical example of a situation in which two ancestor-descendant components may naturally schedule updates in a batch, take a look at https://github.com/tappleby/redux-batched-subscribe.

Legacy context API is used by popular components like Transition from https://github.com/reactjs/react-transition-group or by commonly used react-redux.

A real-life example of this bug could be:
Two Redux-connected component A and C are scheduled for update in the same batch. A Transition (from react-transition-group) injecting legacy context is rendered between them. Some component B in between A and C (descendent of A, ancestor of C) implements shouldComponentUpdate to optimize the rendering of some unchanging subtree (which eventually, down the line, renders B). Both A and C component instances rerender correctly (as expected, since react-redux uses legacy context API correctly as described in https://medium.com/@mweststrate/how-to-safely-use-react-context-b7e343eff076). However the instances corresponding to the unchanging element tree returned by B will also be unexpectedly rerendered.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem.
https://codesandbox.io/s/9zymvq479r

What is the expected behavior?
All IntermediateComponent instances should be rendered only once.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
react "16.6.0-alpha.f47a958"
react-dom "16.6.0-alpha.8af6728 - canary"

I originally discovered this issue in the React/ReactDOM version 16.5.2

Please let me know if there is something unclear about this bug. It's definitely not an easy thing to reproduce, since so many different components need to be involved!

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions