How to implement shouldComponentUpdate with this.context?

[gaearon]

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?

[jimfb]

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]

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 implement shouldComponentUpdate.

[gaearon]

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]

@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]

@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]

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 its getChildContext() gets written in componentWillUpdate, and all descendant context consumers that declared keys from that context in contextTypes, 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]

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 to getInitialState(), 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 in ActiveState mixin cc @mjackson), nothing prevents you from passing { addChangeListener, removeChangeListener, getActiveRoutes } in context. Descendants can now subscribe to changes and put them in state.

Is this a reasonable solution?

[glenjamin]

I think the key question to answer is:

In what scenarios is passing data through context preferable to passing data through props or triggering an event to make a component call its own setState.

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]

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]

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.

Ouch. You're totally right, I haven't considered this.

[gaearon]

@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]

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:

• shouldComponentUpdate is a complex escape hatch, the implementation of which requires you to have a solid understanding of what is happening under the component. For this reason, it's not unreasonable to assume you know which context variables are being used, since you already need to know which properties are being used and how they're being used.
• This change probably isn't making the situation much worse than it already was, and it's an improvement over using the old "owner" relationship. Therefore, this issue is still worth discussion, but is probably non-blocking.
• Contexts are a complex feature/experiment at this point, and we're probably going to need to do more thinking before we officially support/document them. As we learn more, we'll likely continue to iterate on this topic.

@sebmarkbage, let me know if I missed anything!

Good discussions though! Thanks for all the feedback!

[gaearon]

Thank you for taking time to discuss this!

it's not unreasonable to assume you know which context variables are being used, since you already need to know which properties are being used and how they're being used.

If a top-level Feed component implements shouldComponentUpdate by using PureRenderMixin to avoid extra updates, it doesn't mean that Feed knows that somewhere inside it there is a Cell that happens to contain a Link 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 get nextContext in their shouldComponentUpdate.

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 implement shouldComponentUpdate which goes against what React is saying about it.

Some frameworks on top of React, such as @swannodette's Om, make PureRenderMixin-ish shouldComponentUpdate default, it's weird to cut them off like that. That means context-breaking shouldComponentUpdate 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 its children 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]

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 a shouldUpdateChildContext or something that determines if the entire subtree needs to reconcile.