Fix context warning with non-pure getChildContext

Fixes facebook#3709.
sophiebits · Apr 21, 2015 · a8c292a · a8c292a
1 parent 4053465
commit a8c292a
Show file tree

Hide file tree

Showing 3 changed files with 64 additions and 17 deletions.
diff --git a/src/classic/__tests__/ReactContextValidator-test.js b/src/classic/__tests__/ReactContextValidator-test.js
@@ -218,20 +218,16 @@ describe('ReactContextValidator', function() {
     });
 
     ReactTestUtils.renderIntoDocument(<Component testContext={{bar: 123}} />);
-    expect(console.warn.argsForCall.length).toBe(2);
+    expect(console.warn.argsForCall.length).toBe(1);
     expect(console.warn.argsForCall[0][0]).toBe(
       'Warning: Failed Context Types: ' +
       'Required child context `foo` was not specified in `Component`.'
     );
-    expect(console.warn.argsForCall[1][0]).toBe(
-      'Warning: Failed Context Types: ' +
-      'Required child context `foo` was not specified in `Component`.'
-    );
 
     ReactTestUtils.renderIntoDocument(<Component testContext={{foo: 123}} />);
 
-    expect(console.warn.argsForCall.length).toBe(4);
-    expect(console.warn.argsForCall[3][0]).toBe(
+    expect(console.warn.argsForCall.length).toBe(2);
+    expect(console.warn.argsForCall[1][0]).toBe(
       'Warning: Failed Context Types: ' +
       'Invalid child context `foo` of type `number` ' +
       'supplied to `Component`, expected `string`.'
@@ -246,7 +242,7 @@ describe('ReactContextValidator', function() {
     );
 
     // Previous calls should not log errors
-    expect(console.warn.argsForCall.length).toBe(4);
+    expect(console.warn.argsForCall.length).toBe(2);
   });
 
 });
diff --git a/src/core/ReactCompositeComponent.js b/src/core/ReactCompositeComponent.js
@@ -218,6 +218,7 @@ var ReactCompositeComponentMixin = {
     this._pendingReplaceState = false;
     this._pendingForceUpdate = false;
 
+    var childContext;
     var renderedElement;
 
     var previouslyMounting = ReactLifeCycle.currentlyMountingInstance;
@@ -232,7 +233,8 @@ var ReactCompositeComponentMixin = {
         }
       }
 
-      renderedElement = this._renderValidatedComponent();
+      childContext = this._getValidatedChildContext(context);
+      renderedElement = this._renderValidatedComponent(childContext);
     } finally {
       ReactLifeCycle.currentlyMountingInstance = previouslyMounting;
     }
@@ -246,7 +248,7 @@ var ReactCompositeComponentMixin = {
       this._renderedComponent,
       rootID,
       transaction,
-      this._processChildContext(context)
+      this._mergeChildContext(context, childContext)
     );
     if (inst.componentDidMount) {
       transaction.getReactMountReady().enqueue(inst.componentDidMount, inst);
@@ -376,7 +378,7 @@ var ReactCompositeComponentMixin = {
    * @return {object}
    * @private
    */
-  _processChildContext: function(currentContext) {
+  _getValidatedChildContext: function(currentContext) {
     var inst = this._instance;
     var childContext = inst.getChildContext && inst.getChildContext();
     if (childContext) {
@@ -401,6 +403,13 @@ var ReactCompositeComponentMixin = {
           name
         );
       }
+      return childContext;
+    }
+    return null;
+  },
+
+  _mergeChildContext: function(currentContext, childContext) {
+    if (childContext) {
       return assign({}, currentContext, childContext);
     }
     return currentContext;
@@ -729,13 +738,14 @@ var ReactCompositeComponentMixin = {
   _updateRenderedComponent: function(transaction, context) {
     var prevComponentInstance = this._renderedComponent;
     var prevRenderedElement = prevComponentInstance._currentElement;
-    var nextRenderedElement = this._renderValidatedComponent();
+    var childContext = this._getValidatedChildContext();
+    var nextRenderedElement = this._renderValidatedComponent(childContext);
     if (shouldUpdateReactComponent(prevRenderedElement, nextRenderedElement)) {
       ReactReconciler.receiveComponent(
         prevComponentInstance,
         nextRenderedElement,
         transaction,
-        this._processChildContext(context)
+        this._mergeChildContext(context, childContext)
       );
     } else {
       // These two IDs are actually the same! But nothing should rely on that.
@@ -751,7 +761,7 @@ var ReactCompositeComponentMixin = {
         this._renderedComponent,
         thisID,
         transaction,
-        this._processChildContext(context)
+        this._mergeChildContext(context, childContext)
       );
       this._replaceNodeWithMarkupByID(prevComponentID, nextMarkup);
     }
@@ -789,11 +799,12 @@ var ReactCompositeComponentMixin = {
   /**
    * @private
    */
-  _renderValidatedComponent: function() {
+  _renderValidatedComponent: function(childContext) {
     var renderedComponent;
     var previousContext = ReactContext.current;
-    ReactContext.current = this._processChildContext(
-      this._currentElement._context
+    ReactContext.current = this._mergeChildContext(
+      this._currentElement._context,
+      childContext
     );
     ReactCurrentOwner.current = this;
     try {

diff --git a/src/core/__tests__/ReactCompositeComponent-test.js b/src/core/__tests__/ReactCompositeComponent-test.js
@@ -836,6 +836,46 @@ describe('ReactCompositeComponent', function() {
     );
   });
 
+  it('should not warn if getChildContext returns a new object', function() {
+    var getChildContextCalls = 0;
+
+    var Parent = React.createClass({
+      childContextTypes: {
+        x: React.PropTypes.object
+      },
+      getChildContext: function() {
+        getChildContextCalls++;
+        return {
+          x: {}
+        };
+      },
+      render: function() {
+        return <Child />;
+      }
+    });
+
+    var Child = React.createClass({
+      contextTypes: {
+        x: React.PropTypes.object
+      },
+      render: function() {
+        return null;
+      }
+    });
+
+    var div = document.createElement('div');
+
+    // Initial render gives no context warning
+    React.render(<Parent />, div);
+    expect(getChildContextCalls).toBe(1);
+    expect(console.warn.argsForCall.length).toBe(0);
+
+    // Rerender
+    React.render(<Parent />, div);
+    expect(getChildContextCalls).toBe(2);
+    expect(console.warn.argsForCall.length).toBe(0);
+  });
+
   it('unmasked context propagates through updates', function() {
 
     var Leaf = React.createClass({