From f295d69bc74b0f46a45b29bda727ef4e40da5237 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Thu, 12 Apr 2018 09:02:42 -0700 Subject: [PATCH 1/6] Added failing debug perf tests for AsyncMode, StrictMode, and forwardRef components --- .../ReactIncrementalPerf-test.internal.js | 28 ++++++++++++++++ ...ReactIncrementalPerf-test.internal.js.snap | 32 +++++++++++++++++++ 2 files changed, 60 insertions(+) diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalPerf-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalPerf-test.internal.js index 854a85efadc8c..e626ab2a79f32 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalPerf-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalPerf-test.internal.js @@ -161,6 +161,34 @@ describe('ReactDebugFiberPerf', () => { expect(getFlameChart()).toMatchSnapshot(); }); + it('measures and identifies a forwardRef component correctly', () => { + const ForwardRef = React.forwardRef(function refForwarder(props, ref) { + return ; + }); + + ReactNoop.render(); + addComment('Mount'); + ReactNoop.flush(); + + expect(getFlameChart()).toMatchSnapshot(); + }); + + it('measures and identifies StrictMode and AsyncMode components correctly', () => { + ReactNoop.render( + + + + + + + , + ); + addComment('Mount'); + ReactNoop.flush(); + + expect(getFlameChart()).toMatchSnapshot(); + }); + it('skips parents during setState', () => { class A extends React.Component { render() { diff --git a/packages/react-reconciler/src/__tests__/__snapshots__/ReactIncrementalPerf-test.internal.js.snap b/packages/react-reconciler/src/__tests__/__snapshots__/ReactIncrementalPerf-test.internal.js.snap index 48d0e6f4f67cd..bbdf536683fa8 100644 --- a/packages/react-reconciler/src/__tests__/__snapshots__/ReactIncrementalPerf-test.internal.js.snap +++ b/packages/react-reconciler/src/__tests__/__snapshots__/ReactIncrementalPerf-test.internal.js.snap @@ -177,6 +177,38 @@ exports[`ReactDebugFiberPerf measures a simple reconciliation 1`] = ` " `; +exports[`ReactDebugFiberPerf measures and identifies StrictMode and AsyncMode components correctly 1`] = ` +"⚛ (Waiting for async callback... will force flush in 5230 ms) + +// Mount +⚛ (React Tree Reconciliation: Completed Root) + ⚛ Unknown [mount] + ⚛ Parent [mount] + ⚛ Unknown [mount] + ⚛ Child [mount] + +⚛ (Committing Changes) + ⚛ (Committing Snapshot Effects: 0 Total) + ⚛ (Committing Host Effects: 1 Total) + ⚛ (Calling Lifecycle Methods: 0 Total) +" +`; + +exports[`ReactDebugFiberPerf measures and identifies a forwardRef component correctly 1`] = ` +"⚛ (Waiting for async callback... will force flush in 5230 ms) + +// Mount +⚛ (React Tree Reconciliation: Completed Root) + ⚛ Unknown [mount] + ⚛ Child [mount] + +⚛ (Committing Changes) + ⚛ (Committing Snapshot Effects: 0 Total) + ⚛ (Committing Host Effects: 1 Total) + ⚛ (Calling Lifecycle Methods: 0 Total) +" +`; + exports[`ReactDebugFiberPerf measures deferred work in chunks 1`] = ` "⚛ (Waiting for async callback... will force flush in 5230 ms) From d24af4562123c8d0fc1db30957d7a08b75359e8d Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Thu, 12 Apr 2018 09:04:17 -0700 Subject: [PATCH 2/6] Fixed labels and updated snapshots --- .../ReactIncrementalPerf-test.internal.js.snap | 6 +++--- packages/shared/getComponentName.js | 18 ++++++++++++++++-- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/packages/react-reconciler/src/__tests__/__snapshots__/ReactIncrementalPerf-test.internal.js.snap b/packages/react-reconciler/src/__tests__/__snapshots__/ReactIncrementalPerf-test.internal.js.snap index bbdf536683fa8..b2e52b9096d73 100644 --- a/packages/react-reconciler/src/__tests__/__snapshots__/ReactIncrementalPerf-test.internal.js.snap +++ b/packages/react-reconciler/src/__tests__/__snapshots__/ReactIncrementalPerf-test.internal.js.snap @@ -182,9 +182,9 @@ exports[`ReactDebugFiberPerf measures and identifies StrictMode and AsyncMode co // Mount ⚛ (React Tree Reconciliation: Completed Root) - ⚛ Unknown [mount] + ⚛ StrictMode [mount] ⚛ Parent [mount] - ⚛ Unknown [mount] + ⚛ AsyncMode [mount] ⚛ Child [mount] ⚛ (Committing Changes) @@ -199,7 +199,7 @@ exports[`ReactDebugFiberPerf measures and identifies a forwardRef component corr // Mount ⚛ (React Tree Reconciliation: Completed Root) - ⚛ Unknown [mount] + ⚛ ForwardRef [mount] ⚛ Child [mount] ⚛ (Committing Changes) diff --git a/packages/shared/getComponentName.js b/packages/shared/getComponentName.js index e00bd89725162..87325d90804f9 100644 --- a/packages/shared/getComponentName.js +++ b/packages/shared/getComponentName.js @@ -10,10 +10,13 @@ import type {Fiber} from 'react-reconciler/src/ReactFiber'; import { + REACT_ASYNC_MODE_TYPE, REACT_CALL_TYPE, + REACT_FORWARD_REF_TYPE, REACT_FRAGMENT_TYPE, REACT_RETURN_TYPE, REACT_PORTAL_TYPE, + REACT_STRICT_MODE_TYPE, } from 'shared/ReactSymbols'; function getComponentName(fiber: Fiber): string | null { @@ -25,15 +28,26 @@ function getComponentName(fiber: Fiber): string | null { return type; } switch (type) { + case REACT_ASYNC_MODE_TYPE: + return 'AsyncMode'; + case REACT_CALL_TYPE: + return 'ReactCall'; case REACT_FRAGMENT_TYPE: return 'ReactFragment'; case REACT_PORTAL_TYPE: return 'ReactPortal'; - case REACT_CALL_TYPE: - return 'ReactCall'; case REACT_RETURN_TYPE: return 'ReactReturn'; + case REACT_STRICT_MODE_TYPE: + return 'StrictMode'; + } + if (typeof type === 'object' && type !== null) { + switch (type.$$typeof) { + case REACT_FORWARD_REF_TYPE: + return 'ForwardRef'; + } } + return null; } From a5a6bf15a2f5a9478ae1888b36bd245974ce2fe1 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Thu, 12 Apr 2018 09:13:55 -0700 Subject: [PATCH 3/6] Updated to exclude AsyncMode, StrictMode, and forwardRef from perf timings. --- .../src/ReactDebugFiberPerf.js | 4 + .../ReactIncrementalPerf-test.internal.js | 20 ++++- ...ReactIncrementalPerf-test.internal.js.snap | 76 +++++++++++-------- packages/shared/getComponentName.js | 18 +---- 4 files changed, 68 insertions(+), 50 deletions(-) diff --git a/packages/react-reconciler/src/ReactDebugFiberPerf.js b/packages/react-reconciler/src/ReactDebugFiberPerf.js index 1c84527e9ce77..492e705293c54 100644 --- a/packages/react-reconciler/src/ReactDebugFiberPerf.js +++ b/packages/react-reconciler/src/ReactDebugFiberPerf.js @@ -21,6 +21,8 @@ import { Fragment, ContextProvider, ContextConsumer, + Mode, + ForwardRef, } from 'shared/ReactTypeOfWork'; type MeasurementPhase = @@ -175,6 +177,8 @@ const shouldIgnoreFiber = (fiber: Fiber): boolean => { case Fragment: case ContextProvider: case ContextConsumer: + case Mode: + case ForwardRef: return true; default: return false; diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalPerf-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalPerf-test.internal.js index e626ab2a79f32..2fd39ce8f2ac4 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalPerf-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalPerf-test.internal.js @@ -161,7 +161,7 @@ describe('ReactDebugFiberPerf', () => { expect(getFlameChart()).toMatchSnapshot(); }); - it('measures and identifies a forwardRef component correctly', () => { + it('does not include forwardRef component in measurements', () => { const ForwardRef = React.forwardRef(function refForwarder(props, ref) { return ; }); @@ -173,7 +173,7 @@ describe('ReactDebugFiberPerf', () => { expect(getFlameChart()).toMatchSnapshot(); }); - it('measures and identifies StrictMode and AsyncMode components correctly', () => { + it('does not include StrictMode or AsyncMode components in measurements', () => { ReactNoop.render( @@ -189,6 +189,22 @@ describe('ReactDebugFiberPerf', () => { expect(getFlameChart()).toMatchSnapshot(); }); + it('does not include context provider or consumer in measurements', () => { + const {Consumer, Provider} = React.createContext(true); + + ReactNoop.render( + + + {value => } + + , + ); + addComment('Mount'); + ReactNoop.flush(); + + expect(getFlameChart()).toMatchSnapshot(); + }); + it('skips parents during setState', () => { class A extends React.Component { render() { diff --git a/packages/react-reconciler/src/__tests__/__snapshots__/ReactIncrementalPerf-test.internal.js.snap b/packages/react-reconciler/src/__tests__/__snapshots__/ReactIncrementalPerf-test.internal.js.snap index b2e52b9096d73..9259aa8b743c7 100644 --- a/packages/react-reconciler/src/__tests__/__snapshots__/ReactIncrementalPerf-test.internal.js.snap +++ b/packages/react-reconciler/src/__tests__/__snapshots__/ReactIncrementalPerf-test.internal.js.snap @@ -91,6 +91,50 @@ exports[`ReactDebugFiberPerf deduplicates lifecycle names during commit to reduc " `; +exports[`ReactDebugFiberPerf does not include StrictMode or AsyncMode components in measurements 1`] = ` +"⚛ (Waiting for async callback... will force flush in 5230 ms) + +// Mount +⚛ (React Tree Reconciliation: Completed Root) + ⚛ Parent [mount] + ⚛ Child [mount] + +⚛ (Committing Changes) + ⚛ (Committing Snapshot Effects: 0 Total) + ⚛ (Committing Host Effects: 1 Total) + ⚛ (Calling Lifecycle Methods: 0 Total) +" +`; + +exports[`ReactDebugFiberPerf does not include context provider or consumer in measurements 1`] = ` +"⚛ (Waiting for async callback... will force flush in 5230 ms) + +// Mount +⚛ (React Tree Reconciliation: Completed Root) + ⚛ Parent [mount] + ⚛ Child [mount] + +⚛ (Committing Changes) + ⚛ (Committing Snapshot Effects: 0 Total) + ⚛ (Committing Host Effects: 1 Total) + ⚛ (Calling Lifecycle Methods: 0 Total) +" +`; + +exports[`ReactDebugFiberPerf does not include forwardRef component in measurements 1`] = ` +"⚛ (Waiting for async callback... will force flush in 5230 ms) + +// Mount +⚛ (React Tree Reconciliation: Completed Root) + ⚛ Child [mount] + +⚛ (Committing Changes) + ⚛ (Committing Snapshot Effects: 0 Total) + ⚛ (Committing Host Effects: 1 Total) + ⚛ (Calling Lifecycle Methods: 0 Total) +" +`; + exports[`ReactDebugFiberPerf does not schedule an extra callback if setState is called during a synchronous commit phase 1`] = ` "⚛ (React Tree Reconciliation: Completed Root) ⚛ Component [mount] @@ -177,38 +221,6 @@ exports[`ReactDebugFiberPerf measures a simple reconciliation 1`] = ` " `; -exports[`ReactDebugFiberPerf measures and identifies StrictMode and AsyncMode components correctly 1`] = ` -"⚛ (Waiting for async callback... will force flush in 5230 ms) - -// Mount -⚛ (React Tree Reconciliation: Completed Root) - ⚛ StrictMode [mount] - ⚛ Parent [mount] - ⚛ AsyncMode [mount] - ⚛ Child [mount] - -⚛ (Committing Changes) - ⚛ (Committing Snapshot Effects: 0 Total) - ⚛ (Committing Host Effects: 1 Total) - ⚛ (Calling Lifecycle Methods: 0 Total) -" -`; - -exports[`ReactDebugFiberPerf measures and identifies a forwardRef component correctly 1`] = ` -"⚛ (Waiting for async callback... will force flush in 5230 ms) - -// Mount -⚛ (React Tree Reconciliation: Completed Root) - ⚛ ForwardRef [mount] - ⚛ Child [mount] - -⚛ (Committing Changes) - ⚛ (Committing Snapshot Effects: 0 Total) - ⚛ (Committing Host Effects: 1 Total) - ⚛ (Calling Lifecycle Methods: 0 Total) -" -`; - exports[`ReactDebugFiberPerf measures deferred work in chunks 1`] = ` "⚛ (Waiting for async callback... will force flush in 5230 ms) diff --git a/packages/shared/getComponentName.js b/packages/shared/getComponentName.js index 87325d90804f9..e00bd89725162 100644 --- a/packages/shared/getComponentName.js +++ b/packages/shared/getComponentName.js @@ -10,13 +10,10 @@ import type {Fiber} from 'react-reconciler/src/ReactFiber'; import { - REACT_ASYNC_MODE_TYPE, REACT_CALL_TYPE, - REACT_FORWARD_REF_TYPE, REACT_FRAGMENT_TYPE, REACT_RETURN_TYPE, REACT_PORTAL_TYPE, - REACT_STRICT_MODE_TYPE, } from 'shared/ReactSymbols'; function getComponentName(fiber: Fiber): string | null { @@ -28,26 +25,15 @@ function getComponentName(fiber: Fiber): string | null { return type; } switch (type) { - case REACT_ASYNC_MODE_TYPE: - return 'AsyncMode'; - case REACT_CALL_TYPE: - return 'ReactCall'; case REACT_FRAGMENT_TYPE: return 'ReactFragment'; case REACT_PORTAL_TYPE: return 'ReactPortal'; + case REACT_CALL_TYPE: + return 'ReactCall'; case REACT_RETURN_TYPE: return 'ReactReturn'; - case REACT_STRICT_MODE_TYPE: - return 'StrictMode'; - } - if (typeof type === 'object' && type !== null) { - switch (type.$$typeof) { - case REACT_FORWARD_REF_TYPE: - return 'ForwardRef'; - } } - return null; } From fdfc6d190edad3038e21db0efdfa6c6b55e19bb7 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Thu, 12 Apr 2018 09:18:30 -0700 Subject: [PATCH 4/6] Include forwardRef component, but not Async/Strict modes --- .../src/ReactDebugFiberPerf.js | 2 -- .../ReactIncrementalPerf-test.internal.js | 2 +- ...ReactIncrementalPerf-test.internal.js.snap | 29 ++++++++++--------- packages/shared/getComponentName.js | 7 +++++ 4 files changed, 23 insertions(+), 17 deletions(-) diff --git a/packages/react-reconciler/src/ReactDebugFiberPerf.js b/packages/react-reconciler/src/ReactDebugFiberPerf.js index 492e705293c54..0bd2dfb0ad4f9 100644 --- a/packages/react-reconciler/src/ReactDebugFiberPerf.js +++ b/packages/react-reconciler/src/ReactDebugFiberPerf.js @@ -22,7 +22,6 @@ import { ContextProvider, ContextConsumer, Mode, - ForwardRef, } from 'shared/ReactTypeOfWork'; type MeasurementPhase = @@ -178,7 +177,6 @@ const shouldIgnoreFiber = (fiber: Fiber): boolean => { case ContextProvider: case ContextConsumer: case Mode: - case ForwardRef: return true; default: return false; diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalPerf-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalPerf-test.internal.js index 2fd39ce8f2ac4..547c8385875b9 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalPerf-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalPerf-test.internal.js @@ -161,7 +161,7 @@ describe('ReactDebugFiberPerf', () => { expect(getFlameChart()).toMatchSnapshot(); }); - it('does not include forwardRef component in measurements', () => { + it('properly displays the forwardRef component in measurements', () => { const ForwardRef = React.forwardRef(function refForwarder(props, ref) { return ; }); diff --git a/packages/react-reconciler/src/__tests__/__snapshots__/ReactIncrementalPerf-test.internal.js.snap b/packages/react-reconciler/src/__tests__/__snapshots__/ReactIncrementalPerf-test.internal.js.snap index 9259aa8b743c7..2f7a7885aebad 100644 --- a/packages/react-reconciler/src/__tests__/__snapshots__/ReactIncrementalPerf-test.internal.js.snap +++ b/packages/react-reconciler/src/__tests__/__snapshots__/ReactIncrementalPerf-test.internal.js.snap @@ -121,20 +121,6 @@ exports[`ReactDebugFiberPerf does not include context provider or consumer in me " `; -exports[`ReactDebugFiberPerf does not include forwardRef component in measurements 1`] = ` -"⚛ (Waiting for async callback... will force flush in 5230 ms) - -// Mount -⚛ (React Tree Reconciliation: Completed Root) - ⚛ Child [mount] - -⚛ (Committing Changes) - ⚛ (Committing Snapshot Effects: 0 Total) - ⚛ (Committing Host Effects: 1 Total) - ⚛ (Calling Lifecycle Methods: 0 Total) -" -`; - exports[`ReactDebugFiberPerf does not schedule an extra callback if setState is called during a synchronous commit phase 1`] = ` "⚛ (React Tree Reconciliation: Completed Root) ⚛ Component [mount] @@ -275,6 +261,21 @@ exports[`ReactDebugFiberPerf measures deprioritized work 1`] = ` " `; +exports[`ReactDebugFiberPerf properly displays the forwardRef component in measurements 1`] = ` +"⚛ (Waiting for async callback... will force flush in 5230 ms) + +// Mount +⚛ (React Tree Reconciliation: Completed Root) + ⚛ ForwardRef [mount] + ⚛ Child [mount] + +⚛ (Committing Changes) + ⚛ (Committing Snapshot Effects: 0 Total) + ⚛ (Committing Host Effects: 1 Total) + ⚛ (Calling Lifecycle Methods: 0 Total) +" +`; + exports[`ReactDebugFiberPerf recovers from caught errors 1`] = ` "⚛ (Waiting for async callback... will force flush in 5230 ms) diff --git a/packages/shared/getComponentName.js b/packages/shared/getComponentName.js index e00bd89725162..aa1e06a1381aa 100644 --- a/packages/shared/getComponentName.js +++ b/packages/shared/getComponentName.js @@ -14,6 +14,7 @@ import { REACT_FRAGMENT_TYPE, REACT_RETURN_TYPE, REACT_PORTAL_TYPE, + REACT_FORWARD_REF_TYPE, } from 'shared/ReactSymbols'; function getComponentName(fiber: Fiber): string | null { @@ -34,6 +35,12 @@ function getComponentName(fiber: Fiber): string | null { case REACT_RETURN_TYPE: return 'ReactReturn'; } + if (typeof type === 'object' && type !== null) { + switch (type.$$typeof) { + case REACT_FORWARD_REF_TYPE: + return 'ForwardRef'; + } + } return null; } From 0cc677aea85277dfa4e6a0bd489526f4ddfa78f3 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Thu, 12 Apr 2018 09:23:41 -0700 Subject: [PATCH 5/6] Added forwardRef function name support to perf label --- .../ReactIncrementalPerf-test.internal.js | 18 ++++++++++++++++-- .../ReactIncrementalPerf-test.internal.js.snap | 9 +++++++-- packages/shared/getComponentName.js | 6 +++++- 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalPerf-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalPerf-test.internal.js index 547c8385875b9..d5dc2b6f6ab76 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalPerf-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalPerf-test.internal.js @@ -162,11 +162,25 @@ describe('ReactDebugFiberPerf', () => { }); it('properly displays the forwardRef component in measurements', () => { - const ForwardRef = React.forwardRef(function refForwarder(props, ref) { + const AnonymousForwardRef = React.forwardRef((props, ref) => ( + + )); + const NamedForwardRef = React.forwardRef(function refForwarder(props, ref) { return ; }); + function notImportant(props, ref) { + return ; + } + notImportant.displayName = 'OverriddenName'; + const DisplayNamedForwardRef = React.forwardRef(notImportant); - ReactNoop.render(); + ReactNoop.render( + + + + + , + ); addComment('Mount'); ReactNoop.flush(); diff --git a/packages/react-reconciler/src/__tests__/__snapshots__/ReactIncrementalPerf-test.internal.js.snap b/packages/react-reconciler/src/__tests__/__snapshots__/ReactIncrementalPerf-test.internal.js.snap index 2f7a7885aebad..40e37fe439489 100644 --- a/packages/react-reconciler/src/__tests__/__snapshots__/ReactIncrementalPerf-test.internal.js.snap +++ b/packages/react-reconciler/src/__tests__/__snapshots__/ReactIncrementalPerf-test.internal.js.snap @@ -266,8 +266,13 @@ exports[`ReactDebugFiberPerf properly displays the forwardRef component in measu // Mount ⚛ (React Tree Reconciliation: Completed Root) - ⚛ ForwardRef [mount] - ⚛ Child [mount] + ⚛ Parent [mount] + ⚛ ForwardRef [mount] + ⚛ Child [mount] + ⚛ ForwardRef(refForwarder) [mount] + ⚛ Child [mount] + ⚛ ForwardRef(OverriddenName) [mount] + ⚛ Child [mount] ⚛ (Committing Changes) ⚛ (Committing Snapshot Effects: 0 Total) diff --git a/packages/shared/getComponentName.js b/packages/shared/getComponentName.js index aa1e06a1381aa..ef501c02b9267 100644 --- a/packages/shared/getComponentName.js +++ b/packages/shared/getComponentName.js @@ -38,7 +38,11 @@ function getComponentName(fiber: Fiber): string | null { if (typeof type === 'object' && type !== null) { switch (type.$$typeof) { case REACT_FORWARD_REF_TYPE: - return 'ForwardRef'; + const functionName = + type.render.displayName || type.render.name || ''; + return functionName !== '' + ? `ForwardRef(${functionName})` + : 'ForwardRef'; } } return null; From 11685d249e672c75a6e352255700997d817b52fc Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Thu, 12 Apr 2018 09:29:48 -0700 Subject: [PATCH 6/6] Pretier --- packages/shared/getComponentName.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/shared/getComponentName.js b/packages/shared/getComponentName.js index ef501c02b9267..23e10d1a73a97 100644 --- a/packages/shared/getComponentName.js +++ b/packages/shared/getComponentName.js @@ -38,8 +38,7 @@ function getComponentName(fiber: Fiber): string | null { if (typeof type === 'object' && type !== null) { switch (type.$$typeof) { case REACT_FORWARD_REF_TYPE: - const functionName = - type.render.displayName || type.render.name || ''; + const functionName = type.render.displayName || type.render.name || ''; return functionName !== '' ? `ForwardRef(${functionName})` : 'ForwardRef';