Merge pull request #8742 from bvaughn/stack-fiber-gcc-warning

Warn about missing getChildContext method

Author: bvaughn

scripts/fiber/tests-passing.txt

@@ -157,6 +157,8 @@ src/isomorphic/classic/__tests__/ReactContextValidator-test.js
 * should pass next context to lifecycles
 * should check context types
 * should check child context types
+* should warn (but not error) if getChildContext method is missing
+* should pass parent context if getChildContext method is missing
 
 src/isomorphic/classic/class/__tests__/ReactBind-test.js
 * Holds reference to instance

src/isomorphic/classic/__tests__/ReactContextValidator-test.js

@@ -289,4 +289,91 @@ describe('ReactContextValidator', () => {
     expectDev(console.error.calls.count()).toBe(2);
   });
 
+  // TODO (bvaughn) Remove this test and the associated behavior in the future.
+  // It has only been added in Fiber to match the (unintentional) behavior in Stack.
+  it('should warn (but not error) if getChildContext method is missing', () => {
+    spyOn(console, 'error');
+
+    class ComponentA extends React.Component {
+      static childContextTypes = {
+        foo: React.PropTypes.string.isRequired,
+      };
+      render() {
+        return <div />;
+      }
+    }
+    class ComponentB extends React.Component {
+      static childContextTypes = {
+        foo: React.PropTypes.string.isRequired,
+      };
+      render() {
+        return <div />;
+      }
+    }
+
+    ReactTestUtils.renderIntoDocument(<ComponentA/>);
+    expectDev(console.error.calls.count()).toBe(1);
+    expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe(
+      'Warning: ComponentA.childContextTypes is specified but there is no ' +
+      'getChildContext() method on the instance. You can either define ' +
+      'getChildContext() on ComponentA or remove childContextTypes from it.'
+    );
+
+    // Warnings should be deduped by component type
+    ReactTestUtils.renderIntoDocument(<ComponentA/>);
+    expectDev(console.error.calls.count()).toBe(1);
+    ReactTestUtils.renderIntoDocument(<ComponentB/>);
+    expectDev(console.error.calls.count()).toBe(2);
+    expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(1)[0])).toBe(
+      'Warning: ComponentB.childContextTypes is specified but there is no ' +
+      'getChildContext() method on the instance. You can either define ' +
+      'getChildContext() on ComponentB or remove childContextTypes from it.'
+    );
+  });
+
+  // TODO (bvaughn) Remove this test and the associated behavior in the future.
+  // It has only been added in Fiber to match the (unintentional) behavior in Stack.
+  it('should pass parent context if getChildContext method is missing', () => {
+    spyOn(console, 'error');
+
+    class ParentContextProvider extends React.Component {
+      static childContextTypes = {
+        foo: React.PropTypes.number,
+      };
+      getChildContext() {
+        return {
+          foo: 'FOO',
+        };
+      }
+      render() {
+        return <MiddleMissingContext />;
+      }
+    }
+
+    class MiddleMissingContext extends React.Component {
+      static childContextTypes = {
+        bar: React.PropTypes.string.isRequired,
+      };
+      render() {
+        return <ChildContextConsumer />;
+      }
+    }
+
+    var childContext;
+    var ChildContextConsumer = React.createClass({
+      contextTypes: {
+        bar: React.PropTypes.string.isRequired,
+        foo: React.PropTypes.string.isRequired,
+      },
+      render: function() {
+        childContext = this.context;
+        return <div />;
+      },
+    });
+
+    ReactTestUtils.renderIntoDocument(<ParentContextProvider/>);
+    expect(childContext.bar).toBeUndefined();
+    expect(childContext.foo).toBe('FOO');
+  });
+
 });

src/renderers/shared/fiber/ReactFiberContext.js

@@ -17,6 +17,7 @@ import type { StackCursor } from 'ReactFiberStack';
 
 var emptyObject = require('emptyObject');
 var invariant = require('invariant');
+var warning = require('warning');
 var {
   getComponentName,
   isFiberMounted,
@@ -33,6 +34,7 @@ const {
 
 if (__DEV__) {
   var checkReactTypeSpec = require('checkReactTypeSpec');
+  var warnedAboutMissingGetChildContext = {};
 }
 
 // A cursor to the current merged context object on the stack.
@@ -141,6 +143,28 @@ exports.pushTopLevelContextObject = function(fiber : Fiber, context : Object, di
 function processChildContext(fiber : Fiber, parentContext : Object, isReconciling : boolean): Object {
   const instance = fiber.stateNode;
   const childContextTypes = fiber.type.childContextTypes;
+
+  // TODO (bvaughn) Replace this behavior with an invariant() in the future.
+  // It has only been added in Fiber to match the (unintentional) behavior in Stack.
+  if (typeof instance.getChildContext !== 'function') {
+    if (__DEV__) {
+      const componentName = getComponentName(fiber);
+      
+      if (!warnedAboutMissingGetChildContext[componentName]) {
+        warnedAboutMissingGetChildContext[componentName] = true;
+        warning(
+          false,
+          '%s.childContextTypes is specified but there is no getChildContext() method ' +
+          'on the instance. You can either define getChildContext() on %s or remove ' +
+          'childContextTypes from it.',
+          componentName,
+          componentName,
+        );
+      }
+    }
+    return parentContext;
+  }
+
   const childContext = instance.getChildContext();
   for (let contextKey in childContext) {
     invariant(

src/renderers/shared/shared/__tests__/ReactStatelessComponent-test.js

@@ -118,11 +118,17 @@ describe('ReactStatelessComponent', () => {
 
     ReactDOM.render(<StatelessComponentWithChildContext name="A" />, container);
 
-    expectDev(console.error.calls.count()).toBe(1);
+    expectDev(console.error.calls.count()).toBe(2);
     expectDev(console.error.calls.argsFor(0)[0]).toContain(
       'StatelessComponentWithChildContext(...): childContextTypes cannot ' +
       'be defined on a functional component.'
     );
+    expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(1)[0])).toBe(
+      'Warning: StatelessComponentWithChildContext.childContextTypes is specified ' +
+      'but there is no getChildContext() method on the instance. You can either ' +
+      'define getChildContext() on StatelessComponentWithChildContext or remove ' +
+      'childContextTypes from it.'
+    );
   });
 
   if (!ReactDOMFeatureFlags.useFiber) {

src/renderers/shared/stack/reconciler/ReactCompositeComponent.js

@@ -24,6 +24,7 @@ var ReactReconciler = require('ReactReconciler');
 
 if (__DEV__) {
   var checkReactTypeSpec = require('checkReactTypeSpec');
+  var warningAboutMissingGetChildContext = {};
 }
 
 var emptyObject = require('emptyObject');
@@ -698,6 +699,22 @@ var ReactCompositeComponent = {
         );
       }
       return Object.assign({}, currentContext, childContext);
+    } else {
+      if (__DEV__) {
+        const componentName = this.getName();
+        
+        if (!warningAboutMissingGetChildContext[componentName]) {
+          warningAboutMissingGetChildContext[componentName] = true;
+          warning(
+            !Component.childContextTypes,
+            '%s.childContextTypes is specified but there is no getChildContext() method ' +
+            'on the instance. You can either define getChildContext() on %s or remove ' +
+            'childContextTypes from it.',
+            componentName,
+            componentName,
+          );
+        }
+      }
     }
     return currentContext;
   },