From c87d77ef09c42f481cdab250b68f434eec9e41f0 Mon Sep 17 00:00:00 2001
From: Dan Abramov <dan.abramov@gmail.com>
Date: Mon, 18 Mar 2019 19:44:43 +0000
Subject: [PATCH 1/3] Improve warning for invalid class contextType

---
 .../__tests__/ReactServerRendering-test.js    | 89 +++++++++++++++++++
 .../src/server/ReactPartialRendererContext.js | 62 +++++++++----
 .../src/ReactFiberClassComponent.js           | 49 +++++++---
 .../__tests__/ReactContextValidator-test.js   | 89 +++++++++++++++++++
 4 files changed, 258 insertions(+), 31 deletions(-)

diff --git a/packages/react-dom/src/__tests__/ReactServerRendering-test.js b/packages/react-dom/src/__tests__/ReactServerRendering-test.js
index 738148ba3b642..bbb2b0f7ac7a5 100644
--- a/packages/react-dom/src/__tests__/ReactServerRendering-test.js
+++ b/packages/react-dom/src/__tests__/ReactServerRendering-test.js
@@ -943,4 +943,93 @@ describe('ReactDOMServer', () => {
       {withoutStack: true},
     );
   });
+
+  it('should warn when class contextType is null', () => {
+    class Foo extends React.Component {
+      static contextType = null;
+      render() {
+        return this.context.hello.world;
+      }
+    }
+
+    expect(() => {
+      expect(() => {
+        ReactDOMServer.renderToString(<Foo />);
+      }).toThrow("Cannot read property 'world' of undefined");
+    }).toWarnDev(
+      'Foo defines an invalid contextType. ' +
+        'contextType should point to the Context object returned by React.createContext(). ' +
+        'However, it is set to null.',
+      {withoutStack: true},
+    );
+  });
+
+  it('should warn when class contextType is undefined', () => {
+    class Foo extends React.Component {
+      // This commonly happens with circular deps
+      // https://github.com/facebook/react/issues/13969
+      static contextType = undefined;
+      render() {
+        return this.context.hello.world;
+      }
+    }
+
+    expect(() => {
+      expect(() => {
+        ReactDOMServer.renderToString(<Foo />);
+      }).toThrow("Cannot read property 'world' of undefined");
+    }).toWarnDev(
+      'Foo defines an invalid contextType. ' +
+        'contextType should point to the Context object returned by React.createContext(). ' +
+        'However, it is set to undefined. ' +
+        'This can be caused by a typo or by mixing up named and default imports. ' +
+        'This can also happen due to a circular dependency, ' +
+        'so try moving the createContext() call to a separate file.',
+      {withoutStack: true},
+    );
+  });
+
+  it('should warn when class contextType is an object', () => {
+    class Foo extends React.Component {
+      // Can happen due to a typo
+      static contextType = {
+        x: 42,
+        y: 'hello',
+      };
+      render() {
+        return this.context.hello.world;
+      }
+    }
+
+    expect(() => {
+      expect(() => {
+        ReactDOMServer.renderToString(<Foo />);
+      }).toThrow("Cannot read property 'hello' of undefined");
+    }).toWarnDev(
+      'Foo defines an invalid contextType. ' +
+        'contextType should point to the Context object returned by React.createContext(). ' +
+        'However, it is set to an object with keys {x, y}.',
+      {withoutStack: true},
+    );
+  });
+
+  it('should warn when class contextType is a primitive', () => {
+    class Foo extends React.Component {
+      static contextType = 'foo';
+      render() {
+        return this.context.hello.world;
+      }
+    }
+
+    expect(() => {
+      expect(() => {
+        ReactDOMServer.renderToString(<Foo />);
+      }).toThrow("Cannot read property 'world' of undefined");
+    }).toWarnDev(
+      'Foo defines an invalid contextType. ' +
+        'contextType should point to the Context object returned by React.createContext(). ' +
+        'However, it is set to string.',
+      {withoutStack: true},
+    );
+  });
 });
