Validate propTypes for memo in the reconciler when necessary

packages/react-reconciler/src/ReactFiberBeginWork.js

@@ -54,6 +54,7 @@ import invariant from 'shared/invariant';
 import shallowEqual from 'shared/shallowEqual';
 import getComponentName from 'shared/getComponentName';
 import ReactStrictModeWarnings from './ReactStrictModeWarnings';
+import {refineResolvedLazyComponent} from 'shared/ReactLazyComponent';
 import {REACT_LAZY_TYPE} from 'shared/ReactSymbols';
 import warning from 'shared/warning';
 import warningWithoutStack from 'shared/warningWithoutStack';
@@ -213,6 +214,23 @@ function updateForwardRef(
   nextProps: any,
   renderExpirationTime: ExpirationTime,
 ) {
+  if (__DEV__) {
+    if (workInProgress.type !== workInProgress.elementType) {
+      // Lazy component props can't be validated in createElement
+      // because they're only guaranteed to be resolved here.
+      const innerPropTypes = Component.propTypes;
+      if (innerPropTypes) {
+        checkPropTypes(
+          innerPropTypes,
+          nextProps, // Resolved props
+          'prop',
+          getComponentName(Component),
+          getCurrentFiberStackInDev,
+        );
+      }
+    }
+  }
+
   const render = Component.render;
   const ref = workInProgress.ref;
 
@@ -274,6 +292,19 @@ function updateMemoComponent(
         renderExpirationTime,
       );
     }
