diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index a6de9c5009..7575eaa167 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/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, ); diff --git a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js index 0ea276d0fd..52d2327d14 100644 --- a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js +++ b/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( - }> - - , - ); + 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( + }> + + , + ); + 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( - }> - - , - { - 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( - }> - - , - ); - }).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 () => { diff --git a/packages/react-reconciler/src/__tests__/ReactMemo-test.internal.js b/packages/react-reconciler/src/__tests__/ReactMemo-test.internal.js index 2a71174be6..dbbe0683d0 100644 --- a/packages/react-reconciler/src/__tests__/ReactMemo-test.internal.js +++ b/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(); expect(() => { - ReactNoop.render(); + 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(); + 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(); + ReactNoop.render(); }).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(); + }).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(); - }).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(); + // No warning expected because defaultProps satisfy both. + ReactNoop.render(); + ReactNoop.flush(); + + // Mount + expect(() => { + ReactNoop.render(); + }).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( + , + ); + }).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`.', + ]); }); }); } diff --git a/packages/react/src/ReactElementValidator.js b/packages/react/src/ReactElementValidator.js index 4105fb0976..f25677580e 100644 --- a/packages/react/src/ReactElementValidator.js +++ b/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( diff --git a/packages/shared/getComponentName.js b/packages/shared/getComponentName.js index cbea431c8f..30c4e7deb9 100644 --- a/packages/shared/getComponentName.js +++ b/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 = (type: any);