diff --git a/packages/react-devtools-shared/src/__tests__/__snapshots__/store-test.js.snap b/packages/react-devtools-shared/src/__tests__/__snapshots__/store-test.js.snap index 5ff48f317f479..eedcaa33fc07d 100644 --- a/packages/react-devtools-shared/src/__tests__/__snapshots__/store-test.js.snap +++ b/packages/react-devtools-shared/src/__tests__/__snapshots__/store-test.js.snap @@ -670,20 +670,3 @@ exports[`Store should properly serialize non-string key values: 1: mount 1`] = ` [root] `; - -exports[`Store should show the right display names for special component types 1`] = ` -[root] - ▾ - - [ForwardRef] - ▾ [ForwardRef] - - [ForwardRef] - [Memo] - ▾ [Memo] - [ForwardRef] - [withFoo][withBar] - [Memo][withFoo][withBar] - [ForwardRef][withFoo][withBar] - -`; diff --git a/packages/react-devtools-shared/src/__tests__/store-test.js b/packages/react-devtools-shared/src/__tests__/store-test.js index f971946b686aa..b63b8f088e8c6 100644 --- a/packages/react-devtools-shared/src/__tests__/store-test.js +++ b/packages/react-devtools-shared/src/__tests__/store-test.js @@ -879,6 +879,17 @@ describe('Store', () => { FakeHigherOrderComponent, ); + const MemoizedFakeHigherOrderComponentWithDisplayNameOverride = React.memo( + FakeHigherOrderComponent, + ); + MemoizedFakeHigherOrderComponentWithDisplayNameOverride.displayName = + 'memoRefOverride'; + const ForwardRefFakeHigherOrderComponentWithDisplayNameOverride = React.forwardRef( + FakeHigherOrderComponent, + ); + ForwardRefFakeHigherOrderComponentWithDisplayNameOverride.displayName = + 'forwardRefOverride'; + const App = () => ( @@ -891,6 +902,8 @@ describe('Store', () => { + + ); @@ -904,7 +917,24 @@ describe('Store', () => { // Render again after it resolves act(() => ReactDOM.render(, container)); - expect(store).toMatchSnapshot(); + expect(store).toMatchInlineSnapshot(` + [root] + ▾ + + [ForwardRef] + ▾ [ForwardRef] + + [ForwardRef] + [Memo] + ▾ [Memo] + [ForwardRef] + [withFoo][withBar] + [Memo][withFoo][withBar] + [ForwardRef][withFoo][withBar] + + [Memo] + [ForwardRef] + `); }); describe('Lazy', () => { diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js index a45f510ff230e..18fa9d7efddae 100644 --- a/packages/react-devtools-shared/src/backend/renderer.js +++ b/packages/react-devtools-shared/src/backend/renderer.js @@ -393,7 +393,7 @@ export function getInternalReactConstants( // NOTICE Keep in sync with shouldFilterFiber() and other get*ForFiber methods function getDisplayNameForFiber(fiber: Fiber): string | null { - const {type, tag} = fiber; + const {elementType, type, tag} = fiber; let resolvedType = type; if (typeof type === 'object' && type !== null) { @@ -432,7 +432,11 @@ export function getInternalReactConstants( return 'Lazy'; case MemoComponent: case SimpleMemoComponent: - return getDisplayName(resolvedType, 'Anonymous'); + return ( + (elementType && elementType.displayName) || + (type && type.displayName) || + getDisplayName(resolvedType, 'Anonymous') + ); case SuspenseComponent: return 'Suspense'; case LegacyHiddenComponent: diff --git a/packages/react-reconciler/src/__tests__/ReactMemo-test.js b/packages/react-reconciler/src/__tests__/ReactMemo-test.js index 5be63d65a3757..89bf6e2cd4e60 100644 --- a/packages/react-reconciler/src/__tests__/ReactMemo-test.js +++ b/packages/react-reconciler/src/__tests__/ReactMemo-test.js @@ -498,11 +498,66 @@ describe('memo', () => { }); }); + it('should fall back to showing something meaningful if no displayName or name are present', () => { + const MemoComponent = React.memo(props =>
); + MemoComponent.propTypes = { + required: PropTypes.string.isRequired, + }; + + expect(() => + ReactNoop.render(), + ).toErrorDev( + 'Warning: Failed prop type: The prop `required` is marked as required in ' + + '`Memo`, but its value is `undefined`.', + // There's no component stack in this warning because the inner function is anonymous. + // If we wanted to support this (for the Error frames / source location) + // we could do this by updating ReactComponentStackFrame. + {withoutStack: true}, + ); + }); + + it('should honor a displayName if set on the inner component in warnings', () => { + function Component(props) { + return
; + } + Component.displayName = 'Inner'; + const MemoComponent = React.memo(Component); + MemoComponent.propTypes = { + required: PropTypes.string.isRequired, + }; + + expect(() => + ReactNoop.render(), + ).toErrorDev( + 'Warning: Failed prop type: The prop `required` is marked as required in ' + + '`Inner`, but its value is `undefined`.\n' + + ' in Inner (at **)', + ); + }); + it('should honor a displayName if set on the memo wrapper in warnings', () => { const MemoComponent = React.memo(function Component(props) { return
; }); - MemoComponent.displayName = 'Foo'; + MemoComponent.displayName = 'Outer'; + MemoComponent.propTypes = { + required: PropTypes.string.isRequired, + }; + + expect(() => + ReactNoop.render(), + ).toErrorDev( + 'Warning: Failed prop type: The prop `required` is marked as required in ' + + '`Outer`, but its value is `undefined`.\n' + + ' in Component (at **)', + ); + }); + + it('should pass displayName to an anonymous inner component so it shows up in component stacks', () => { + const MemoComponent = React.memo(props => { + return
; + }); + MemoComponent.displayName = 'Memo'; MemoComponent.propTypes = { required: PropTypes.string.isRequired, }; @@ -511,19 +566,19 @@ describe('memo', () => { ReactNoop.render(), ).toErrorDev( 'Warning: Failed prop type: The prop `required` is marked as required in ' + - '`Foo`, but its value is `undefined`.\n' + - ' in Foo (at **)', + '`Memo`, but its value is `undefined`.\n' + + ' in Memo (at **)', ); }); - it('should honor a inner displayName if set on the wrapped function', () => { + it('should honor a outer displayName when wrapped component and memo component set displayName at the same time.', () => { function Component(props) { return
; } - Component.displayName = 'Foo'; + Component.displayName = 'Inner'; const MemoComponent = React.memo(Component); - MemoComponent.displayName = 'Bar'; + MemoComponent.displayName = 'Outer'; MemoComponent.propTypes = { required: PropTypes.string.isRequired, }; @@ -532,8 +587,8 @@ describe('memo', () => { ReactNoop.render(), ).toErrorDev( 'Warning: Failed prop type: The prop `required` is marked as required in ' + - '`Foo`, but its value is `undefined`.\n' + - ' in Foo (at **)', + '`Outer`, but its value is `undefined`.\n' + + ' in Inner (at **)', ); }); } diff --git a/packages/react/src/ReactForwardRef.js b/packages/react/src/ReactForwardRef.js index d76310ac74cf8..796571d51f6b3 100644 --- a/packages/react/src/ReactForwardRef.js +++ b/packages/react/src/ReactForwardRef.js @@ -57,7 +57,15 @@ export function forwardRef( }, set: function(name) { ownName = name; - if (render.displayName == null) { + + // The inner component shouldn't inherit this display name in most cases, + // because the component may be used elsewhere. + // But it's nice for anonymous functions to inherit the name, + // so that our component-stack generation logic will display their frames. + // An anonymous function generally suggests a pattern like: + // React.forwardRef((props, ref) => {...}); + // This kind of inner function is not used elsewhere so the side effect is okay. + if (!render.name && !render.displayName) { render.displayName = name; } }, diff --git a/packages/react/src/ReactMemo.js b/packages/react/src/ReactMemo.js index 2531c1a1879f0..d742948bf5ede 100644 --- a/packages/react/src/ReactMemo.js +++ b/packages/react/src/ReactMemo.js @@ -37,7 +37,15 @@ export function memo( }, set: function(name) { ownName = name; - if (type.displayName == null) { + + // The inner component shouldn't inherit this display name in most cases, + // because the component may be used elsewhere. + // But it's nice for anonymous functions to inherit the name, + // so that our component-stack generation logic will display their frames. + // An anonymous function generally suggests a pattern like: + // React.memo((props) => {...}); + // This kind of inner function is not used elsewhere so the side effect is okay. + if (!type.name && !type.displayName) { type.displayName = name; } }, diff --git a/packages/react/src/__tests__/forwardRef-test.js b/packages/react/src/__tests__/forwardRef-test.js index 4dfb4a3f9cd88..0c2ee9aa4ea72 100644 --- a/packages/react/src/__tests__/forwardRef-test.js +++ b/packages/react/src/__tests__/forwardRef-test.js @@ -217,14 +217,43 @@ describe('forwardRef', () => { ); }); - it('should honor a displayName if set on the forwardRef wrapper in warnings', () => { + it('should fall back to showing something meaningful if no displayName or name are present', () => { const Component = props =>
; const RefForwardingComponent = React.forwardRef((props, ref) => ( )); - RefForwardingComponent.displayName = 'Foo'; + RefForwardingComponent.propTypes = { + optional: PropTypes.string, + required: PropTypes.string.isRequired, + }; + + RefForwardingComponent.defaultProps = { + optional: 'default', + }; + + const ref = React.createRef(); + + expect(() => + ReactNoop.render(), + ).toErrorDev( + 'Warning: Failed prop type: The prop `required` is marked as required in ' + + '`ForwardRef`, but its value is `undefined`.', + // There's no component stack in this warning because the inner function is anonymous. + // If we wanted to support this (for the Error frames / source location) + // we could do this by updating ReactComponentStackFrame. + {withoutStack: true}, + ); + }); + + it('should honor a displayName if set on the forwardRef wrapper in warnings', () => { + const Component = props =>
; + + const RefForwardingComponent = React.forwardRef(function Inner(props, ref) { + ; + }); + RefForwardingComponent.displayName = 'Custom'; RefForwardingComponent.propTypes = { optional: PropTypes.string, @@ -241,8 +270,36 @@ describe('forwardRef', () => { ReactNoop.render(), ).toErrorDev( 'Warning: Failed prop type: The prop `required` is marked as required in ' + - '`Foo`, but its value is `undefined`.\n' + - ' in Foo (at **)', + '`Custom`, but its value is `undefined`.\n' + + ' in Inner (at **)', + ); + }); + + it('should pass displayName to an anonymous inner component so it shows up in component stacks', () => { + const Component = props =>
; + + const RefForwardingComponent = React.forwardRef((props, ref) => ( + + )); + RefForwardingComponent.displayName = 'Custom'; + + RefForwardingComponent.propTypes = { + optional: PropTypes.string, + required: PropTypes.string.isRequired, + }; + + RefForwardingComponent.defaultProps = { + optional: 'default', + }; + + const ref = React.createRef(); + + expect(() => + ReactNoop.render(), + ).toErrorDev( + 'Warning: Failed prop type: The prop `required` is marked as required in ' + + '`Custom`, but its value is `undefined`.\n' + + ' in Custom (at **)', ); }); @@ -250,8 +307,36 @@ describe('forwardRef', () => { const Component = props =>
; const inner = (props, ref) => ; - inner.displayName = 'Foo'; + inner.displayName = 'Inner'; + const RefForwardingComponent = React.forwardRef(inner); + + RefForwardingComponent.propTypes = { + optional: PropTypes.string, + required: PropTypes.string.isRequired, + }; + + RefForwardingComponent.defaultProps = { + optional: 'default', + }; + + const ref = React.createRef(); + + expect(() => + ReactNoop.render(), + ).toErrorDev( + 'Warning: Failed prop type: The prop `required` is marked as required in ' + + '`ForwardRef(Inner)`, but its value is `undefined`.\n' + + ' in Inner (at **)', + ); + }); + + it('should honor a outer displayName when wrapped component and memo component set displayName at the same time.', () => { + const Component = props =>
; + + const inner = (props, ref) => ; + inner.displayName = 'Inner'; const RefForwardingComponent = React.forwardRef(inner); + RefForwardingComponent.displayName = 'Outer'; RefForwardingComponent.propTypes = { optional: PropTypes.string, @@ -268,8 +353,8 @@ describe('forwardRef', () => { ReactNoop.render(), ).toErrorDev( 'Warning: Failed prop type: The prop `required` is marked as required in ' + - '`ForwardRef(Foo)`, but its value is `undefined`.\n' + - ' in Foo (at **)', + '`Outer`, but its value is `undefined`.\n' + + ' in Inner (at **)', ); }); diff --git a/packages/shared/getComponentNameFromType.js b/packages/shared/getComponentNameFromType.js index f0f51d852f8d3..7428f8c11d735 100644 --- a/packages/shared/getComponentNameFromType.js +++ b/packages/shared/getComponentNameFromType.js @@ -31,11 +31,12 @@ function getWrappedName( innerType: any, wrapperName: string, ): string { + const displayName = (outerType: any).displayName; + if (displayName) { + return displayName; + } const functionName = innerType.displayName || innerType.name || ''; - return ( - (outerType: any).displayName || - (functionName !== '' ? `${wrapperName}(${functionName})` : wrapperName) - ); + return functionName !== '' ? `${wrapperName}(${functionName})` : wrapperName; } // Keep in sync with react-reconciler/getComponentNameFromFiber @@ -90,7 +91,11 @@ export default function getComponentNameFromType(type: mixed): string | null { case REACT_FORWARD_REF_TYPE: return getWrappedName(type, type.render, 'ForwardRef'); case REACT_MEMO_TYPE: - return getComponentNameFromType(type.type); + const outerName = (type: any).displayName || null; + if (outerName !== null) { + return outerName; + } + return getComponentNameFromType(type.type) || 'Memo'; case REACT_LAZY_TYPE: { const lazyComponent: LazyComponent = (type: any); const payload = lazyComponent._payload;