From 6f2849ef7eb6a5cc8e26e202e01411c854c2e74b Mon Sep 17 00:00:00 2001 From: Waseem Dahman Date: Sun, 3 Nov 2019 22:50:28 -0500 Subject: [PATCH 1/4] [react-is] return correct typeOf value of forwardRef --- packages/react-is/src/ReactIs.js | 1 + packages/react-is/src/__tests__/ReactIs-test.js | 9 ++++++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/react-is/src/ReactIs.js b/packages/react-is/src/ReactIs.js index 085decb4a74fe..6900c686302cd 100644 --- a/packages/react-is/src/ReactIs.js +++ b/packages/react-is/src/ReactIs.js @@ -56,6 +56,7 @@ export function typeOf(object: any) { } case REACT_LAZY_TYPE: case REACT_MEMO_TYPE: + case REACT_FORWARD_REF_TYPE: case REACT_PORTAL_TYPE: return $$typeof; } diff --git a/packages/react-is/src/__tests__/ReactIs-test.js b/packages/react-is/src/__tests__/ReactIs-test.js index 1c47fd551bb58..373f985c1eb68 100644 --- a/packages/react-is/src/__tests__/ReactIs-test.js +++ b/packages/react-is/src/__tests__/ReactIs-test.js @@ -106,7 +106,9 @@ describe('ReactIs', () => { it('should identify ref forwarding component', () => { const RefForwardingComponent = React.forwardRef((props, ref) => null); + expect(ReactIs.typeOf(RefForwardingComponent)).toBe(ReactIs.ForwardRef); expect(ReactIs.typeOf()).toBe(ReactIs.ForwardRef); + expect(ReactIs.isForwardRef(RefForwardingComponent)).toBe(true); expect(ReactIs.isForwardRef()).toBe(true); expect(ReactIs.isForwardRef({type: ReactIs.StrictMode})).toBe(false); expect(ReactIs.isForwardRef(
)).toBe(false); @@ -131,9 +133,10 @@ describe('ReactIs', () => { it('should identify memo', () => { const Component = () => React.createElement('div'); - const memoized = React.memo(Component); - expect(ReactIs.typeOf(memoized)).toBe(ReactIs.Memo); - expect(ReactIs.isMemo(memoized)).toBe(true); + const Memoized = React.memo(Component); + expect(ReactIs.typeOf(Memoized)).toBe(ReactIs.Memo); + expect(ReactIs.typeOf()).toBe(ReactIs.Element); + expect(ReactIs.isMemo(Memoized)).toBe(true); expect(ReactIs.isMemo(Component)).toBe(false); }); From 04341d4896f04de770d57fbfaa1f2a33bdca3bd3 Mon Sep 17 00:00:00 2001 From: Waseem Dahman Date: Sun, 3 Nov 2019 22:53:30 -0500 Subject: [PATCH 2/4] [react-devtools-shared] use correct displayName of memo(forwardRef(Component)) --- .../react-devtools-shared/src/backend/renderer.js | 14 ++++---------- packages/react-devtools-shared/src/utils.js | 6 ++++++ 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js index 761c7893505e2..bc21b4c9dfa8d 100644 --- a/packages/react-devtools-shared/src/backend/renderer.js +++ b/packages/react-devtools-shared/src/backend/renderer.js @@ -328,7 +328,7 @@ export function getInternalReactConstants( // NOTICE Keep in sync with shouldFilterFiber() and other get*ForFiber methods function getDisplayNameForFiber(fiber: Fiber): string | null { - const {elementType, type, tag} = fiber; + const {type, tag} = fiber; // This is to support lazy components with a Promise as the type. // see https://github.com/facebook/react/pull/13397 @@ -348,10 +348,11 @@ export function getInternalReactConstants( case FunctionComponent: case IndeterminateComponent: return getDisplayName(resolvedType); + case MemoComponent: + case SimpleMemoComponent: case ForwardRef: return ( - resolvedType.displayName || - getDisplayName(resolvedType.render, 'Anonymous') + resolvedType.displayName || getDisplayName(resolvedType, 'Anonymous') ); case HostRoot: return null; @@ -361,13 +362,6 @@ export function getInternalReactConstants( case HostText: case Fragment: return null; - case MemoComponent: - case SimpleMemoComponent: - if (elementType.displayName) { - return elementType.displayName; - } else { - return getDisplayName(type, 'Anonymous'); - } case SuspenseComponent: return 'Suspense'; case SuspenseListComponent: diff --git a/packages/react-devtools-shared/src/utils.js b/packages/react-devtools-shared/src/utils.js index 5c22b92100b62..672ba75cf21dc 100644 --- a/packages/react-devtools-shared/src/utils.js +++ b/packages/react-devtools-shared/src/utils.js @@ -29,6 +29,8 @@ import { } from 'react-devtools-shared/src/types'; import {localStorageGetItem, localStorageSetItem} from './storage'; +import {isMemo, isForwardRef} from 'react-is'; + import type {ComponentFilter, ElementType} from './types'; const cachedDisplayNames: WeakMap = new WeakMap(); @@ -55,6 +57,10 @@ export function getDisplayName( displayName = type.displayName; } else if (typeof type.name === 'string' && type.name !== '') { displayName = type.name; + } else if (isMemo(type)) { + displayName = getDisplayName(type.type); + } else if (isForwardRef(type)) { + displayName = getDisplayName(type.render); } cachedDisplayNames.set(type, displayName); From c55be004a740af82e6142b443ffd0f1f2a480a61 Mon Sep 17 00:00:00 2001 From: Waseem Dahman Date: Mon, 4 Nov 2019 22:53:29 -0500 Subject: [PATCH 3/4] [react-devtools-shared] add resolveFiberType and resolve fiber type of memo recursively Resolving the fiber type of memo recursively before passing it to getDisplayName will prevent it from displaying "Anonymous" as displayName for components wrapped with both memo and forwardRef: memo(forwardRef(Component)) --- .../src/backend/renderer.js | 34 ++++++++++++++----- packages/react-devtools-shared/src/utils.js | 6 ---- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js index bc21b4c9dfa8d..5dbd77362fea9 100644 --- a/packages/react-devtools-shared/src/backend/renderer.js +++ b/packages/react-devtools-shared/src/backend/renderer.js @@ -49,6 +49,7 @@ import { patch as patchConsole, registerRenderer as registerRendererWithConsole, } from './console'; +import {isMemo, isForwardRef} from 'react-is'; import type {Fiber} from 'react-reconciler/src/ReactFiber'; import type { @@ -326,17 +327,29 @@ export function getInternalReactConstants( SCOPE_SYMBOL_STRING, } = ReactSymbols; + function resolveFiberType(type: any) { + // This is to support lazy components with a Promise as the type. + // see https://github.com/facebook/react/pull/13397 + if (typeof type.then === 'function') { + return type._reactResult; + } + if (isForwardRef(type)) { + return type.render; + } + // recursively resolving memo type in case of memo(forwardRef(Component)) + if (isMemo(type)) { + return resolveFiberType(type.type); + } + return type; + } + // 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; - // This is to support lazy components with a Promise as the type. - // see https://github.com/facebook/react/pull/13397 let resolvedType = type; if (typeof type === 'object' && type !== null) { - if (typeof type.then === 'function') { - resolvedType = type._reactResult; - } + resolvedType = resolveFiberType(type); } let resolvedContext: any = null; @@ -348,8 +361,6 @@ export function getInternalReactConstants( case FunctionComponent: case IndeterminateComponent: return getDisplayName(resolvedType); - case MemoComponent: - case SimpleMemoComponent: case ForwardRef: return ( resolvedType.displayName || getDisplayName(resolvedType, 'Anonymous') @@ -362,6 +373,13 @@ export function getInternalReactConstants( case HostText: case Fragment: return null; + case MemoComponent: + case SimpleMemoComponent: + if (elementType.displayName) { + return elementType.displayName; + } else { + return getDisplayName(resolvedType, 'Anonymous'); + } case SuspenseComponent: return 'Suspense'; case SuspenseListComponent: diff --git a/packages/react-devtools-shared/src/utils.js b/packages/react-devtools-shared/src/utils.js index 672ba75cf21dc..5c22b92100b62 100644 --- a/packages/react-devtools-shared/src/utils.js +++ b/packages/react-devtools-shared/src/utils.js @@ -29,8 +29,6 @@ import { } from 'react-devtools-shared/src/types'; import {localStorageGetItem, localStorageSetItem} from './storage'; -import {isMemo, isForwardRef} from 'react-is'; - import type {ComponentFilter, ElementType} from './types'; const cachedDisplayNames: WeakMap = new WeakMap(); @@ -57,10 +55,6 @@ export function getDisplayName( displayName = type.displayName; } else if (typeof type.name === 'string' && type.name !== '') { displayName = type.name; - } else if (isMemo(type)) { - displayName = getDisplayName(type.type); - } else if (isForwardRef(type)) { - displayName = getDisplayName(type.render); } cachedDisplayNames.set(type, displayName); From 6bcd7573184afcc43990f4820667cd15139449b1 Mon Sep 17 00:00:00 2001 From: Waseem Dahman Date: Tue, 5 Nov 2019 16:24:35 -0500 Subject: [PATCH 4/4] rework resolveFiberType --- .../src/backend/renderer.js | 23 ++++++++++++------- .../react-is/src/__tests__/ReactIs-test.js | 1 - 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js index 5dbd77362fea9..d12215c03369a 100644 --- a/packages/react-devtools-shared/src/backend/renderer.js +++ b/packages/react-devtools-shared/src/backend/renderer.js @@ -49,7 +49,6 @@ import { patch as patchConsole, registerRenderer as registerRendererWithConsole, } from './console'; -import {isMemo, isForwardRef} from 'react-is'; import type {Fiber} from 'react-reconciler/src/ReactFiber'; import type { @@ -325,6 +324,10 @@ export function getInternalReactConstants( PROFILER_SYMBOL_STRING, SCOPE_NUMBER, SCOPE_SYMBOL_STRING, + FORWARD_REF_NUMBER, + FORWARD_REF_SYMBOL_STRING, + MEMO_NUMBER, + MEMO_SYMBOL_STRING, } = ReactSymbols; function resolveFiberType(type: any) { @@ -333,14 +336,18 @@ export function getInternalReactConstants( if (typeof type.then === 'function') { return type._reactResult; } - if (isForwardRef(type)) { - return type.render; - } - // recursively resolving memo type in case of memo(forwardRef(Component)) - if (isMemo(type)) { - return resolveFiberType(type.type); + const typeSymbol = getTypeSymbol(type); + switch (typeSymbol) { + case MEMO_NUMBER: + case MEMO_SYMBOL_STRING: + // recursively resolving memo type in case of memo(forwardRef(Component)) + return resolveFiberType(type.type); + case FORWARD_REF_NUMBER: + case FORWARD_REF_SYMBOL_STRING: + return type.render; + default: + return type; } - return type; } // NOTICE Keep in sync with shouldFilterFiber() and other get*ForFiber methods diff --git a/packages/react-is/src/__tests__/ReactIs-test.js b/packages/react-is/src/__tests__/ReactIs-test.js index 50f703e22e976..ae2de7fe1cfe5 100644 --- a/packages/react-is/src/__tests__/ReactIs-test.js +++ b/packages/react-is/src/__tests__/ReactIs-test.js @@ -110,7 +110,6 @@ describe('ReactIs', () => { const RefForwardingComponent = React.forwardRef((props, ref) => null); expect(ReactIs.isValidElementType(RefForwardingComponent)).toBe(true); expect(ReactIs.typeOf()).toBe(ReactIs.ForwardRef); - expect(ReactIs.isForwardRef(RefForwardingComponent)).toBe(true); expect(ReactIs.isForwardRef()).toBe(true); expect(ReactIs.isForwardRef({type: ReactIs.StrictMode})).toBe(false); expect(ReactIs.isForwardRef(
)).toBe(false);