diff --git a/packages/react-dom/src/server/ReactPartialRendererContext.js b/packages/react-dom/src/server/ReactPartialRendererContext.js
index 93c857d283715..b4facc253f919 100644
--- a/packages/react-dom/src/server/ReactPartialRendererContext.js
+++ b/packages/react-dom/src/server/ReactPartialRendererContext.js
@@ -10,19 +10,19 @@
 import type {ThreadID} from './ReactThreadIDAllocator';
 import type {ReactContext} from 'shared/ReactTypes';
 
-import {REACT_CONTEXT_TYPE} from 'shared/ReactSymbols';
+import {REACT_CONTEXT_TYPE, REACT_PROVIDER_TYPE} from 'shared/ReactSymbols';
 import ReactSharedInternals from 'shared/ReactSharedInternals';
 import getComponentName from 'shared/getComponentName';
 import warningWithoutStack from 'shared/warningWithoutStack';
 import checkPropTypes from 'prop-types/checkPropTypes';
 
 let ReactDebugCurrentFrame;
+let didWarnAboutInvalidateContextType;
 if (__DEV__) {
   ReactDebugCurrentFrame = ReactSharedInternals.ReactDebugCurrentFrame;
+  didWarnAboutInvalidateContextType = new Set();
 }
 
-const didWarnAboutInvalidateContextType = {};
-
 export const emptyObject = {};
 if (__DEV__) {
   Object.freeze(emptyObject);
@@ -75,26 +75,50 @@ export function processContext(
   threadID: ThreadID,
 ) {
   const contextType = type.contextType;
-  if (typeof contextType === 'object' && contextType !== null) {
-    if (__DEV__) {
-      const isContextConsumer =
+  if (__DEV__) {
+    if ('contextType' in (type: any)) {
+      let isValid =
+        contextType !== null &&
+        contextType !== undefined &&
         contextType.$$typeof === REACT_CONTEXT_TYPE &&
-        contextType._context !== undefined;
-      if (contextType.$$typeof !== REACT_CONTEXT_TYPE || isContextConsumer) {
-        let name = getComponentName(type) || 'Component';
-        if (!didWarnAboutInvalidateContextType[name]) {
-          didWarnAboutInvalidateContextType[name] = true;
-          warningWithoutStack(
-            false,
-            '%s defines an invalid contextType. ' +
-              'contextType should point to the Context object returned by React.createContext(). ' +
-              'Did you accidentally pass the Context.%s instead?',
-            name,
-            isContextConsumer ? 'Consumer' : 'Provider',
-          );
+        contextType._context === undefined; // Not a <Context.Consumer>
+
+      if (!isValid && !didWarnAboutInvalidateContextType.has(type)) {
+        didWarnAboutInvalidateContextType.add(type);
+
+        let addendum = '';
+        if (contextType === null) {
+          addendum = ' However, it is set to null.';
+        } else if (contextType === undefined) {
+          addendum =
+            ' However, it is set to undefined. ' +
+            'This can be caused by a typo or by mixing up named and default imports. ' +
+            'This can also happen due to a circular dependency, so ' +
+            'try moving the createContext() call to a separate file.';
+        } else if (typeof contextType !== 'object') {
+          addendum = ' However, it is set to ' + typeof contextType + '.';
+        } else if (contextType.$$typeof === REACT_PROVIDER_TYPE) {
+          addendum = ' Did you accidentally pass the Context.Provider instead?';
+        } else if (contextType._context !== undefined) {
+          // <Context.Consumer>
+          addendum = ' Did you accidentally pass the Context.Consumer instead?';
+        } else {
+          addendum =
+            ' However, it is set to an object with keys {' +
+            Object.keys(contextType).join(', ') +
+            '}.';
         }
+        warningWithoutStack(
+          false,
+          '%s defines an invalid contextType. ' +
+            'contextType should point to the Context object returned by React.createContext().%s',
+          getComponentName(type) || 'Component',
+          addendum,
+        );
       }
     }
+  }
+  if (typeof contextType === 'object' && contextType !== null) {
     validateContextBounds(contextType, threadID);
     return contextType[threadID];
   } else {
diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.js b/packages/react-reconciler/src/ReactFiberClassComponent.js
index 40c7aca28b9a3..01b1e14917ee8 100644
--- a/packages/react-reconciler/src/ReactFiberClassComponent.js
+++ b/packages/react-reconciler/src/ReactFiberClassComponent.js
@@ -24,7 +24,7 @@ import shallowEqual from 'shared/shallowEqual';
 import getComponentName from 'shared/getComponentName';
 import invariant from 'shared/invariant';
 import warningWithoutStack from 'shared/warningWithoutStack';
-import {REACT_CONTEXT_TYPE} from 'shared/ReactSymbols';
+import {REACT_CONTEXT_TYPE, REACT_PROVIDER_TYPE} from 'shared/ReactSymbols';
 
 import {startPhaseTimer, stopPhaseTimer} from './ReactDebugFiberPerf';
 import {resolveDefaultProps} from './ReactFiberLazyComponent';
@@ -513,27 +513,52 @@ function constructClassInstance(
   let unmaskedContext = emptyContextObject;
   let context = null;
   const contextType = ctor.contextType;
-  if (typeof contextType === 'object' && contextType !== null) {
-    if (__DEV__) {
-      const isContextConsumer =
+
+  if (__DEV__) {
+    if ('contextType' in ctor) {
+      let isValid =
+        contextType !== null &&
+        contextType !== undefined &&
         contextType.$$typeof === REACT_CONTEXT_TYPE &&
-        contextType._context !== undefined;
-      if (
-        (contextType.$$typeof !== REACT_CONTEXT_TYPE || isContextConsumer) &&
-        !didWarnAboutInvalidateContextType.has(ctor)
-      ) {
+        contextType._context === undefined; // Not a <Context.Consumer>
+
+      if (!isValid && !didWarnAboutInvalidateContextType.has(ctor)) {
         didWarnAboutInvalidateContextType.add(ctor);
+
+        let addendum = '';
+        if (contextType === null) {
+          addendum = ' However, it is set to null.';
+        } else if (contextType === undefined) {
+          addendum =
+            ' However, it is set to undefined. ' +
+            'This can be caused by a typo or by mixing up named and default imports. ' +
+            'This can also happen due to a circular dependency, so ' +
+            'try moving the createContext() call to a separate file.';
+        } else if (typeof contextType !== 'object') {
+          addendum = ' However, it is set to ' + typeof contextType + '.';
+        } else if (contextType.$$typeof === REACT_PROVIDER_TYPE) {
+          addendum = ' Did you accidentally pass the Context.Provider instead?';
+        } else if (contextType._context !== undefined) {
+          // <Context.Consumer>
+          addendum = ' Did you accidentally pass the Context.Consumer instead?';
+        } else {
+          addendum =
+            ' However, it is set to an object with keys {' +
+            Object.keys(contextType).join(', ') +
+            '}.';
+        }
         warningWithoutStack(
           false,
           '%s defines an invalid contextType. ' +
-            'contextType should point to the Context object returned by React.createContext(). ' +
-            'Did you accidentally pass the Context.%s instead?',
+            'contextType should point to the Context object returned by React.createContext().%s',
           getComponentName(ctor) || 'Component',
-          isContextConsumer ? 'Consumer' : 'Provider',
+          addendum,
         );
       }
     }
+  }
 
+  if (typeof contextType === 'object' && contextType !== null) {
     context = readContext((contextType: any));
   } else {
     unmaskedContext = getUnmaskedContext(workInProgress, ctor, true);
diff --git a/packages/react/src/__tests__/ReactContextValidator-test.js b/packages/react/src/__tests__/ReactContextValidator-test.js
index b01f008bacb17..fff4f8dc33393 100644
--- a/packages/react/src/__tests__/ReactContextValidator-test.js
+++ b/packages/react/src/__tests__/ReactContextValidator-test.js
@@ -578,6 +578,95 @@ describe('ReactContextValidator', () => {
     );
   });
 
+  it('should warn when class contextType is null', () => {
+    class Foo extends React.Component {
+      static contextType = null;
+      render() {
+        return this.context.hello.world;
+      }
+    }
+
+    expect(() => {
+      expect(() => {
+        ReactTestUtils.renderIntoDocument(<Foo />);
+      }).toThrow("Cannot read property 'world' of undefined");
+    }).toWarnDev(
+      'Foo defines an invalid contextType. ' +
+        'contextType should point to the Context object returned by React.createContext(). ' +
+        'However, it is set to null.',
+      {withoutStack: true},
+    );
+  });
+
+  it('should warn when class contextType is undefined', () => {
+    class Foo extends React.Component {
+      // This commonly happens with circular deps
+      // https://github.com/facebook/react/issues/13969
+      static contextType = undefined;
+      render() {
+        return this.context.hello.world;
+      }
+    }
+
+    expect(() => {
+      expect(() => {
+        ReactTestUtils.renderIntoDocument(<Foo />);
+      }).toThrow("Cannot read property 'world' of undefined");
+    }).toWarnDev(
+      'Foo defines an invalid contextType. ' +
+        'contextType should point to the Context object returned by React.createContext(). ' +
+        'However, it is set to undefined. ' +
+        'This can be caused by a typo or by mixing up named and default imports. ' +
+        'This can also happen due to a circular dependency, ' +
+        'so try moving the createContext() call to a separate file.',
+      {withoutStack: true},
+    );
+  });
+
+  it('should warn when class contextType is an object', () => {
+    class Foo extends React.Component {
+      // Can happen due to a typo
+      static contextType = {
+        x: 42,
+        y: 'hello',
+      };
+      render() {
+        return this.context.hello.world;
+      }
+    }
+
+    expect(() => {
+      expect(() => {
+        ReactTestUtils.renderIntoDocument(<Foo />);
+      }).toThrow("Cannot read property 'hello' of undefined");
+    }).toWarnDev(
+      'Foo defines an invalid contextType. ' +
+        'contextType should point to the Context object returned by React.createContext(). ' +
+        'However, it is set to an object with keys {x, y}.',
+      {withoutStack: true},
+    );
+  });
+
+  it('should warn when class contextType is a primitive', () => {
+    class Foo extends React.Component {
+      static contextType = 'foo';
+      render() {
+        return this.context.hello.world;
+      }
+    }
+
+    expect(() => {
+      expect(() => {
+        ReactTestUtils.renderIntoDocument(<Foo />);
+      }).toThrow("Cannot read property 'world' of undefined");
+    }).toWarnDev(
+      'Foo defines an invalid contextType. ' +
+        'contextType should point to the Context object returned by React.createContext(). ' +
+        'However, it is set to string.',
+      {withoutStack: true},
+    );
+  });
+
   it('should warn if you define contextType on a function component', () => {
     const Context = React.createContext();
 

From 9808cc318832322e5d64300447e34c4a110ec0a0 Mon Sep 17 00:00:00 2001
From: Dan Abramov <dan.abramov@gmail.com>
Date: Tue, 19 Mar 2019 13:20:16 +0000
Subject: [PATCH 2/3] Don't warn for null

---
 .../src/__tests__/ReactServerRendering-test.js   | 15 ++++-----------
 .../src/server/ReactPartialRendererContext.js    | 13 ++++++-------
 .../src/ReactFiberClassComponent.js              | 13 ++++++-------
 .../src/__tests__/ReactContextValidator-test.js  | 16 ++++------------
 4 files changed, 20 insertions(+), 37 deletions(-)

diff --git a/packages/react-dom/src/__tests__/ReactServerRendering-test.js b/packages/react-dom/src/__tests__/ReactServerRendering-test.js
index bbb2b0f7ac7a5..1aaf95576518d 100644
--- a/packages/react-dom/src/__tests__/ReactServerRendering-test.js
+++ b/packages/react-dom/src/__tests__/ReactServerRendering-test.js
@@ -944,24 +944,17 @@ describe('ReactDOMServer', () => {
     );
   });
 
-  it('should warn when class contextType is null', () => {
+  it('should not warn when class contextType is null', () => {
     class Foo extends React.Component {
-      static contextType = null;
+      static contextType = null; // Handy for conditional declaration
       render() {
         return this.context.hello.world;
       }
     }
 
     expect(() => {
-      expect(() => {
-        ReactDOMServer.renderToString(<Foo />);
-      }).toThrow("Cannot read property 'world' of undefined");
-    }).toWarnDev(
-      'Foo defines an invalid contextType. ' +
-        'contextType should point to the Context object returned by React.createContext(). ' +
-        'However, it is set to null.',
-      {withoutStack: true},
-    );
+      ReactDOMServer.renderToString(<Foo />);
+    }).toThrow("Cannot read property 'world' of undefined");
   });
 
   it('should warn when class contextType is undefined', () => {
diff --git a/packages/react-dom/src/server/ReactPartialRendererContext.js b/packages/react-dom/src/server/ReactPartialRendererContext.js
index b4facc253f919..7574719ca6c8e 100644
--- a/packages/react-dom/src/server/ReactPartialRendererContext.js
+++ b/packages/react-dom/src/server/ReactPartialRendererContext.js
@@ -78,18 +78,17 @@ export function processContext(
   if (__DEV__) {
     if ('contextType' in (type: any)) {
       let isValid =
-        contextType !== null &&
-        contextType !== undefined &&
-        contextType.$$typeof === REACT_CONTEXT_TYPE &&
-        contextType._context === undefined; // Not a <Context.Consumer>
+        // Allow null for conditional declaration
+        contextType === null ||
+        (contextType !== undefined &&
+          contextType.$$typeof === REACT_CONTEXT_TYPE &&
+          contextType._context === undefined); // Not a <Context.Consumer>
 
       if (!isValid && !didWarnAboutInvalidateContextType.has(type)) {
         didWarnAboutInvalidateContextType.add(type);
 
         let addendum = '';
-        if (contextType === null) {
-          addendum = ' However, it is set to null.';
-        } else if (contextType === undefined) {
+        if (contextType === undefined) {
           addendum =
             ' However, it is set to undefined. ' +
             'This can be caused by a typo or by mixing up named and default imports. ' +
diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.js b/packages/react-reconciler/src/ReactFiberClassComponent.js
index 01b1e14917ee8..8447c6e4a32be 100644
--- a/packages/react-reconciler/src/ReactFiberClassComponent.js
+++ b/packages/react-reconciler/src/ReactFiberClassComponent.js
@@ -517,18 +517,17 @@ function constructClassInstance(
   if (__DEV__) {
     if ('contextType' in ctor) {
       let isValid =
-        contextType !== null &&
-        contextType !== undefined &&
-        contextType.$$typeof === REACT_CONTEXT_TYPE &&
-        contextType._context === undefined; // Not a <Context.Consumer>
+        // Allow null for conditional declaration
+        contextType === null ||
+        (contextType !== undefined &&
+          contextType.$$typeof === REACT_CONTEXT_TYPE &&
+          contextType._context === undefined); // Not a <Context.Consumer>
 
       if (!isValid && !didWarnAboutInvalidateContextType.has(ctor)) {
         didWarnAboutInvalidateContextType.add(ctor);
 
         let addendum = '';
-        if (contextType === null) {
-          addendum = ' However, it is set to null.';
-        } else if (contextType === undefined) {
+        if (contextType === undefined) {
           addendum =
             ' However, it is set to undefined. ' +
             'This can be caused by a typo or by mixing up named and default imports. ' +
diff --git a/packages/react/src/__tests__/ReactContextValidator-test.js b/packages/react/src/__tests__/ReactContextValidator-test.js
index fff4f8dc33393..f5566240b8103 100644
--- a/packages/react/src/__tests__/ReactContextValidator-test.js
+++ b/packages/react/src/__tests__/ReactContextValidator-test.js
@@ -578,24 +578,16 @@ describe('ReactContextValidator', () => {
     );
   });
 
-  it('should warn when class contextType is null', () => {
+  it('should not warn when class contextType is null', () => {
     class Foo extends React.Component {
-      static contextType = null;
+      static contextType = null; // Handy for conditional declaration
       render() {
         return this.context.hello.world;
       }
     }
-
     expect(() => {
-      expect(() => {
-        ReactTestUtils.renderIntoDocument(<Foo />);
-      }).toThrow("Cannot read property 'world' of undefined");
-    }).toWarnDev(
-      'Foo defines an invalid contextType. ' +
-        'contextType should point to the Context object returned by React.createContext(). ' +
-        'However, it is set to null.',
-      {withoutStack: true},
-    );
+      ReactTestUtils.renderIntoDocument(<Foo />);
+    }).toThrow("Cannot read property 'world' of undefined");
   });
 
   it('should warn when class contextType is undefined', () => {

From a123591ba0e3c54125e181ff250072129f46adfc Mon Sep 17 00:00:00 2001
From: Dan Abramov <dan.abramov@gmail.com>
Date: Tue, 19 Mar 2019 13:24:20 +0000
Subject: [PATCH 3/3] Grammar

---
 packages/react-dom/src/__tests__/ReactServerRendering-test.js | 2 +-
 packages/react-dom/src/server/ReactPartialRendererContext.js  | 2 +-
 packages/react-reconciler/src/ReactFiberClassComponent.js     | 2 +-
 packages/react/src/__tests__/ReactContextValidator-test.js    | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/packages/react-dom/src/__tests__/ReactServerRendering-test.js b/packages/react-dom/src/__tests__/ReactServerRendering-test.js
index 1aaf95576518d..8c68b07a5fe88 100644
--- a/packages/react-dom/src/__tests__/ReactServerRendering-test.js
+++ b/packages/react-dom/src/__tests__/ReactServerRendering-test.js
@@ -1021,7 +1021,7 @@ describe('ReactDOMServer', () => {
     }).toWarnDev(
       'Foo defines an invalid contextType. ' +
         'contextType should point to the Context object returned by React.createContext(). ' +
-        'However, it is set to string.',
+        'However, it is set to a string.',
       {withoutStack: true},
     );
   });
diff --git a/packages/react-dom/src/server/ReactPartialRendererContext.js b/packages/react-dom/src/server/ReactPartialRendererContext.js
index 7574719ca6c8e..2ad914eb2b1ad 100644
--- a/packages/react-dom/src/server/ReactPartialRendererContext.js
+++ b/packages/react-dom/src/server/ReactPartialRendererContext.js
@@ -95,7 +95,7 @@ export function processContext(
             'This can also happen due to a circular dependency, so ' +
             'try moving the createContext() call to a separate file.';
         } else if (typeof contextType !== 'object') {
-          addendum = ' However, it is set to ' + typeof contextType + '.';
+          addendum = ' However, it is set to a ' + typeof contextType + '.';
         } else if (contextType.$$typeof === REACT_PROVIDER_TYPE) {
           addendum = ' Did you accidentally pass the Context.Provider instead?';
         } else if (contextType._context !== undefined) {
diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.js b/packages/react-reconciler/src/ReactFiberClassComponent.js
index 8447c6e4a32be..17fd298ef7fce 100644
--- a/packages/react-reconciler/src/ReactFiberClassComponent.js
+++ b/packages/react-reconciler/src/ReactFiberClassComponent.js
@@ -534,7 +534,7 @@ function constructClassInstance(
             'This can also happen due to a circular dependency, so ' +
             'try moving the createContext() call to a separate file.';
         } else if (typeof contextType !== 'object') {
-          addendum = ' However, it is set to ' + typeof contextType + '.';
+          addendum = ' However, it is set to a ' + typeof contextType + '.';
         } else if (contextType.$$typeof === REACT_PROVIDER_TYPE) {
           addendum = ' Did you accidentally pass the Context.Provider instead?';
         } else if (contextType._context !== undefined) {
diff --git a/packages/react/src/__tests__/ReactContextValidator-test.js b/packages/react/src/__tests__/ReactContextValidator-test.js
index f5566240b8103..bc935753b6e55 100644
--- a/packages/react/src/__tests__/ReactContextValidator-test.js
+++ b/packages/react/src/__tests__/ReactContextValidator-test.js
@@ -654,7 +654,7 @@ describe('ReactContextValidator', () => {
     }).toWarnDev(
       'Foo defines an invalid contextType. ' +
         'contextType should point to the Context object returned by React.createContext(). ' +
-        'However, it is set to string.',
+        'However, it is set to a string.',
       {withoutStack: true},
     );
   });