Make this.context {} when disabled, but function context is undefined

facebook · Aug 1, 2019 · 133a4b5 · 133a4b5
1 parent f142e3e
commit 133a4b5
Show file tree

Hide file tree

Showing 6 changed files with 125 additions and 85 deletions.
diff --git a/...s/react-dom/src/__tests__/ReactDOMServerIntegrationLegacyContextDisabled-test.internal.js b/...s/react-dom/src/__tests__/ReactDOMServerIntegrationLegacyContextDisabled-test.internal.js
@@ -38,6 +38,19 @@ function initModules() {
 
 const {resetModules, itRenders} = ReactDOMServerIntegrationUtils(initModules);
 
+function formatValue(val) {
+  if (val === null) {
+    return 'null';
+  }
+  if (val === undefined) {
+    return 'undefined';
+  }
+  if (typeof val === 'string') {
+    return val;
+  }
+  return JSON.stringify(val);
+}
+
 describe('ReactDOMServerIntegrationLegacyContextDisabled', () => {
   beforeEach(() => {
     resetModules();
@@ -72,17 +85,17 @@ describe('ReactDOMServerIntegrationLegacyContextDisabled', () => {
         lifecycleContextLog.push(nextContext);
       }
       render() {
-        return typeof this.context;
+        return formatValue(this.context);
       }
     }
 
     function LegacyFnConsumer(props, context) {
-      return typeof context;
+      return formatValue(context);
     }
     LegacyFnConsumer.contextTypes = {foo() {}};
 
     function RegularFn(props, context) {
-      return typeof context;
+      return formatValue(context);
     }
 
     const e = await render(
@@ -95,7 +108,7 @@ describe('ReactDOMServerIntegrationLegacyContextDisabled', () => {
       </LegacyProvider>,
       3,
     );
-    expect(e.textContent).toBe('undefinedundefinedundefined');
+    expect(e.textContent).toBe('{}undefinedundefined');
     expect(lifecycleContextLog).toEqual([]);
   });
 
@@ -114,7 +127,7 @@ describe('ReactDOMServerIntegrationLegacyContextDisabled', () => {
 
     class RenderPropConsumer extends React.Component {
       render() {
-        return <Ctx.Consumer>{value => value}</Ctx.Consumer>;
+        return <Ctx.Consumer>{value => formatValue(value)}</Ctx.Consumer>;
       }
     }
 
@@ -132,12 +145,12 @@ describe('ReactDOMServerIntegrationLegacyContextDisabled', () => {
         lifecycleContextLog.push(nextContext);
       }
       render() {
-        return this.context;
+        return formatValue(this.context);
       }
     }
 
     function FnConsumer() {
-      return React.useContext(Ctx);
+      return formatValue(React.useContext(Ctx));
     }
 
     const e = await render(

diff --git a/packages/react-dom/src/__tests__/ReactLegacyContextDisabled-test.internal.js b/packages/react-dom/src/__tests__/ReactLegacyContextDisabled-test.internal.js
@@ -23,6 +23,19 @@ describe('ReactLegacyContextDisabled', () => {
     ReactFeatureFlags.disableLegacyContext = true;
   });
 
+  function formatValue(val) {
+    if (val === null) {
+      return 'null';
+    }
+    if (val === undefined) {
+      return 'undefined';
+    }
+    if (typeof val === 'string') {
+      return val;
+    }
+    return JSON.stringify(val);
+  }
+
   it('warns for legacy context', () => {
     class LegacyProvider extends React.Component {
       static childContextTypes = {
@@ -52,17 +65,17 @@ describe('ReactLegacyContextDisabled', () => {
         lifecycleContextLog.push(nextContext);
       }
       render() {
-        return typeof this.context;
+        return formatValue(this.context);
       }
     }
 
     function LegacyFnConsumer(props, context) {
-      return typeof context;
+      return formatValue(context);
     }
     LegacyFnConsumer.contextTypes = {foo() {}};
 
     function RegularFn(props, context) {
-      return typeof context;
+      return formatValue(context);
     }
 
     const container = document.createElement('div');
@@ -88,7 +101,7 @@ describe('ReactLegacyContextDisabled', () => {
       ],
       {withoutStack: true},
     );
-    expect(container.textContent).toBe('undefinedundefinedundefined');
+    expect(container.textContent).toBe('{}undefinedundefined');
     expect(lifecycleContextLog).toEqual([]);
 
     // Test update path.
@@ -102,8 +115,8 @@ describe('ReactLegacyContextDisabled', () => {
       </LegacyProvider>,
       container,
     );
-    expect(container.textContent).toBe('undefinedundefinedundefined');
-    expect(lifecycleContextLog).toEqual([undefined, undefined, undefined]);
+    expect(container.textContent).toBe('{}undefinedundefined');
+    expect(lifecycleContextLog).toEqual([{}, {}, {}]);
     ReactDOM.unmountComponentAtNode(container);
   });
 
@@ -122,7 +135,7 @@ describe('ReactLegacyContextDisabled', () => {
 
     class RenderPropConsumer extends React.Component {
       render() {
-        return <Ctx.Consumer>{value => value}</Ctx.Consumer>;
+        return <Ctx.Consumer>{value => formatValue(value)}</Ctx.Consumer>;
       }
     }
 
@@ -140,12 +153,12 @@ describe('ReactLegacyContextDisabled', () => {
         lifecycleContextLog.push(nextContext);
       }
       render() {
-        return this.context;
+        return formatValue(this.context);
       }
     }
 
     function FnConsumer() {
-      return React.useContext(Ctx);
+      return formatValue(React.useContext(Ctx));
     }
 
     const container = document.createElement('div');

diff --git a/packages/react-dom/src/server/ReactPartialRenderer.js b/packages/react-dom/src/server/ReactPartialRenderer.js
@@ -431,7 +431,8 @@ function resolve(
 
   // Extra closure so queue and replace can be captured properly
   function processChild(element, Component) {
-    let publicContext = processContext(Component, context, threadID);
+    const isClass = shouldConstruct(Component);
+    const publicContext = processContext(Component, context, threadID, isClass);
 
     let queue = [];
     let replace = false;
@@ -459,7 +460,7 @@ function resolve(
     };
 
     let inst;
-    if (shouldConstruct(Component)) {
+    if (isClass) {
       inst = new Component(element.props, publicContext, updater);
 
       if (typeof Component.getDerivedStateFromProps === 'function') {

diff --git a/packages/react-dom/src/server/ReactPartialRendererContext.js b/packages/react-dom/src/server/ReactPartialRendererContext.js
@@ -74,72 +74,89 @@ export function processContext(
   type: Function,
   context: Object,
   threadID: ThreadID,
+  isClass: boolean,
 ) {
-  const contextType = type.contextType;
-  if (__DEV__) {
-    if ('contextType' in (type: any)) {
-      let isValid =
-        // Allow null for conditional declaration
-        contextType === null ||
-        (contextType !== undefined &&
-          contextType.$$typeof === REACT_CONTEXT_TYPE &&
-          contextType._context === undefined); // Not a <Context.Consumer>
+  if (isClass) {
+    const contextType = type.contextType;
+    if (__DEV__) {
+      if ('contextType' in (type: any)) {
+        let isValid =
+          // 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);
+        if (!isValid && !didWarnAboutInvalidateContextType.has(type)) {
+          didWarnAboutInvalidateContextType.add(type);
 
-        let addendum = '';
-        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 a ' + 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(', ') +
-            '}.';
+          let addendum = '';
+          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 a ' + 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,
+          );
         }
-        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];
+    if (typeof contextType === 'object' && contextType !== null) {
+      validateContextBounds(contextType, threadID);
+      return contextType[threadID];
+    }
+    if (disableLegacyContext) {
+      if (__DEV__) {
+        if (type.contextTypes) {
+          warningWithoutStack(
+            false,
+            '%s uses the legacy contextTypes API which is no longer supported. ' +
+              'Use React.createContext() with static contextType instead.',
+            getComponentName(type) || 'Unknown',
+          );
+        }
+      }
+      return emptyObject;
+    } else {
+      const maskedContext = maskContext(type, context);
+      if (__DEV__) {
+        if (type.contextTypes) {
+          checkContextTypes(type.contextTypes, maskedContext, 'context');
+        }
+      }
+      return maskedContext;
+    }
   } else {
     if (disableLegacyContext) {
       if (__DEV__) {
         if (type.contextTypes) {
-          if (type.prototype && type.prototype.isReactComponent) {
-            warningWithoutStack(
-              false,
-              '%s uses the legacy contextTypes API which is no longer supported. ' +
-                'Use React.createContext() with static contextType instead.',
-              getComponentName(type) || 'Unknown',
-            );
-          } else {
-            warningWithoutStack(
-              false,
-              '%s uses the legacy contextTypes API which is no longer supported. ' +
-                'Use React.createContext() with React.useContext() instead.',
-              getComponentName(type) || 'Unknown',
-            );
-          }
+          warningWithoutStack(
+            false,
+            '%s uses the legacy contextTypes API which is no longer supported. ' +
+              'Use React.createContext() with React.useContext() instead.',
+            getComponentName(type) || 'Unknown',
+          );
         }
       }
       return undefined;

diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.js b/packages/react-reconciler/src/ReactFiberClassComponent.js
@@ -555,7 +555,7 @@ function constructClassInstance(
 ): any {
   let isLegacyContextConsumer = false;
   let unmaskedContext = emptyContextObject;
-  let context;
+  let context = emptyContextObject;
   const contextType = ctor.contextType;
 
   if (__DEV__) {
@@ -719,11 +719,6 @@ function constructClassInstance(
   // Cache unmasked context so we can avoid recreating masked context unless necessary.
   // ReactFiberContext usually updates this cache but can't for newly-created instances.
   if (isLegacyContextConsumer) {
-    invariant(
-      context !== undefined,
-      'Expected legacy context to always be present. ' +
-        'This is likely a bug in React. Please file an issue.',
-    );
     cacheContext(workInProgress, unmaskedContext, context);
   }
 
@@ -811,7 +806,9 @@ function mountClassInstance(
   const contextType = ctor.contextType;
   if (typeof contextType === 'object' && contextType !== null) {
     instance.context = readContext(contextType);
-  } else if (!disableLegacyContext) {
+  } else if (disableLegacyContext) {
+    instance.context = emptyContextObject;
+  } else {
     const unmaskedContext = getUnmaskedContext(workInProgress, ctor, true);
     instance.context = getMaskedContext(workInProgress, unmaskedContext);
   }
@@ -911,7 +908,7 @@ function resumeMountClassInstance(
 
   const oldContext = instance.context;
   const contextType = ctor.contextType;
-  let nextContext;
+  let nextContext = emptyContextObject;
   if (typeof contextType === 'object' && contextType !== null) {
     nextContext = readContext(contextType);
   } else if (!disableLegacyContext) {
@@ -1060,7 +1057,7 @@ function updateClassInstance(
 
   const oldContext = instance.context;
   const contextType = ctor.contextType;
-  let nextContext;
+  let nextContext = emptyContextObject;
   if (typeof contextType === 'object' && contextType !== null) {
     nextContext = readContext(contextType);
   } else if (!disableLegacyContext) {

diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json
@@ -338,6 +338,5 @@
   "337": "An invalid event responder was provided to host component",
   "338": "ReactDOMServer does not yet support the fundamental API.",
   "339": "An invalid value was used as an event responder. Expect one or many event responders created via React.unstable_createResponer().",
-  "340": "An invalid value was used as an event listener. Expect one or many event listeners created via React.unstable_useResponer().",
-  "341": "Expected legacy context to always be present. This is likely a bug in React. Please file an issue."
+  "340": "An invalid value was used as an event listener. Expect one or many event listeners created via React.unstable_useResponer()."
 }