+    if (__DEV__) {
+      const type = Component.type;
+      const innerPropTypes = type.propTypes;
+      if (innerPropTypes) {
+        checkPropTypes(
+          innerPropTypes,
+          nextProps, // Resolved props
+          'prop',
+          getComponentName(type),
+          getCurrentFiberStackInDev,
+        );
+      }
+    }
     let child = createFiberFromTypeAndProps(
       Component.type,
       null,
@@ -287,6 +318,19 @@ function updateMemoComponent(
     workInProgress.child = child;
     return child;
   }
+  if (__DEV__) {
+    const type = Component.type;
+    const innerPropTypes = type.propTypes;
+    if (innerPropTypes) {
+      checkPropTypes(
+        innerPropTypes,
+        nextProps, // Resolved props
+        'prop',
+        getComponentName(type),
+        getCurrentFiberStackInDev,
+      );
+    }
+  }
   let currentChild = ((current.child: any): Fiber); // This is always exactly one child
   if (updateExpirationTime < renderExpirationTime) {
     // This will be the props with resolved defaultProps,
@@ -324,6 +368,24 @@ function updateSimpleMemoComponent(
   updateExpirationTime,
   renderExpirationTime: ExpirationTime,
 ): null | Fiber {
+  if (__DEV__) {
+    if (workInProgress.type !== workInProgress.elementType) {
+      let Component = workInProgress.elementType;
+      if (Component.$$typeof === REACT_LAZY_TYPE) {
+        Component = refineResolvedLazyComponent(Component);
+      }
+      const outerPropTypes = Component && Component.propTypes;
+      if (outerPropTypes) {
+        checkPropTypes(
+          outerPropTypes,
+          nextProps, // Resolved (we know there's no defaultProps)
+          'prop',
+          getComponentName(Component),
+          getCurrentFiberStackInDev,
+        );
+      }
+    }
+  }
   if (current !== null && updateExpirationTime < renderExpirationTime) {
     const prevProps = current.memoizedProps;
     if (
@@ -337,6 +399,18 @@ function updateSimpleMemoComponent(
       );
     }
   }
+  if (__DEV__) {
+    const innerPropTypes = Component.propTypes;
+    if (innerPropTypes) {
+      checkPropTypes(
+        innerPropTypes,
+        nextProps, // Resolved props
+        'prop',
+        getComponentName(Component),
+        getCurrentFiberStackInDev,
+      );
+    }
+  }
   return updateFunctionComponent(
     current,
     workInProgress,
@@ -413,6 +487,23 @@ function updateFunctionComponent(
   nextProps: any,
   renderExpirationTime,
 ) {
+  if (__DEV__) {
+    if (workInProgress.type !== workInProgress.elementType) {
+      // Lazy component props can't be validated in createElement
+      // because they're only guaranteed to be resolved here.
+      const innerPropTypes = Component.propTypes;
+      if (innerPropTypes) {
+        checkPropTypes(
+          innerPropTypes,
+          nextProps, // Resolved props
+          'prop',
+          getComponentName(Component),
+          getCurrentFiberStackInDev,
+        );
+      }
+    }
+  }
+
   const unmaskedContext = getUnmaskedContext(workInProgress, Component, true);
   const context = getMaskedContext(workInProgress, unmaskedContext);
 
@@ -447,6 +538,23 @@ function updateClassComponent(
   nextProps,
   renderExpirationTime: ExpirationTime,
 ) {
+  if (__DEV__) {
+    if (workInProgress.type !== workInProgress.elementType) {
+      // Lazy component props can't be validated in createElement
+      // because they're only guaranteed to be resolved here.
+      const innerPropTypes = Component.propTypes;
+      if (innerPropTypes) {
+        checkPropTypes(
+          innerPropTypes,
+          nextProps, // Resolved props
+          'prop',
+          getComponentName(Component),
+          getCurrentFiberStackInDev,
+        );
+      }
+    }
+  }
+
   // Push context providers early to prevent context stack mismatches.
   // During mounting we don't know the child context yet as the instance doesn't exist.
   // We will invalidate the child context in finishClassComponent() right after rendering.
@@ -828,11 +936,26 @@ function mountLazyComponent(
       break;
     }
     case MemoComponent: {
+      if (__DEV__) {
+        if (workInProgress.type !== workInProgress.elementType) {
+          const outerPropTypes = Component.propTypes;
+          if (outerPropTypes) {
+            checkPropTypes(
+              outerPropTypes,
+              resolvedProps, // Resolved for outer only
+              'prop',
+              getComponentName(Component),
+              getCurrentFiberStackInDev,
+            );
+          }
+        }
+      }
       child = updateMemoComponent(
         null,
         workInProgress,
         Component,
-        resolveDefaultProps(Component.type, resolvedProps), // The inner type can have defaults too
+        // The inner type can have defaults too
+        resolveDefaultProps(Component.type, resolvedProps),
         updateExpirationTime,
         renderExpirationTime,
       );
@@ -1758,6 +1881,20 @@ function beginWork(
       const unresolvedProps = workInProgress.pendingProps;
       // Resolve outer props first, then resolve inner props.
       let resolvedProps = resolveDefaultProps(type, unresolvedProps);
+      if (__DEV__) {
+        if (workInProgress.type !== workInProgress.elementType) {
+          const outerPropTypes = type.propTypes;
+          if (outerPropTypes) {
+            checkPropTypes(
+              outerPropTypes,
+              resolvedProps, // Resolved for outer only
+              'prop',
+              getComponentName(type),
+              getCurrentFiberStackInDev,
+            );
+          }
+        }
+      }
       resolvedProps = resolveDefaultProps(type.type, resolvedProps);
       return updateMemoComponent(
         current,
@@ -1769,11 +1906,13 @@ function beginWork(
       );
     }
     case SimpleMemoComponent: {
+      const type = workInProgress.type;
+      const pendingProps = workInProgress.pendingProps;
       return updateSimpleMemoComponent(
         current,
         workInProgress,
-        workInProgress.type,
-        workInProgress.pendingProps,
+        type,
+        pendingProps,
         updateExpirationTime,
         renderExpirationTime,
       );

packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js

@@ -586,19 +586,10 @@ describe('ReactLazy', () => {
     );
   });
 
-  it('only respects propTypes on the inner component', async () => {
-    function Add(props) {
-      ReactTestRenderer.unstable_yield(props.inner + props.outer);
-      return props.inner + props.outer;
-    }
-    Add.propTypes = {
-      inner: PropTypes.number.isRequired,
-    };
+  async function verifyInnerPropTypesAreChecked(Add) {
     const LazyAdd = lazy(() => fakeImport(Add));
     expect(() => {
-      LazyAdd.propTypes = {
-        outer: PropTypes.number.isRequired,
-      };
+      LazyAdd.propTypes = {};
     }).toWarnDev(
       'React.lazy(...): It is not supported to assign `propTypes` to ' +
         'a lazy component import. Either specify them where the component ' +
@@ -618,54 +609,162 @@ describe('ReactLazy', () => {
     expect(root).toFlushAndYield(['Loading...']);
     expect(root).toMatchRenderedOutput(null);
 
+    // Mount
     await Promise.resolve();
     expect(() => {
-      root.update(
-        <Suspense fallback={<Text text="Loading..." />}>
-          <LazyAdd inner="2" outer="2" />
-        </Suspense>,
-      );
+      root.unstable_flushAll();
     }).toWarnDev(
-      'Failed prop type: Invalid prop `inner` of type `string` supplied to `Add`, expected `number`.',
+      'Invalid prop `inner` of type `string` supplied to `Add`, expected `number`.',
     );
-    expect(root).toFlushAndYield(['22']);
     expect(root).toMatchRenderedOutput('22');
+
+    // Update
+    root.update(
+      <Suspense fallback={<Text text="Loading..." />}>
+        <LazyAdd inner={false} outer={false} />
+      </Suspense>,
+    );
+    expect(() => {
+      root.unstable_flushAll();
+    }).toWarnDev(
+      'Invalid prop `inner` of type `boolean` supplied to `Add`, expected `number`.',
+    );
+    expect(root).toMatchRenderedOutput('0');
+  }
+
+  it('respects propTypes on function component with defaultProps', async () => {
+    function Add(props) {
+      expect(props.innerWithDefault).toBe(42);
+      return props.inner + props.outer;
+    }
+    Add.propTypes = {
+      inner: PropTypes.number.isRequired,
+      innerWithDefault: PropTypes.number.isRequired,
+    };
+    Add.defaultProps = {
+      innerWithDefault: 42,
+    };
+    await verifyInnerPropTypesAreChecked(Add);
   });
 
-  it('respects propTypes on lazy forwardRef component', async () => {
-    const T = React.forwardRef(function T(props, ref) {
-      ReactTestRenderer.unstable_yield(props.text);
-      return props.text;
-    });
-    T.propTypes = {
-      text: PropTypes.string.isRequired,
+  it('respects propTypes on function component without defaultProps', async () => {
+    function Add(props) {
+      return props.inner + props.outer;
+    }
+    Add.propTypes = {
+      inner: PropTypes.number.isRequired,
     };
-    const LazyText = lazy(() => fakeImport(T));
+    await verifyInnerPropTypesAreChecked(Add);
+  });
 
-    const root = ReactTestRenderer.create(
-      <Suspense fallback={<Text text="Loading..." />}>
-        <LazyText text={42} />
-      </Suspense>,
-      {
-        unstable_isConcurrent: true,
-      },
-    );
-    expect(root).toFlushAndYield(['Loading...']);
-    expect(root).toMatchRenderedOutput(null);
+  it('respects propTypes on class component with defaultProps', async () => {
+    class Add extends React.Component {
+      render() {
+        expect(this.props.innerWithDefault).toBe(42);
+        return this.props.inner + this.props.outer;
+      }
+    }
+    Add.propTypes = {
+      inner: PropTypes.number.isRequired,
+      innerWithDefault: PropTypes.number.isRequired,
+    };
+    Add.defaultProps = {
+      innerWithDefault: 42,
+    };
+    await verifyInnerPropTypesAreChecked(Add);
+  });
 
-    await Promise.resolve();
-    expect(() => {
-      root.update(
-        <Suspense fallback={<Text text="Loading..." />}>
-          <LazyText text={42} />
-        </Suspense>,
-      );
-    }).toWarnDev(
-      'Failed prop type: Invalid prop `text` of type `number` ' +
-        'supplied to `ForwardRef(T)`, expected `string`.',
-    );
-    expect(root).toFlushAndYield([42]);
-    expect(root).toMatchRenderedOutput('42');
+  it('respects propTypes on class component without defaultProps', async () => {
+    class Add extends React.Component {
+      render() {
+        return this.props.inner + this.props.outer;
+      }
+    }
+    Add.propTypes = {
+      inner: PropTypes.number.isRequired,
+    };
+    await verifyInnerPropTypesAreChecked(Add);
+  });
+
+  it('respects propTypes on forwardRef component with defaultProps', async () => {
+    const Add = React.forwardRef((props, ref) => {
+      expect(props.innerWithDefault).toBe(42);
+      return props.inner + props.outer;
+    });
+    Add.displayName = 'Add';
+    Add.propTypes = {
+      inner: PropTypes.number.isRequired,
+      innerWithDefault: PropTypes.number.isRequired,
+    };
+    Add.defaultProps = {
+      innerWithDefault: 42,
+    };
+    await verifyInnerPropTypesAreChecked(Add);
+  });
+
+  it('respects propTypes on forwardRef component without defaultProps', async () => {
+    const Add = React.forwardRef((props, ref) => {
+      return props.inner + props.outer;
+    });
+    Add.displayName = 'Add';
+    Add.propTypes = {
+      inner: PropTypes.number.isRequired,
+    };
+    await verifyInnerPropTypesAreChecked(Add);
+  });
+
+  it('respects propTypes on outer memo component with defaultProps', async () => {
+    const Add = React.memo(props => {
+      expect(props.innerWithDefault).toBe(42);
+      return props.inner + props.outer;
+    });
+    Add.displayName = 'Add';
+    Add.propTypes = {
+      inner: PropTypes.number.isRequired,
+      innerWithDefault: PropTypes.number.isRequired,
+    };
+    Add.defaultProps = {
+      innerWithDefault: 42,
+    };
+    await verifyInnerPropTypesAreChecked(Add);
+  });
+
+  it('respects propTypes on outer memo component without defaultProps', async () => {
+    const Add = React.memo(props => {
+      return props.inner + props.outer;
+    });
+    Add.displayName = 'Add';
+    Add.propTypes = {
+      inner: PropTypes.number.isRequired,
+    };
+    await verifyInnerPropTypesAreChecked(Add);
+  });
+
+  it('respects propTypes on inner memo component with defaultProps', async () => {
+    const Add = props => {
+      expect(props.innerWithDefault).toBe(42);
+      return props.inner + props.outer;
+    };
+    Add.displayName = 'Add';
+    Add.propTypes = {
+      inner: PropTypes.number.isRequired,
+      innerWithDefault: PropTypes.number.isRequired,
+    };
+    Add.defaultProps = {
+      innerWithDefault: 42,
+    };
+    await verifyInnerPropTypesAreChecked(React.memo(Add));
+  });
+
+  it('respects propTypes on inner memo component without defaultProps', async () => {
+    const Add = props => {
+      return props.inner + props.outer;
+    };
+    Add.displayName = 'Add';
+    Add.propTypes = {
+      inner: PropTypes.number.isRequired,
+    };
+    await verifyInnerPropTypesAreChecked(React.memo(Add));
   });
 
   it('includes lazy-loaded component in warning stack', async () => {

packages/react-reconciler/src/__tests__/ReactMemo-test.internal.js

@@ -326,54 +326,102 @@ describe('memo', () => {
 
       it('validates propTypes declared on the inner component', () => {
         function FnInner(props) {
-          return props.inner + props.outer;
+          return props.inner;
         }
         FnInner.propTypes = {inner: PropTypes.number.isRequired};
         const Fn = React.memo(FnInner);
+
+        // Mount
+        ReactNoop.render(<Fn inner="2" />);
         expect(() => {
-          ReactNoop.render(<Fn inner="2" outer="3" />);
+          ReactNoop.flush();
         }).toWarnDev(
-          'Invalid prop `inner` of type `string` supplied to `FnInner`, expected `number`.\n' +
-            '    in FnInner (at **)',
+          'Invalid prop `inner` of type `string` supplied to `FnInner`, expected `number`.',
+        );
+
+        // Update
+        ReactNoop.render(<Fn inner={false} />);
+        expect(() => {
+          ReactNoop.flush();
+        }).toWarnDev(
+          'Invalid prop `inner` of type `boolean` supplied to `FnInner`, expected `number`.',
         );
       });
 
       it('validates propTypes declared on the outer component', () => {
         function FnInner(props) {
-          return props.inner + props.outer;
+          return props.outer;
         }
         const Fn = React.memo(FnInner);
         Fn.propTypes = {outer: PropTypes.number.isRequired};
+
+        // Mount
         expect(() => {
-          ReactNoop.render(<Fn inner="2" outer="3" />);
+          ReactNoop.render(<Fn outer="3" />);
         }).toWarnDev(
-          'Invalid prop `outer` of type `string` supplied to `FnInner`, expected `number`.\n' +
-            '    in FnInner (at **)',
+          // Outer props are checked in createElement
+          'Invalid prop `outer` of type `string` supplied to `FnInner`, expected `number`.',
         );
+        ReactNoop.flush();
+
+        // Update
+        expect(() => {
+          ReactNoop.render(<Fn outer={false} />);
+        }).toWarnDev(
+          // Outer props are checked in createElement
+          'Invalid prop `outer` of type `boolean` supplied to `FnInner`, expected `number`.',
+        );
+        ReactNoop.flush();
       });
 
-      it('warns about propTypes declared on both the outer and the inner component', () => {
-        function FnInner(props) {
-          return props.inner + props.outer;
+      it('validates nested propTypes declarations', () => {
+        function Inner(props) {
+          return props.inner + props.middle + props.outer;
         }
-        FnInner.propTypes = {inner: PropTypes.number.isRequired};
-        const Fn = React.memo(FnInner);
-        Fn.propTypes = {outer: PropTypes.number.isRequired};
-        expect(() => {
-          ReactNoop.render(<Fn inner="2" outer="3" />);
-        }).toWarnDev(
-          [
-            'React.memo(FnInner): `propTypes` are defined both on the `React.memo()` result ' +
-              'and on the inner `FnInner` component. Remove either one of these `propTypes` definitions.',
-            'Invalid prop `outer` of type `string` supplied to `FnInner`, expected `number`.\n' +
-              '    in FnInner (at **)',
-            // Outer propTypes shadow the inner ones and aren't validated in this case.
-          ],
-          {withoutStack: 1},
-        );
+        Inner.propTypes = {inner: PropTypes.number.isRequired};
+        Inner.defaultProps = {inner: 0};
+        const Middle = React.memo(Inner);
+        Middle.propTypes = {middle: PropTypes.number.isRequired};
+        Middle.defaultProps = {middle: 0};
+        const Outer = React.memo(Middle);
+        Outer.propTypes = {outer: PropTypes.number.isRequired};
+        Outer.defaultProps = {outer: 0};
 
-        // Deduplication
-        ReactNoop.render(<Fn inner="2" outer="3" />);
+        // No warning expected because defaultProps satisfy both.
+        ReactNoop.render(<Outer />);
+        ReactNoop.flush();
+
+        // Mount
+        expect(() => {
+          ReactNoop.render(<Outer inner="2" middle="3" outer="4" />);
+        }).toWarnDev(
+          // Outer props are checked in createElement
+          'Invalid prop `outer` of type `string` supplied to `Inner`, expected `number`.',
+        );
+        expect(() => {
+          ReactNoop.flush();
+        }).toWarnDev([
+          // There are no actual elements created for these, so they're checked in reconciler.
+          'Invalid prop `middle` of type `string` supplied to `Inner`, expected `number`.',
+          'Invalid prop `inner` of type `string` supplied to `Inner`, expected `number`.',
+        ]);
+
+        // Update
+        expect(() => {
+          ReactNoop.render(
+            <Outer inner={false} middle={false} outer={false} />,
+          );
+        }).toWarnDev(
+          // Outer props are checked in createElement
+          'Invalid prop `outer` of type `boolean` supplied to `Inner`, expected `number`.',
+        );
+        expect(() => {
+          ReactNoop.flush();
+        }).toWarnDev([
+          // There are no actual elements created for these, so they're checked in reconciler.
+          'Invalid prop `middle` of type `boolean` supplied to `Inner`, expected `number`.',
+          'Invalid prop `inner` of type `boolean` supplied to `Inner`, expected `number`.',
+        ]);
       });
     });
   }

packages/react/src/ReactElementValidator.js

@@ -15,14 +15,12 @@
 import lowPriorityWarning from 'shared/lowPriorityWarning';
 import isValidElementType from 'shared/isValidElementType';
 import getComponentName from 'shared/getComponentName';
-import {refineResolvedLazyComponent} from 'shared/ReactLazyComponent';
 import {
   getIteratorFn,
   REACT_FORWARD_REF_TYPE,
+  REACT_MEMO_TYPE,
   REACT_FRAGMENT_TYPE,
   REACT_ELEMENT_TYPE,
-  REACT_LAZY_TYPE,
-  REACT_MEMO_TYPE,
 } from 'shared/ReactSymbols';
 import checkPropTypes from 'prop-types/checkPropTypes';
 import warning from 'shared/warning';
@@ -35,11 +33,9 @@ import ReactDebugCurrentFrame, {
 } from './ReactDebugCurrentFrame';
 
 let propTypesMisspellWarningShown;
-let didWarnAboutDuplicatePropTypes;
 
 if (__DEV__) {
   propTypesMisspellWarningShown = false;
-  didWarnAboutDuplicatePropTypes = {};
 }
 
 function getDeclarationErrorAddendum() {
@@ -182,42 +178,6 @@ function validateChildKeys(node, parentType) {
   }
 }
 
-function getPropTypes(type) {
-  if (typeof type === 'function') {
-    // Class or function component
-    return type.propTypes;
-  }
-  if (typeof type === 'object') {
-    switch (type.$$typeof) {
-      case REACT_FORWARD_REF_TYPE:
-        return type.propTypes;
-      case REACT_MEMO_TYPE:
-        const outerPropTypes = type.propTypes;
-        const innerPropTypes = type.type.propTypes;
-        if (outerPropTypes && innerPropTypes) {
-          const componentName = getComponentName(type);
-          if (!didWarnAboutDuplicatePropTypes[componentName]) {
-            didWarnAboutDuplicatePropTypes[componentName] = true;
-            warning(
-              false,
-              'React.memo(%s): `propTypes` are defined both on the `React.memo()` result and on the ' +
-                'inner `%s` component. Remove either one of these `propTypes` definitions.',
-              componentName,
-              componentName,
-            );
-          }
-        }
-        return outerPropTypes || innerPropTypes;
-      case REACT_LAZY_TYPE:
-        const resolvedType = refineResolvedLazyComponent(type);
-        if (resolvedType !== null) {
-          return getPropTypes(resolvedType);
-        }
-    }
-  }
-  return null;
-}
-
 /**
  * Given an element, validate that its props follow the propTypes definition,
  * provided by the type.
@@ -226,11 +186,24 @@ function getPropTypes(type) {
  */
 function validatePropTypes(element) {
   const type = element.type;
-  if (typeof type === 'string' || type === null || type === undefined) {
-    return null;
+  if (type === null || type === undefined || typeof type === 'string') {
+    return;
   }
   const name = getComponentName(type);
-  const propTypes = getPropTypes(type);
+  let propTypes;
+  if (typeof type === 'function') {
+    propTypes = type.propTypes;
+  } else if (
+    typeof type === 'object' &&
+    (type.$$typeof === REACT_FORWARD_REF_TYPE ||
+      // Note: Memo only checks outer props here.
+      // Inner props are checked in the reconciler.
+      type.$$typeof === REACT_MEMO_TYPE)
+  ) {
+    propTypes = type.propTypes;
+  } else {
+    return;
+  }
   if (propTypes) {
     setCurrentlyValidatingElement(element);
     checkPropTypes(

packages/shared/getComponentName.js

@@ -80,6 +80,9 @@ function getComponentName(type: mixed): string | null {
       case REACT_FORWARD_REF_TYPE:
         return getWrappedName(type, type.render, 'ForwardRef');
       case REACT_MEMO_TYPE:
+        if (type.displayName) {
+          return type.displayName;
+        }
         return getComponentName(type.type);
       case REACT_LAZY_TYPE: {
         const thenable: LazyComponent<mixed> = (type: any);