Description
I know this.context
is not officially there but quite a few libraries rely on it, and it seems like it's getting into shape with #2509.
I'm trying to understand how exactly shouldComponentUpdate
is supposed to be implemented with context
in mind. I noticed it accepts a third argument (nextContext
) and I can extend PureRenderMixin
to also check it:
shouldComponentUpdate: function(nextProps, nextState, nextContext) {
return !shallowEqual(this.props, nextProps) ||
!shallowEqual(this.state, nextState) ||
!shallowEqual(this.context, nextContext); // this will throw without context, read on
}
Components that don't opt into this.context
by not omitting contextTypes
will not get this third argument, which is understandable.
However this presents a problem when we have a <Middle />
component in between between <Top />
context owner and <Bottom />
context consumer. If <Middle />
implements a restrictive shouldComponentUpdate
, there is no way for <Bottom />
to react to <Top />
's context updates at all:
(fiddle)
var Bottom = React.createClass({
contextTypes: {
number: React.PropTypes.number.isRequired
},
render: function () {
return <h1>{this.context.number}</h1>
}
});
var Middle = React.createClass({
shouldComponentUpdate: function (nextProps, nextState, nextContext) {
return false;
},
render: function () {
return <Bottom />;
}
});
var Top = React.createClass({
childContextTypes: {
number: React.PropTypes.number.isRequired
},
getInitialState: function () {
return { number: 0 };
},
getChildContext: function () {
return { number: this.state.number };
},
componentDidMount: function () {
setInterval(function () {
this.setState({
number: this.state.number + 1
});
}.bind(this), 1000);
},
render: function() {
return <Middle />;
}
});
React.render(<Top />, document.body);
The same problem would occur if I tried to give Middle
a generic context-aware shouldComponentUpdate
as I wrote above, because Middle
has no this.context
unless it opts in.
This is possible to work around by adding contextTypes
to Middle
, but it doesn't look like a good solution. You'd need to explicitly add necessary contextTypes
on every level with smart shouldComponentUpdate
so it's too easy to slip up.
Will this be solved by #2112? Is there another solution in the meantime? What is the recommended way?
Activity
jimfb commentedon Nov 14, 2014
This is an extremely good question. Thanks for raising it!
This is a case where you're manually pruning/optimizing the tree recalculation. It seems to me that you'd need to request access to any context variables that are of relevance to your calculation. We could also consider doing other fancy things like pass in a hash of the full context (or even the full context) if necessary.
@sebmarkbage - thoughts?
gaearon commentedon Nov 14, 2014
Yeah, the problem is middle components don't know what context some distant child might need. There is no way they can know which
contextTypes
to declare to even be able to correctly implementshouldComponentUpdate
.gaearon commentedon Nov 14, 2014
I wonder if context propagation could happen in a separate tree traversal, without being blocked by falsy
shouldComponentUpdate
in the middle?Basically, when parent's context changes, it should mark all its descendants that receive this context as dirty, no matter how distant. Ancestor's context change should have the same effect as a state change for descendants who opt into this context—they should receive new
context
regardless of what their parents said.andreypopp commentedon Nov 14, 2014
@gaearon I think in that case you need to re-render everything and so
shouldComponentUpdate
won't have an effect of pruning subtrees. Otherwise context will be in inconsistent state with element tree.gaearon commentedon Nov 14, 2014
@andreypopp
I think that
shouldComponentUpdate
on middle components should have no effect on whether their descendants receive the new context, just like it has no effect on whether they receive new state:http://jsbin.com/geseduneso/2/edit?js,output
(If this was the case, both numbers would be incrementing)
Where is the inconsistency?
gaearon commentedon Nov 14, 2014
To put it differently, I think
context
should work exactly the same as if context owner had something like a “store” into which result of itsgetChildContext()
gets written incomponentWillUpdate
, and all descendant context consumers that declared keys from that context incontextTypes
, should receive that context it as if they were subscribed to that “store” and updated their own state.I don't mean the actual implementation—but I want to show that this model is not any more inconsistent than any Flux app with
shouldComponentUpdate
in the middle. Context should act like a “sideway storage”, but scoped to a subtree.gaearon commentedon Nov 14, 2014
Edit: this won't work because parent may change
I talked to Glenjamin on IRC and he convinced me changing context might be a bad idea per se. You lose ability to reason about why something was updated if some root update implicitly causes different child updates.
But then, the only reasonable solution I see is to completely forbid context changes. That is, make
getChildContext()
similar togetInitialState()
, which only gets called once before component is mounted.This would make context a hell lot simpler to think about. We can remove third parameter from
shouldComponentUpdate
since context never updates.In case when you need updates from context owner (e.g. like react-router wants
activeRoutes
to be passed top-down for usage inActiveState
mixin cc @mjackson), nothing prevents you from passing{ addChangeListener, removeChangeListener, getActiveRoutes }
incontext
. Descendants can now subscribe to changes and put them instate
.Is this a reasonable solution?
glenjamin commentedon Nov 14, 2014
I think the key question to answer is:
I've been using context for passing object references around quite happily, as I only ever write to those objects, not read. eg.
this.context.triggerAction("something")
jimfb commentedon Nov 14, 2014
The use case for context has been for parameters that are applicable across a potentially large subtree, but which you don't want to pass through more generic container nodes. An example might be a color theme, where a large number of nodes might listen to see if the background color is supposed to be white or black; you wouldn't want to pass those around everywhere as parameters.
Making getChildContext() behave more like getInitialState() wouldn't actually solve the problem, because you could always replace a parent node which provides a given context value with another parent node that provides a different context value. For the affected subtree, this is indistinguishable from mutating the context variable's value.
I think we can find a solution that avoids having users wire up change listeners.
I think @andreypopp may be right. Or at the very least, we need to provide a way for shouldComponentUpdate to know if anything in the context has changed, so it can decide to always return true if there was a context variable change.
I'll chat with @sebmarkbage later today and see what he thinks.
gaearon commentedon Nov 14, 2014
Ouch. You're totally right, I haven't considered this.
gaearon commentedon Nov 14, 2014
@JSFB On a second thought, I don't quite understand your comment. If you replace a parent node, entire child subtree will re-mount anyway, wouldn't it?
jimfb commentedon Nov 14, 2014
Currently, yes, it will re-mount, but that is partly an implementation detail. You can imagine (and we have been discussing the implementations and ramifications of) reparenting subtrees without loosing node state.
@sebmarkbage and I chatted, and came to the following conclusions:
@sebmarkbage, let me know if I missed anything!
Good discussions though! Thanks for all the feedback!
gaearon commentedon Nov 14, 2014
Thank you for taking time to discuss this!
If a top-level
Feed
component implementsshouldComponentUpdate
by usingPureRenderMixin
to avoid extra updates, it doesn't mean thatFeed
knows that somewhere inside it there is aCell
that happens to contain aLink
that depends on router's context.This gets even worse when frameworks (such as most popular React routing framework) use context and you may not even be aware of it. Somewhere, there are apps that don't change active link state because somebody optimized top-level components and had literally no idea they had to declare corresponding
contextTypes
to even getnextContext
in theirshouldComponentUpdate
.There is no way components are aware of all their possible descendants. Basically, if I move a context-dependant component anywhere inside a
PureRenderMixin
-enabled component, it will break but very subtly. Therefore, if you use context, the only way to avoid this subtle breakage of unrelated components is to never implementshouldComponentUpdate
which goes against what React is saying about it.Some frameworks on top of React, such as @swannodette's Om, make
PureRenderMixin
-ishshouldComponentUpdate
default, it's weird to cut them off like that. That means context-breakingshouldComponentUpdate
in every component for them.I agree this is not blocking for your work, but I can't agree that current situation is satisfactory if context is to be used at all. It's not just hard to implement
shouldComponentUpdate
with context now—it's downright impossible unless we make assumption that each component is always aware of what itschildren
could be.Popular libraries already rely on context heavily. I understand it's not a supported feature, but in my opinion it either needs to at least be possible to make it work with
shouldComponentUpdate
, or it should be disabled, and libraries should be forced to not use it, because it is subtly broken.sebmarkbage commentedon Nov 14, 2014
This has been known from the start with introduced contexts. We need to be able to have undocumented and unsupported features as experimental features to be able to iterate on them to find special cases. For example, if we didn't have contexts we wouldn't have known that they need to changed to be container-based instead of owner-based. Subtle breakage is part of the contract of using undocumented features.
I think what we need to do is by-pass
shouldComponentUpdate
if there's a new context anywhere in the parent tree. Potentially with ashouldUpdateChildContext
or something that determines if the entire subtree needs to reconcile.218 remaining items