From b1f7764c0016d0742bce5b0c6303c59645d48599 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Tue, 29 Mar 2022 11:11:31 -0400 Subject: [PATCH] Profiler should only report stateful hooks that change between renders (#24189) The Profiler has an advanced feature that shows why a component re-rendered. In the case of props and (class) state, it shows the names of props/state values that changed between renders. For hooks, DevTools tries to detect which ones may been related to the update by comparing prev/next internal hook structures. My initial implementation tried to detect all changed hooks. In hindsight this is confusing, because only stateful hooks (e.g. useState, useReducer, and useSyncExternalStore) can schedule an update. (Other types of hooks can change between renders, but in a reactive way.) This PR changes the behavior to only report hooks that scheduled the update. --- .../__snapshots__/profilingCache-test.js.snap | 684 +++++++++--------- .../src/__tests__/profilingCache-test.js | 217 ++++-- .../src/backend/renderer.js | 64 +- 3 files changed, 523 insertions(+), 442 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/__snapshots__/profilingCache-test.js.snap b/packages/react-devtools-shared/src/__tests__/__snapshots__/profilingCache-test.js.snap index 3611f67652aa4..9724472b31cb7 100644 --- a/packages/react-devtools-shared/src/__tests__/__snapshots__/profilingCache-test.js.snap +++ b/packages/react-devtools-shared/src/__tests__/__snapshots__/profilingCache-test.js.snap @@ -4543,245 +4543,6 @@ Object { } `; -exports[`ProfilingCache should properly detect changed hooks: CommitDetails commitIndex: 0 1`] = ` -Object { - "changeDescriptions": Map { - 4 => Object { - "context": null, - "didHooksChange": false, - "isFirstMount": true, - "props": null, - "state": null, - }, - }, - "duration": 0, - "effectDuration": 0, - "fiberActualDurations": Map { - 1 => 0, - 2 => 0, - 3 => 0, - 4 => 0, - }, - "fiberSelfDurations": Map { - 1 => 0, - 2 => 0, - 3 => 0, - 4 => 0, - }, - "passiveEffectDuration": 0, - "priorityLevel": "Immediate", - "timestamp": 0, - "updaters": Array [ - Object { - "displayName": "render()", - "hocDisplayNames": null, - "id": 1, - "key": null, - "type": 11, - }, - ], -} -`; - -exports[`ProfilingCache should properly detect changed hooks: CommitDetails commitIndex: 1 1`] = ` -Object { - "changeDescriptions": Map { - 4 => Object { - "context": false, - "didHooksChange": false, - "hooks": Array [], - "isFirstMount": false, - "props": Array [ - "count", - ], - "state": null, - }, - }, - "duration": 0, - "effectDuration": null, - "fiberActualDurations": Map { - 4 => 0, - 3 => 0, - 2 => 0, - 1 => 0, - }, - "fiberSelfDurations": Map { - 4 => 0, - 3 => 0, - 2 => 0, - 1 => 0, - }, - "passiveEffectDuration": null, - "priorityLevel": "Immediate", - "timestamp": 0, - "updaters": Array [ - Object { - "displayName": "render()", - "hocDisplayNames": null, - "id": 1, - "key": null, - "type": 11, - }, - ], -} -`; - -exports[`ProfilingCache should properly detect changed hooks: CommitDetails commitIndex: 2 1`] = ` -Object { - "changeDescriptions": Map { - 4 => Object { - "context": false, - "didHooksChange": true, - "hooks": Array [ - 1, - ], - "isFirstMount": false, - "props": Array [], - "state": null, - }, - }, - "duration": 0, - "effectDuration": null, - "fiberActualDurations": Map { - 4 => 0, - }, - "fiberSelfDurations": Map { - 4 => 0, - }, - "passiveEffectDuration": null, - "priorityLevel": "Immediate", - "timestamp": 0, - "updaters": Array [ - Object { - "displayName": "Component", - "hocDisplayNames": null, - "id": 4, - "key": null, - "type": 5, - }, - ], -} -`; - -exports[`ProfilingCache should properly detect changed hooks: CommitDetails commitIndex: 3 1`] = ` -Object { - "changeDescriptions": Map { - 4 => Object { - "context": false, - "didHooksChange": true, - "hooks": Array [ - 0, - ], - "isFirstMount": false, - "props": Array [], - "state": null, - }, - }, - "duration": 0, - "effectDuration": null, - "fiberActualDurations": Map { - 4 => 0, - }, - "fiberSelfDurations": Map { - 4 => 0, - }, - "passiveEffectDuration": null, - "priorityLevel": "Immediate", - "timestamp": 0, - "updaters": Array [ - Object { - "displayName": "Component", - "hocDisplayNames": null, - "id": 4, - "key": null, - "type": 5, - }, - ], -} -`; - -exports[`ProfilingCache should properly detect changed hooks: CommitDetails commitIndex: 4 1`] = ` -Object { - "changeDescriptions": Map { - 4 => Object { - "context": true, - "didHooksChange": false, - "hooks": Array [], - "isFirstMount": false, - "props": Array [], - "state": null, - }, - }, - "duration": 0, - "effectDuration": null, - "fiberActualDurations": Map { - 4 => 0, - 3 => 0, - 2 => 0, - 1 => 0, - }, - "fiberSelfDurations": Map { - 4 => 0, - 3 => 0, - 2 => 0, - 1 => 0, - }, - "passiveEffectDuration": null, - "priorityLevel": "Immediate", - "timestamp": 0, - "updaters": Array [ - Object { - "displayName": "render()", - "hocDisplayNames": null, - "id": 1, - "key": null, - "type": 11, - }, - ], -} -`; - -exports[`ProfilingCache should properly detect changed hooks: CommitDetails commitIndex: 5 1`] = ` -Object { - "changeDescriptions": Map { - 4 => Object { - "context": true, - "didHooksChange": false, - "hooks": Array [], - "isFirstMount": false, - "props": Array [], - "state": null, - }, - }, - "duration": 0, - "effectDuration": null, - "fiberActualDurations": Map { - 4 => 0, - 3 => 0, - 2 => 0, - 1 => 0, - }, - "fiberSelfDurations": Map { - 4 => 0, - 3 => 0, - 2 => 0, - 1 => 0, - }, - "passiveEffectDuration": null, - "priorityLevel": "Immediate", - "timestamp": 0, - "updaters": Array [ - Object { - "displayName": "render()", - "hocDisplayNames": null, - "id": 1, - "key": null, - "type": 11, - }, - ], -} -`; - exports[`ProfilingCache should properly detect changed hooks: imported data 1`] = ` Object { "dataForRoots": Array [ @@ -4790,7 +4551,7 @@ Object { Object { "changeDescriptions": Array [ Array [ - 4, + 3, Object { "context": null, "didHooksChange": false, @@ -4815,10 +4576,6 @@ Object { 3, 0, ], - Array [ - 4, - 0, - ], ], "fiberSelfDurations": Array [ Array [ @@ -4833,10 +4590,6 @@ Object { 3, 0, ], - Array [ - 4, - 0, - ], ], "passiveEffectDuration": 0, "priorityLevel": "Immediate", @@ -4854,7 +4607,7 @@ Object { Object { "changeDescriptions": Array [ Array [ - 4, + 3, Object { "context": false, "didHooksChange": false, @@ -4868,12 +4621,8 @@ Object { ], ], "duration": 0, - "effectDuration": null, + "effectDuration": 0, "fiberActualDurations": Array [ - Array [ - 4, - 0, - ], Array [ 3, 0, @@ -4888,10 +4637,6 @@ Object { ], ], "fiberSelfDurations": Array [ - Array [ - 4, - 0, - ], Array [ 3, 0, @@ -4905,7 +4650,7 @@ Object { 0, ], ], - "passiveEffectDuration": null, + "passiveEffectDuration": 0, "priorityLevel": "Immediate", "timestamp": 0, "updaters": Array [ @@ -4921,7 +4666,7 @@ Object { Object { "changeDescriptions": Array [ Array [ - 4, + 3, Object { "context": false, "didHooksChange": true, @@ -4935,27 +4680,27 @@ Object { ], ], "duration": 0, - "effectDuration": null, + "effectDuration": 0, "fiberActualDurations": Array [ Array [ - 4, + 3, 0, ], ], "fiberSelfDurations": Array [ Array [ - 4, + 3, 0, ], ], - "passiveEffectDuration": null, + "passiveEffectDuration": 0, "priorityLevel": "Immediate", "timestamp": 0, "updaters": Array [ Object { "displayName": "Component", "hocDisplayNames": null, - "id": 4, + "id": 3, "key": null, "type": 5, }, @@ -4964,7 +4709,7 @@ Object { Object { "changeDescriptions": Array [ Array [ - 4, + 3, Object { "context": false, "didHooksChange": true, @@ -4978,27 +4723,27 @@ Object { ], ], "duration": 0, - "effectDuration": null, + "effectDuration": 0, "fiberActualDurations": Array [ Array [ - 4, + 3, 0, ], ], "fiberSelfDurations": Array [ Array [ - 4, + 3, 0, ], ], - "passiveEffectDuration": null, + "passiveEffectDuration": 0, "priorityLevel": "Immediate", "timestamp": 0, "updaters": Array [ Object { "displayName": "Component", "hocDisplayNames": null, - "id": 4, + "id": 3, "key": null, "type": 5, }, @@ -5007,7 +4752,7 @@ Object { Object { "changeDescriptions": Array [ Array [ - 4, + 3, Object { "context": true, "didHooksChange": false, @@ -5019,12 +4764,8 @@ Object { ], ], "duration": 0, - "effectDuration": null, + "effectDuration": 0, "fiberActualDurations": Array [ - Array [ - 4, - 0, - ], Array [ 3, 0, @@ -5039,10 +4780,6 @@ Object { ], ], "fiberSelfDurations": Array [ - Array [ - 4, - 0, - ], Array [ 3, 0, @@ -5056,7 +4793,7 @@ Object { 0, ], ], - "passiveEffectDuration": null, + "passiveEffectDuration": 0, "priorityLevel": "Immediate", "timestamp": 0, "updaters": Array [ @@ -5072,11 +4809,13 @@ Object { Object { "changeDescriptions": Array [ Array [ - 4, + 3, Object { - "context": true, - "didHooksChange": false, - "hooks": Array [], + "context": false, + "didHooksChange": true, + "hooks": Array [ + 2, + ], "isFirstMount": false, "props": Array [], "state": null, @@ -5084,53 +4823,29 @@ Object { ], ], "duration": 0, - "effectDuration": null, + "effectDuration": 0, "fiberActualDurations": Array [ - Array [ - 4, - 0, - ], Array [ 3, 0, ], - Array [ - 2, - 0, - ], - Array [ - 1, - 0, - ], ], "fiberSelfDurations": Array [ - Array [ - 4, - 0, - ], Array [ 3, 0, ], - Array [ - 2, - 0, - ], - Array [ - 1, - 0, - ], ], - "passiveEffectDuration": null, + "passiveEffectDuration": 0, "priorityLevel": "Immediate", "timestamp": 0, "updaters": Array [ Object { - "displayName": "render()", + "displayName": "Component", "hocDisplayNames": null, - "id": 1, + "id": 3, "key": null, - "type": 11, + "type": 5, }, ], }, @@ -5188,23 +4903,13 @@ Object { 0, 1, 3, - 2, - 2, - 0, - 1, - 0, - 4, - 3, - 0, - 1, - 4, 5, - 3, + 2, 0, 2, 0, 4, - 4, + 3, 0, ], Array [ @@ -5312,6 +5017,22 @@ Object { "timestamp": 10, "type": "commit", }, + Object { + "batchUID": 2, + "depth": 1, + "duration": 0, + "lanes": "0b0000000000000000000000000000001", + "timestamp": 10, + "type": "layout-effects", + }, + Object { + "batchUID": 2, + "depth": 0, + "duration": 0, + "lanes": "0b0000000000000000000000000000001", + "timestamp": 10, + "type": "passive-effects", + }, ], ], Array [ @@ -5341,6 +5062,22 @@ Object { "timestamp": 10, "type": "commit", }, + Object { + "batchUID": 3, + "depth": 1, + "duration": 0, + "lanes": "0b0000000000000000000000000000001", + "timestamp": 10, + "type": "layout-effects", + }, + Object { + "batchUID": 3, + "depth": 0, + "duration": 0, + "lanes": "0b0000000000000000000000000000001", + "timestamp": 10, + "type": "passive-effects", + }, ], ], Array [ @@ -5370,6 +5107,22 @@ Object { "timestamp": 10, "type": "commit", }, + Object { + "batchUID": 4, + "depth": 1, + "duration": 0, + "lanes": "0b0000000000000000000000000000001", + "timestamp": 10, + "type": "layout-effects", + }, + Object { + "batchUID": 4, + "depth": 0, + "duration": 0, + "lanes": "0b0000000000000000000000000000001", + "timestamp": 10, + "type": "passive-effects", + }, ], ], Array [ @@ -5397,7 +5150,23 @@ Object { "duration": 0, "lanes": "0b0000000000000000000000000000001", "timestamp": 10, - "type": "commit", + "type": "commit", + }, + Object { + "batchUID": 5, + "depth": 1, + "duration": 0, + "lanes": "0b0000000000000000000000000000001", + "timestamp": 10, + "type": "layout-effects", + }, + Object { + "batchUID": 5, + "depth": 0, + "duration": 0, + "lanes": "0b0000000000000000000000000000001", + "timestamp": 10, + "type": "passive-effects", }, ], ], @@ -5428,6 +5197,22 @@ Object { "timestamp": 10, "type": "commit", }, + Object { + "batchUID": 6, + "depth": 1, + "duration": 0, + "lanes": "0b0000000000000000000000000000001", + "timestamp": 10, + "type": "layout-effects", + }, + Object { + "batchUID": 6, + "depth": 0, + "duration": 0, + "lanes": "0b0000000000000000000000000000001", + "timestamp": 10, + "type": "passive-effects", + }, ], ], ], @@ -5446,6 +5231,48 @@ Object { "type": "layout-effect-mount", "warning": null, }, + Object { + "componentName": "Component", + "duration": 0, + "timestamp": 10, + "type": "layout-effect-mount", + "warning": null, + }, + Object { + "componentName": "Component", + "duration": 0, + "timestamp": 10, + "type": "layout-effect-mount", + "warning": null, + }, + Object { + "componentName": "Component", + "duration": 0, + "timestamp": 10, + "type": "passive-effect-mount", + "warning": null, + }, + Object { + "componentName": "Component", + "duration": 0, + "timestamp": 10, + "type": "passive-effect-mount", + "warning": null, + }, + Object { + "componentName": "Component", + "duration": 0, + "timestamp": 10, + "type": "passive-effect-mount", + "warning": null, + }, + Object { + "componentName": "Component", + "duration": 0, + "timestamp": 10, + "type": "passive-effect-mount", + "warning": null, + }, Object { "componentName": "Component", "duration": 0, @@ -5460,6 +5287,34 @@ Object { "type": "render", "warning": null, }, + Object { + "componentName": "Component", + "duration": 0, + "timestamp": 10, + "type": "layout-effect-mount", + "warning": null, + }, + Object { + "componentName": "Component", + "duration": 0, + "timestamp": 10, + "type": "layout-effect-mount", + "warning": null, + }, + Object { + "componentName": "Component", + "duration": 0, + "timestamp": 10, + "type": "passive-effect-mount", + "warning": null, + }, + Object { + "componentName": "Component", + "duration": 0, + "timestamp": 10, + "type": "passive-effect-mount", + "warning": null, + }, Object { "componentName": "Component", "duration": 0, @@ -5467,6 +5322,20 @@ Object { "type": "render", "warning": null, }, + Object { + "componentName": "Component", + "duration": 0, + "timestamp": 10, + "type": "layout-effect-mount", + "warning": null, + }, + Object { + "componentName": "Component", + "duration": 0, + "timestamp": 10, + "type": "passive-effect-mount", + "warning": null, + }, Object { "componentName": "Component", "duration": 0, @@ -5474,6 +5343,20 @@ Object { "type": "render", "warning": null, }, + Object { + "componentName": "Component", + "duration": 0, + "timestamp": 10, + "type": "layout-effect-mount", + "warning": null, + }, + Object { + "componentName": "Component", + "duration": 0, + "timestamp": 10, + "type": "passive-effect-mount", + "warning": null, + }, Object { "componentName": "Component", "duration": 0, @@ -5481,6 +5364,20 @@ Object { "type": "render", "warning": null, }, + Object { + "componentName": "Component", + "duration": 0, + "timestamp": 10, + "type": "layout-effect-mount", + "warning": null, + }, + Object { + "componentName": "Component", + "duration": 0, + "timestamp": 10, + "type": "passive-effect-mount", + "warning": null, + }, Object { "componentName": "Component", "duration": 0, @@ -5488,6 +5385,27 @@ Object { "type": "render", "warning": null, }, + Object { + "componentName": "Component", + "duration": 0, + "timestamp": 10, + "type": "layout-effect-mount", + "warning": null, + }, + Object { + "componentName": "Component", + "duration": 0, + "timestamp": 10, + "type": "passive-effect-mount", + "warning": null, + }, + Object { + "componentName": "Component", + "duration": 0, + "timestamp": 10, + "type": "passive-effect-mount", + "warning": null, + }, ], "duration": 20, "flamechart": Array [], @@ -5686,6 +5604,22 @@ Object { "timestamp": 10, "type": "commit", }, + Object { + "batchUID": 2, + "depth": 1, + "duration": 0, + "lanes": "0b0000000000000000000000000000001", + "timestamp": 10, + "type": "layout-effects", + }, + Object { + "batchUID": 2, + "depth": 0, + "duration": 0, + "lanes": "0b0000000000000000000000000000001", + "timestamp": 10, + "type": "passive-effects", + }, Object { "batchUID": 3, "depth": 0, @@ -5710,6 +5644,22 @@ Object { "timestamp": 10, "type": "commit", }, + Object { + "batchUID": 3, + "depth": 1, + "duration": 0, + "lanes": "0b0000000000000000000000000000001", + "timestamp": 10, + "type": "layout-effects", + }, + Object { + "batchUID": 3, + "depth": 0, + "duration": 0, + "lanes": "0b0000000000000000000000000000001", + "timestamp": 10, + "type": "passive-effects", + }, Object { "batchUID": 4, "depth": 0, @@ -5734,6 +5684,22 @@ Object { "timestamp": 10, "type": "commit", }, + Object { + "batchUID": 4, + "depth": 1, + "duration": 0, + "lanes": "0b0000000000000000000000000000001", + "timestamp": 10, + "type": "layout-effects", + }, + Object { + "batchUID": 4, + "depth": 0, + "duration": 0, + "lanes": "0b0000000000000000000000000000001", + "timestamp": 10, + "type": "passive-effects", + }, Object { "batchUID": 5, "depth": 0, @@ -5758,6 +5724,22 @@ Object { "timestamp": 10, "type": "commit", }, + Object { + "batchUID": 5, + "depth": 1, + "duration": 0, + "lanes": "0b0000000000000000000000000000001", + "timestamp": 10, + "type": "layout-effects", + }, + Object { + "batchUID": 5, + "depth": 0, + "duration": 0, + "lanes": "0b0000000000000000000000000000001", + "timestamp": 10, + "type": "passive-effects", + }, Object { "batchUID": 6, "depth": 0, @@ -5782,6 +5764,22 @@ Object { "timestamp": 10, "type": "commit", }, + Object { + "batchUID": 6, + "depth": 1, + "duration": 0, + "lanes": "0b0000000000000000000000000000001", + "timestamp": 10, + "type": "layout-effects", + }, + Object { + "batchUID": 6, + "depth": 0, + "duration": 0, + "lanes": "0b0000000000000000000000000000001", + "timestamp": 10, + "type": "passive-effects", + }, ], ], Array [ @@ -5942,12 +5940,6 @@ Object { "type": "schedule-render", "warning": null, }, - Object { - "lanes": "0b0000000000000000000000000000001", - "timestamp": 10, - "type": "schedule-render", - "warning": null, - }, ], "snapshotHeight": 0, "snapshots": Array [], @@ -6039,10 +6031,8 @@ Object { "changeDescriptions": Map { 5 => Object { "context": null, - "didHooksChange": true, - "hooks": Array [ - 0, - ], + "didHooksChange": false, + "hooks": Array [], "isFirstMount": false, "props": Array [ "count", @@ -6059,10 +6049,8 @@ Object { }, 7 => Object { "context": null, - "didHooksChange": true, - "hooks": Array [ - 0, - ], + "didHooksChange": false, + "hooks": Array [], "isFirstMount": false, "props": Array [ "count", @@ -6506,10 +6494,8 @@ Object { 5, Object { "context": null, - "didHooksChange": true, - "hooks": Array [ - 0, - ], + "didHooksChange": false, + "hooks": Array [], "isFirstMount": false, "props": Array [ "count", @@ -6532,10 +6518,8 @@ Object { 7, Object { "context": null, - "didHooksChange": true, - "hooks": Array [ - 0, - ], + "didHooksChange": false, + "hooks": Array [], "isFirstMount": false, "props": Array [ "count", diff --git a/packages/react-devtools-shared/src/__tests__/profilingCache-test.js b/packages/react-devtools-shared/src/__tests__/profilingCache-test.js index c1d17fd0f6a3f..b5a5d35da23bd 100644 --- a/packages/react-devtools-shared/src/__tests__/profilingCache-test.js +++ b/packages/react-devtools-shared/src/__tests__/profilingCache-test.js @@ -313,7 +313,6 @@ describe('ProfilingCache', () => { it('should properly detect changed hooks', () => { const Context = React.createContext(0); - const Context2 = React.createContext(0); function reducer(state, action) { switch (action.type) { @@ -324,6 +323,19 @@ describe('ProfilingCache', () => { } } + let snapshot = 0; + function getServerSnapshot() { + return snapshot; + } + function getClientSnapshot() { + return snapshot; + } + + let syncExternalStoreCallback; + function subscribe(callback) { + syncExternalStoreCallback = callback; + } + let dispatch = null; let setState = null; @@ -331,20 +343,31 @@ describe('ProfilingCache', () => { // These hooks may change and initiate re-renders. setState = React.useState('abc')[1]; dispatch = React.useReducer(reducer, {value: true})[1]; + React.useSyncExternalStore( + subscribe, + getClientSnapshot, + getServerSnapshot, + ); // This hook's return value may change between renders, // but the hook itself isn't stateful. React.useContext(Context); - React.useContext(Context2); - // These hooks and their dependencies may not change between renders. - // We're using them to ensure that they don't trigger false positives. + // These hooks never change in a way that schedules an update. React.useCallback(() => () => {}, [string]); React.useMemo(() => string, [string]); + React.useCallback(() => () => {}, [count]); + React.useMemo(() => count, [count]); + React.useCallback(() => () => {}); + React.useMemo(() => string); - // These hooks never "change". + // These hooks never change in a way that schedules an update. React.useEffect(() => {}, [string]); React.useLayoutEffect(() => {}, [string]); + React.useEffect(() => {}, [count]); + React.useLayoutEffect(() => {}, [count]); + React.useEffect(() => {}); + React.useLayoutEffect(() => {}); return null; }; @@ -355,9 +378,7 @@ describe('ProfilingCache', () => { utils.act(() => legacyRender( - - - + , container, ), @@ -367,87 +388,173 @@ describe('ProfilingCache', () => { utils.act(() => legacyRender( - - - + , container, ), ); - // Third render has a changed reducer hook + // Third render has a changed reducer hook. utils.act(() => dispatch({type: 'invert'})); - // Fourth render has a changed state hook + // Fourth render has a changed state hook. utils.act(() => setState('def')); - // Fifth render has a changed context value for context 1, but no changed hook. + // Fifth render has a changed context value, but no changed hook. utils.act(() => legacyRender( - - - + , container, ), ); - // Sixth render has another changed context value for context 2, but no changed hook. - utils.act(() => - legacyRender( - - - - - , - container, - ), - ); + // 6th renderer is triggered by a sync external store change. + utils.act(() => { + snapshot++; + syncExternalStoreCallback(); + }); + utils.act(() => store.profilerStore.stopProfiling()); - const allCommitData = []; + const rootID = store.roots[0]; - function Validator({commitIndex, previousCommitDetails, rootID}) { - const commitData = store.profilerStore.getCommitData(rootID, commitIndex); - if (previousCommitDetails != null) { - expect(commitData).toEqual(previousCommitDetails); - } else { - allCommitData.push(commitData); - expect(commitData).toMatchSnapshot( - `CommitDetails commitIndex: ${commitIndex}`, + const allChangeDescriptions = []; + + function getChangeDescriptions(commitIndex, label) { + let changeDescriptions; + + function Validator() { + const commitData = store.profilerStore.getCommitData( + rootID, + commitIndex, ); - } - return null; - } - const rootID = store.roots[0]; + changeDescriptions = commitData.changeDescriptions; + + allChangeDescriptions.push(changeDescriptions); + + return null; + } - for (let commitIndex = 0; commitIndex < 6; commitIndex++) { utils.act(() => { - TestRenderer.create( - , - ); + TestRenderer.create(); }); + + return changeDescriptions; } - expect(allCommitData).toHaveLength(6); + // 1st render: No change + expect(getChangeDescriptions(0)).toMatchInlineSnapshot(` + Map { + 3 => Object { + "context": null, + "didHooksChange": false, + "isFirstMount": true, + "props": null, + "state": null, + }, + } + `); + + // 2nd render: Changed props + expect(getChangeDescriptions(1)).toMatchInlineSnapshot(` + Map { + 3 => Object { + "context": false, + "didHooksChange": false, + "hooks": Array [], + "isFirstMount": false, + "props": Array [ + "count", + ], + "state": null, + }, + } + `); + + // 3rd render: Changed useReducer + expect(getChangeDescriptions(2)).toMatchInlineSnapshot(` + Map { + 3 => Object { + "context": false, + "didHooksChange": true, + "hooks": Array [ + 1, + ], + "isFirstMount": false, + "props": Array [], + "state": null, + }, + } + `); + + // 4th render: Changed useState + expect(getChangeDescriptions(3)).toMatchInlineSnapshot(` + Map { + 3 => Object { + "context": false, + "didHooksChange": true, + "hooks": Array [ + 0, + ], + "isFirstMount": false, + "props": Array [], + "state": null, + }, + } + `); + + // 5th render: Changed context + expect(getChangeDescriptions(4)).toMatchInlineSnapshot(` + Map { + 3 => Object { + "context": true, + "didHooksChange": false, + "hooks": Array [], + "isFirstMount": false, + "props": Array [], + "state": null, + }, + } + `); + + // 6th render: Sync external store + expect(getChangeDescriptions(5)).toMatchInlineSnapshot(` + Map { + 3 => Object { + "context": false, + "didHooksChange": true, + "hooks": Array [ + 2, + ], + "isFirstMount": false, + "props": Array [], + "state": null, + }, + } + `); + + expect(allChangeDescriptions).toHaveLength(6); // Export and re-import profile data and make sure it is retained. utils.exportImportHelper(bridge, store); + function ExportImportValidator({commitIndex}) { + const commitData = store.profilerStore.getCommitData(rootID, commitIndex); + + expect(commitData.changeDescriptions).toEqual( + allChangeDescriptions[commitIndex], + ); + + return null; + } + for (let commitIndex = 0; commitIndex < 6; commitIndex++) { utils.act(() => { TestRenderer.create( - , + , ); }); } diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js index 3c18ce4083099..50ba03d964caf 100644 --- a/packages/react-devtools-shared/src/backend/renderer.js +++ b/packages/react-devtools-shared/src/backend/renderer.js @@ -93,7 +93,6 @@ import { enableStyleXFeatures, } from 'react-devtools-feature-flags'; import is from 'shared/objectIs'; -import isArray from 'shared/isArray'; import hasOwnProperty from 'shared/hasOwnProperty'; import {getStyleXData} from './StyleX/utils'; import {createProfilingHooks} from './profilingHooks'; @@ -1444,50 +1443,41 @@ export function attach( return null; } - function areHookInputsEqual( - nextDeps: Array, - prevDeps: Array | null, - ) { - if (prevDeps === null) { + function isHookThatCanScheduleUpdate(hookObject: any) { + const queue = hookObject.queue; + if (!queue) { return false; } - for (let i = 0; i < prevDeps.length && i < nextDeps.length; i++) { - if (is(nextDeps[i], prevDeps[i])) { - continue; - } - return false; - } - return true; - } + const boundHasOwnProperty = hasOwnProperty.bind(queue); - function isEffect(memoizedState) { - if (memoizedState === null || typeof memoizedState !== 'object') { - return false; - } - const {deps} = memoizedState; - const boundHasOwnProperty = hasOwnProperty.bind(memoizedState); - return ( - boundHasOwnProperty('create') && - boundHasOwnProperty('destroy') && - boundHasOwnProperty('deps') && - boundHasOwnProperty('next') && - boundHasOwnProperty('tag') && - (deps === null || isArray(deps)) - ); + // Detect the shape of useState() or useReducer() + // using the attributes that are unique to these hooks + // but also stable (e.g. not tied to current Lanes implementation) + const isStateOrReducer = + boundHasOwnProperty('pending') && + boundHasOwnProperty('dispatch') && + typeof queue.dispatch === 'function'; + + // Detect useSyncExternalStore() + const isSyncExternalStore = + boundHasOwnProperty('value') && + boundHasOwnProperty('getSnapshot') && + typeof queue.getSnapshot === 'function'; + + // These are the only types of hooks that can schedule an update. + return isStateOrReducer || isSyncExternalStore; } - function didHookChange(prev: any, next: any): boolean { + function didStatefulHookChange(prev: any, next: any): boolean { const prevMemoizedState = prev.memoizedState; const nextMemoizedState = next.memoizedState; - if (isEffect(prevMemoizedState) && isEffect(nextMemoizedState)) { - return ( - prevMemoizedState !== nextMemoizedState && - !areHookInputsEqual(nextMemoizedState.deps, prevMemoizedState.deps) - ); + if (isHookThatCanScheduleUpdate(prev)) { + return prevMemoizedState !== nextMemoizedState; } - return nextMemoizedState !== prevMemoizedState; + + return false; } function didHooksChange(prev: any, next: any): boolean { @@ -1503,7 +1493,7 @@ export function attach( next.hasOwnProperty('queue') ) { while (next !== null) { - if (didHookChange(prev, next)) { + if (didStatefulHookChange(prev, next)) { return true; } else { next = next.next; @@ -1530,7 +1520,7 @@ export function attach( next.hasOwnProperty('queue') ) { while (next !== null) { - if (didHookChange(prev, next)) { + if (didStatefulHookChange(prev, next)) { indices.push(index); } next = next.next;