Description
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 callingsetState
in thecomponentDidMount
lifecycle method.LegacyContextInjector
responsible for creating the child context object according to the legacy context API.SCUBarrier
withshouldComponentUpdate
function always returningfalse
. 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 renderingUpdatingChild
few levels down in the component instance tree.UpdatingChild
responsible for calling its ownsetState
in thecomponentDidMount
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!