Merge pull request #4221 from jimfb/ryans-context-bug

updateComponent should update the context iff it has changed

examples/basic-jsx/index.html

@@ -29,23 +29,54 @@
     <script src="../../build/react.js"></script>
     <script src="../../build/JSXTransformer.js"></script>
     <script type="text/jsx">
-      var ExampleApplication = React.createClass({
-        render: function() {
-          var elapsed = Math.round(this.props.elapsed  / 100);
-          var seconds = elapsed / 10 + (elapsed % 10 ? '' : '.0' );
-          var message =
-            'React has been successfully running for ' + seconds + ' seconds.';
 
-          return <p>{message}</p>;
-        }
+var Top = React.createClass({
+  childContextTypes: {
+    now: React.PropTypes.any
+  },
+ 
+  getChildContext: function() {
+    return {
+      now: this.state.now
+    };
+  },
+ 
+  getInitialState: function() {
+    return {
+      now: Date.now()
+    };
+  },
+ 
+  componentDidMount: function() {
+    setInterval(function() {
+      this.setState({
+        now: Date.now()
       });
-      var start = new Date().getTime();
-      setInterval(function() {
-        React.render(
-          <ExampleApplication elapsed={new Date().getTime() - start} />,
-          document.getElementById('container')
-        );
-      }, 50);
+    }.bind(this), 500);
+  },
+ 
+  render: function() {
+    return (
+      <div>
+        <h1>{this.state.now}</h1>
+        <Child />{this.props.children}
+      </div>
+    );
+  }
+});
+ 
+var Child = React.createClass({
+  contextTypes: {
+    now: React.PropTypes.any
+  },
+ 
+  render: function() {
+    // I never update
+    return <h2>{this.context.now}</h2>;
+  }
+});
+ 
+React.render(<Top><Child/></Top>, document.getElementById('container'));
     </script>
   </body>
 </html>

src/renderers/shared/reconciler/ReactCompositeComponent.js

@@ -521,19 +521,18 @@ var ReactCompositeComponentMixin = {
   ) {
     var inst = this._instance;
 
-    var nextContext;
+    var nextContext = this._context === nextUnmaskedContext ?
+      inst.context :
+      this._processContext(nextUnmaskedContext);
     var nextProps;
 
     // Distinguish between a props update versus a simple state update
     if (prevParentElement === nextParentElement) {
-      nextContext = inst.context;
       // Skip checking prop types again -- we don't read inst.props to avoid
       // warning for DOM component props in this upgrade
       nextProps = nextParentElement.props;
     } else {
-      nextContext = this._processContext(nextUnmaskedContext);
       nextProps = this._processProps(nextParentElement.props);
-
       // An update here will schedule an update but immediately set
       // _pendingStateQueue which will ensure that any state updates gets
       // immediately reconciled instead of waiting for the next batch.

src/renderers/shared/reconciler/ReactReconciler.js

@@ -65,8 +65,10 @@ var ReactReconciler = {
     internalInstance, nextElement, transaction, context
   ) {
     var prevElement = internalInstance._currentElement;
-
-    if (nextElement === prevElement && nextElement._owner != null) {
+    if (nextElement === prevElement &&
+        nextElement._owner != null
+        // TODO: Shouldn't we need to do this: `&& context === internalInstance._context`
+      ) {
       // Since elements are immutable after the owner is rendered,
       // we can do a cheap identity compare here to determine if this is a
       // superfluous reconcile. It's possible for state to be mutable but such
@@ -74,6 +76,9 @@ var ReactReconciler = {
       // the element. We explicitly check for the existence of an owner since
       // it's possible for an element created outside a composite to be
       // deeply mutated and reused.
+
+      // TODO: Bailing out early is just a perf optimization right?
+      // TODO: Removing the return statement should affect correctness?
       return;
     }
 

src/renderers/shared/reconciler/__tests__/ReactCompositeComponent-test.js

@@ -605,6 +605,67 @@ describe('ReactCompositeComponent', function() {
     expect(React.findDOMNode(component).innerHTML).toBe('bar');
   });
 
+  it('should pass context when re-rendered for static child', function() {
+    var parentInstance = null;
+    var childInstance = null;
+
+    var Parent = React.createClass({
+      childContextTypes: {
+        foo: ReactPropTypes.string,
+        flag: ReactPropTypes.bool,
+      },
+
+      getChildContext: function() {
+        return {
+          foo: 'bar',
+          flag: this.state.flag,
+        };
+      },
+
+      getInitialState: function() {
+        return {
+          flag: false,
+        };
+      },
+
+      render: function() {
+        return React.Children.only(this.props.children);
+      },
+    });
+
+    var Middle = React.createClass({
+      render: function() {
+        return this.props.children;
+      },
+    });
+
+    var Child = React.createClass({
+      contextTypes: {
+        foo: ReactPropTypes.string,
+        flag: ReactPropTypes.bool,
+      },
+
+      render: function() {
+        childInstance = this;
+        return <span>Child</span>;
+      },
+    });
+
+    parentInstance = ReactTestUtils.renderIntoDocument(
+      <Parent><Middle><Child /></Middle></Parent>
+    );
+
+    expect(parentInstance.state.flag).toBe(false);
+    reactComponentExpect(childInstance).scalarContextEqual({foo: 'bar', flag: false});
+
+    parentInstance.setState({flag: true});
+    expect(parentInstance.state.flag).toBe(true);
+
+    expect(console.error.argsForCall.length).toBe(0);
+
+    reactComponentExpect(childInstance).scalarContextEqual({foo: 'bar', flag: true});
+  });
+
   it('should pass context transitively', function() {
     var childInstance = null;
     var grandchildInstance = null;

src/renderers/shared/reconciler/__tests__/ReactUpdates-test.js

@@ -347,14 +347,14 @@ describe('ReactUpdates', function() {
 
       render: function() {
         numMiddleRenders++;
-        return <div>{this.props.children}</div>;
+        return React.Children.only(this.props.children);
       },
     });
 
     var Bottom = React.createClass({
       render: function() {
         numBottomRenders++;
-        return <span />;
+        return null;
       },
     });
 
@@ -463,7 +463,6 @@ describe('ReactUpdates', function() {
 
       expectUpdates(desiredWillUpdates, desiredDidUpdates);
     }
-
     testUpdates(
       [root.refs.switcher.refs.box, root.refs.switcher],
       // Owner-child relationships have inverse will and did