diff --git a/src/__tests__/__snapshots__/profilingCache-test.js.snap b/src/__tests__/__snapshots__/profilingCache-test.js.snap index f9901da82a..6d1e6ade29 100644 --- a/src/__tests__/__snapshots__/profilingCache-test.js.snap +++ b/src/__tests__/__snapshots__/profilingCache-test.js.snap @@ -2,6 +2,7 @@ exports[`ProfilingCache should calculate a self duration based on actual children (not filtered children): CommitDetails with filtered self durations 1`] = ` Object { + "changeDescriptions": Map {}, "duration": 16, "fiberActualDurations": Map { 1 => 16, @@ -24,6 +25,7 @@ Object { exports[`ProfilingCache should calculate self duration correctly for suspended views: CommitDetails with filtered self durations 1`] = ` Object { + "changeDescriptions": Map {}, "duration": 15, "fiberActualDurations": Map { 1 => 15, @@ -46,6 +48,7 @@ Object { exports[`ProfilingCache should calculate self duration correctly for suspended views: CommitDetails with filtered self durations 2`] = ` Object { + "changeDescriptions": Map {}, "duration": 3, "fiberActualDurations": Map { 5 => 3, @@ -64,6 +67,7 @@ Object { exports[`ProfilingCache should collect data for each commit: CommitDetails commitIndex: 0 1`] = ` Object { + "changeDescriptions": Map {}, "duration": 12, "fiberActualDurations": Map { 1 => 12, @@ -88,6 +92,25 @@ Object { exports[`ProfilingCache should collect data for each commit: CommitDetails commitIndex: 1 1`] = ` Object { + "changeDescriptions": Map { + 3 => Object { + "didHooksChange": false, + "props": Array [], + "state": Array [], + }, + 4 => Object { + "didHooksChange": false, + "props": Array [], + "state": Array [], + }, + 2 => Object { + "didHooksChange": false, + "props": Array [ + "count", + ], + "state": Array [], + }, + }, "duration": 13, "fiberActualDurations": Map { 3 => 0, @@ -112,6 +135,20 @@ Object { exports[`ProfilingCache should collect data for each commit: CommitDetails commitIndex: 2 1`] = ` Object { + "changeDescriptions": Map { + 3 => Object { + "didHooksChange": false, + "props": Array [], + "state": Array [], + }, + 2 => Object { + "didHooksChange": false, + "props": Array [ + "count", + ], + "state": Array [], + }, + }, "duration": 10, "fiberActualDurations": Map { 3 => 0, @@ -132,6 +169,15 @@ Object { exports[`ProfilingCache should collect data for each commit: CommitDetails commitIndex: 3 1`] = ` Object { + "changeDescriptions": Map { + 2 => Object { + "didHooksChange": false, + "props": Array [ + "count", + ], + "state": Array [], + }, + }, "duration": 10, "fiberActualDurations": Map { 2 => 10, @@ -154,6 +200,7 @@ Object { Object { "commitData": Array [ Object { + "changeDescriptions": Array [], "duration": 12, "fiberActualDurations": Array [ Array [ @@ -205,6 +252,34 @@ Object { "timestamp": 12, }, Object { + "changeDescriptions": Array [ + Array [ + 3, + Object { + "didHooksChange": false, + "props": Array [], + "state": Array [], + }, + ], + Array [ + 4, + Object { + "didHooksChange": false, + "props": Array [], + "state": Array [], + }, + ], + Array [ + 2, + Object { + "didHooksChange": false, + "props": Array [ + "count", + ], + "state": Array [], + }, + ], + ], "duration": 13, "fiberActualDurations": Array [ Array [ @@ -256,6 +331,26 @@ Object { "timestamp": 25, }, Object { + "changeDescriptions": Array [ + Array [ + 3, + Object { + "didHooksChange": false, + "props": Array [], + "state": Array [], + }, + ], + Array [ + 2, + Object { + "didHooksChange": false, + "props": Array [ + "count", + ], + "state": Array [], + }, + ], + ], "duration": 10, "fiberActualDurations": Array [ Array [ @@ -291,6 +386,18 @@ Object { "timestamp": 35, }, Object { + "changeDescriptions": Array [ + Array [ + 2, + Object { + "didHooksChange": false, + "props": Array [ + "count", + ], + "state": Array [], + }, + ], + ], "duration": 10, "fiberActualDurations": Array [ Array [ @@ -507,6 +614,7 @@ Object { Object { "commitData": Array [ Object { + "changeDescriptions": Array [], "duration": 11, "fiberActualDurations": Array [ Array [ @@ -550,6 +658,26 @@ Object { "timestamp": 11, }, Object { + "changeDescriptions": Array [ + Array [ + 3, + Object { + "didHooksChange": false, + "props": Array [], + "state": Array [], + }, + ], + Array [ + 2, + Object { + "didHooksChange": false, + "props": Array [ + "count", + ], + "state": Array [], + }, + ], + ], "duration": 11, "fiberActualDurations": Array [ Array [ @@ -593,6 +721,34 @@ Object { "timestamp": 22, }, Object { + "changeDescriptions": Array [ + Array [ + 3, + Object { + "didHooksChange": false, + "props": Array [], + "state": Array [], + }, + ], + Array [ + 5, + Object { + "didHooksChange": false, + "props": Array [], + "state": Array [], + }, + ], + Array [ + 2, + Object { + "didHooksChange": false, + "props": Array [ + "count", + ], + "state": Array [], + }, + ], + ], "duration": 13, "fiberActualDurations": Array [ Array [ @@ -791,6 +947,25 @@ exports[`ProfilingCache should collect data for each root (including ones added Object { "commitData": Array [ Object { + "changeDescriptions": Map { + 3 => Object { + "didHooksChange": false, + "props": Array [], + "state": Array [], + }, + 4 => Object { + "didHooksChange": false, + "props": Array [], + "state": Array [], + }, + 2 => Object { + "didHooksChange": false, + "props": Array [ + "count", + ], + "state": Array [], + }, + }, "duration": 13, "fiberActualDurations": Map { 3 => 0, @@ -812,6 +987,20 @@ Object { "timestamp": 13, }, Object { + "changeDescriptions": Map { + 3 => Object { + "didHooksChange": false, + "props": Array [], + "state": Array [], + }, + 2 => Object { + "didHooksChange": false, + "props": Array [ + "count", + ], + "state": Array [], + }, + }, "duration": 10, "fiberActualDurations": Map { 3 => 0, @@ -829,6 +1018,15 @@ Object { "timestamp": 34, }, Object { + "changeDescriptions": Map { + 2 => Object { + "didHooksChange": false, + "props": Array [ + "count", + ], + "state": Array [], + }, + }, "duration": 10, "fiberActualDurations": Map { 2 => 10, @@ -971,6 +1169,7 @@ exports[`ProfilingCache should collect data for each root (including ones added Object { "commitData": Array [ Object { + "changeDescriptions": Map {}, "duration": 11, "fiberActualDurations": Map { 11 => 11, @@ -1063,6 +1262,7 @@ exports[`ProfilingCache should collect data for each root (including ones added Object { "commitData": Array [ Object { + "changeDescriptions": Map {}, "duration": 0, "fiberActualDurations": Map {}, "fiberSelfDurations": Map {}, @@ -1139,6 +1339,34 @@ Object { Object { "commitData": Array [ Object { + "changeDescriptions": Array [ + Array [ + 3, + Object { + "didHooksChange": false, + "props": Array [], + "state": Array [], + }, + ], + Array [ + 4, + Object { + "didHooksChange": false, + "props": Array [], + "state": Array [], + }, + ], + Array [ + 2, + Object { + "didHooksChange": false, + "props": Array [ + "count", + ], + "state": Array [], + }, + ], + ], "duration": 13, "fiberActualDurations": Array [ Array [ @@ -1190,6 +1418,26 @@ Object { "timestamp": 13, }, Object { + "changeDescriptions": Array [ + Array [ + 3, + Object { + "didHooksChange": false, + "props": Array [], + "state": Array [], + }, + ], + Array [ + 2, + Object { + "didHooksChange": false, + "props": Array [ + "count", + ], + "state": Array [], + }, + ], + ], "duration": 10, "fiberActualDurations": Array [ Array [ @@ -1225,6 +1473,18 @@ Object { "timestamp": 34, }, Object { + "changeDescriptions": Array [ + Array [ + 2, + Object { + "didHooksChange": false, + "props": Array [ + "count", + ], + "state": Array [], + }, + ], + ], "duration": 10, "fiberActualDurations": Array [ Array [ @@ -1406,6 +1666,7 @@ Object { Object { "commitData": Array [ Object { + "changeDescriptions": Array [], "duration": 11, "fiberActualDurations": Array [ Array [ @@ -1519,6 +1780,7 @@ Object { Object { "commitData": Array [ Object { + "changeDescriptions": Array [], "duration": 0, "fiberActualDurations": Array [], "fiberSelfDurations": Array [], @@ -1639,6 +1901,7 @@ Object { Object { "commitData": Array [ Object { + "changeDescriptions": Array [], "duration": 11, "fiberActualDurations": Array [ Array [ @@ -1684,6 +1947,26 @@ Object { "timestamp": 11, }, Object { + "changeDescriptions": Array [ + Array [ + 3, + Object { + "didHooksChange": false, + "props": Array [], + "state": Array [], + }, + ], + Array [ + 2, + Object { + "didHooksChange": false, + "props": Array [ + "count", + ], + "state": Array [], + }, + ], + ], "duration": 11, "fiberActualDurations": Array [ Array [ diff --git a/src/__tests__/profilerContext-test.js b/src/__tests__/profilerContext-test.js index 3d3a68f591..6857ddb407 100644 --- a/src/__tests__/profilerContext-test.js +++ b/src/__tests__/profilerContext-test.js @@ -29,6 +29,7 @@ describe('ProfilerContext', () => { bridge = global.bridge; store = global.store; store.collapseNodesByDefault = false; + store.recordChangeDescriptions = true; React = require('react'); ReactDOM = require('react-dom'); diff --git a/src/__tests__/profilerStore-test.js b/src/__tests__/profilerStore-test.js index f6ffcfe783..0f4074f165 100644 --- a/src/__tests__/profilerStore-test.js +++ b/src/__tests__/profilerStore-test.js @@ -14,6 +14,7 @@ describe('ProfilerStore', () => { store = global.store; store.collapseNodesByDefault = false; + store.recordChangeDescriptions = true; React = require('react'); ReactDOM = require('react-dom'); diff --git a/src/__tests__/profilingCache-test.js b/src/__tests__/profilingCache-test.js index c93d347ea8..83c1c056d1 100644 --- a/src/__tests__/profilingCache-test.js +++ b/src/__tests__/profilingCache-test.js @@ -21,6 +21,7 @@ describe('ProfilingCache', () => { bridge = global.bridge; store = global.store; store.collapseNodesByDefault = false; + store.recordChangeDescriptions = true; React = require('react'); ReactDOM = require('react-dom'); diff --git a/src/__tests__/profilingCharts-test.js b/src/__tests__/profilingCharts-test.js index d83f8b5a0f..9cb6d25d6f 100644 --- a/src/__tests__/profilingCharts-test.js +++ b/src/__tests__/profilingCharts-test.js @@ -18,6 +18,7 @@ describe('profiling charts', () => { store = global.store; store.collapseNodesByDefault = false; + store.recordChangeDescriptions = true; React = require('react'); ReactDOM = require('react-dom'); diff --git a/src/__tests__/profilingCommitTreeBuilder-test.js b/src/__tests__/profilingCommitTreeBuilder-test.js index e6c98c1d5e..68d42961ea 100644 --- a/src/__tests__/profilingCommitTreeBuilder-test.js +++ b/src/__tests__/profilingCommitTreeBuilder-test.js @@ -17,6 +17,7 @@ describe('commit tree', () => { store = global.store; store.collapseNodesByDefault = false; + store.recordChangeDescriptions = true; React = require('react'); ReactDOM = require('react-dom'); diff --git a/src/__tests__/storeComponentFilters-test.js b/src/__tests__/storeComponentFilters-test.js index 6627912b8d..16b028abcb 100644 --- a/src/__tests__/storeComponentFilters-test.js +++ b/src/__tests__/storeComponentFilters-test.js @@ -21,6 +21,7 @@ describe('Store component filters', () => { store = global.store; store.collapseNodesByDefault = false; store.componentFilters = []; + store.recordChangeDescriptions = true; React = require('react'); ReactDOM = require('react-dom'); diff --git a/src/backend/agent.js b/src/backend/agent.js index 4c1bf1b4c9..12ba9a8a14 100644 --- a/src/backend/agent.js +++ b/src/backend/agent.js @@ -6,6 +6,7 @@ import throttle from 'lodash.throttle'; import { SESSION_STORAGE_LAST_SELECTION_KEY, SESSION_STORAGE_RELOAD_AND_PROFILE_KEY, + SESSION_STORAGE_RECORD_CHANGE_DESCRIPTIONS_KEY, __DEBUG__, } from '../constants'; import { @@ -69,6 +70,7 @@ type PersistedSelection = {| export default class Agent extends EventEmitter { _bridge: Bridge; _isProfiling: boolean = false; + _recordChangeDescriptions: boolean = false; _rendererInterfaces: { [key: RendererID]: RendererInterface } = {}; _persistedSelection: PersistedSelection | null = null; _persistedSelectionMatch: PathMatch | null = null; @@ -79,8 +81,13 @@ export default class Agent extends EventEmitter { if ( sessionStorageGetItem(SESSION_STORAGE_RELOAD_AND_PROFILE_KEY) === 'true' ) { + this._recordChangeDescriptions = + sessionStorageGetItem( + SESSION_STORAGE_RECORD_CHANGE_DESCRIPTIONS_KEY + ) === 'true'; this._isProfiling = true; + sessionStorageRemoveItem(SESSION_STORAGE_RECORD_CHANGE_DESCRIPTIONS_KEY); sessionStorageRemoveItem(SESSION_STORAGE_RELOAD_AND_PROFILE_KEY); } @@ -250,8 +257,12 @@ export default class Agent extends EventEmitter { } }; - reloadAndProfile = () => { + reloadAndProfile = (recordChangeDescriptions: boolean) => { sessionStorageSetItem(SESSION_STORAGE_RELOAD_AND_PROFILE_KEY, 'true'); + sessionStorageSetItem( + SESSION_STORAGE_RECORD_CHANGE_DESCRIPTIONS_KEY, + recordChangeDescriptions ? 'true' : 'false' + ); // This code path should only be hit if the shell has explicitly told the Store that it supports profiling. // In that case, the shell must also listen for this specific message to know when it needs to reload the app. @@ -358,7 +369,7 @@ export default class Agent extends EventEmitter { this._rendererInterfaces[rendererID] = rendererInterface; if (this._isProfiling) { - rendererInterface.startProfiling(); + rendererInterface.startProfiling(this._recordChangeDescriptions); } // When the renderer is attached, we need to tell it whether @@ -397,13 +408,14 @@ export default class Agent extends EventEmitter { window.addEventListener('pointerup', this._onPointerUp, true); }; - startProfiling = () => { + startProfiling = (recordChangeDescriptions: boolean) => { + this._recordChangeDescriptions = recordChangeDescriptions; this._isProfiling = true; for (let rendererID in this._rendererInterfaces) { const renderer = ((this._rendererInterfaces[ (rendererID: any) ]: any): RendererInterface); - renderer.startProfiling(); + renderer.startProfiling(recordChangeDescriptions); } this._bridge.send('profilingStatus', this._isProfiling); }; @@ -422,6 +434,7 @@ export default class Agent extends EventEmitter { stopProfiling = () => { this._isProfiling = false; + this._recordChangeDescriptions = false; for (let rendererID in this._rendererInterfaces) { const renderer = ((this._rendererInterfaces[ (rendererID: any) diff --git a/src/backend/renderer.js b/src/backend/renderer.js index df28dd45fb..6f2bea1d20 100644 --- a/src/backend/renderer.js +++ b/src/backend/renderer.js @@ -30,6 +30,7 @@ import { cleanForBridge, copyWithSet, setInObject } from './utils'; import { __DEBUG__, SESSION_STORAGE_RELOAD_AND_PROFILE_KEY, + SESSION_STORAGE_RECORD_CHANGE_DESCRIPTIONS_KEY, TREE_OPERATION_ADD, TREE_OPERATION_REMOVE, TREE_OPERATION_REORDER_CHILDREN, @@ -38,6 +39,7 @@ import { import { inspectHooksOfFiber } from './ReactDebugHooks'; import type { + ChangeDescription, CommitDataBackend, DevToolsHook, Fiber, @@ -350,7 +352,7 @@ export function attach( // The unmount operations are already significantly smaller than mount opreations though. // This is something to keep in mind for later. function updateComponentFilters(componentFilters: Array) { - if (this._isProfiling) { + if (isProfiling) { // Re-mounting a tree while profiling is in progress might break a lot of assumptions. // If necessary, we could support this- but it doesn't seem like a necessary use case. throw Error('Cannot modify filter preferences while profiling'); @@ -646,8 +648,87 @@ export function attach( return ((fiberToIDMap.get(primaryFiber): any): number); } + function getChangeDescription( + prevFiber: Fiber, + nextFiber: Fiber + ): ChangeDescription | null { + switch (getElementTypeForFiber(nextFiber)) { + case ElementTypeClass: + case ElementTypeFunction: + case ElementTypeMemo: + case ElementTypeForwardRef: + return { + didHooksChange: didHooksChange( + prevFiber.memoizedState, + nextFiber.memoizedState + ), + props: getChangedKeys( + prevFiber.memoizedProps, + nextFiber.memoizedProps + ), + state: getChangedKeys( + prevFiber.memoizedState, + nextFiber.memoizedState + ), + }; + default: + return null; + } + } + + function didHooksChange(prev: any, next: any): boolean { + if (next == null) { + return false; + } + + // We can't report anything meaningful for hooks changes. + if ( + next.hasOwnProperty('baseState') && + next.hasOwnProperty('memoizedState') && + next.hasOwnProperty('next') && + next.hasOwnProperty('queue') + ) { + while (next !== null) { + if (next.memoizedState !== prev.memoizedState) { + return true; + } else { + next = next.next; + prev = prev.next; + } + } + } + + return false; + } + + function getChangedKeys(prev: any, next: any): Array { + const keys = []; + + if (next == null) { + return keys; + } + + // We can't report anything meaningful for hooks changes. + if ( + next.hasOwnProperty('baseState') && + next.hasOwnProperty('memoizedState') && + next.hasOwnProperty('next') && + next.hasOwnProperty('queue') + ) { + return keys; + } + + // TODO (change descriptions) This does not account for props that were added or removed. + for (let key in prev) { + if (prev[key] !== next[key]) { + keys.push(key); + } + } + return keys; + } + // eslint-disable-next-line no-unused-vars - function hasDataChanged(prevFiber: Fiber, nextFiber: Fiber): boolean { + function didFiberRender(prevFiber: Fiber, nextFiber: Fiber): boolean { switch (nextFiber.tag) { case ClassComponent: case FunctionComponent: @@ -1024,7 +1105,7 @@ export function attach( pushOperation(treeBaseDuration); } - if (alternate == null || hasDataChanged(alternate, fiber)) { + if (alternate == null || didFiberRender(alternate, fiber)) { if (actualDuration != null) { // The actual duration reported by React includes time spent working on children. // This is useful information, but it's also useful to be able to exclude child durations. @@ -1049,6 +1130,16 @@ export function attach( metadata.maxActualDuration, actualDuration ); + + if (recordChangeDescriptions && metadata.changeDescriptions) { + const changeDescription = + alternate === null + ? null + : getChangeDescription(alternate, fiber); + if (changeDescription !== null) { + metadata.changeDescriptions.set(id, changeDescription); + } + } } } } @@ -1110,7 +1201,7 @@ export function attach( mostRecentlyInspectedElementID !== null && mostRecentlyInspectedElementID === getFiberID(getPrimaryFiber(nextFiber)) && - hasDataChanged(prevFiber, nextFiber) + didFiberRender(prevFiber, nextFiber) ) { // If this Fiber has updated, clear cached inspected data. // If it is inspected again, it may need to be re-run to obtain updated hooks values. @@ -1300,6 +1391,7 @@ export function attach( // If profiling is active, store commit time and duration, and the current interactions. // The frontend may request this information after profiling has stopped. currentCommitProfilingMetadata = { + changeDescriptions: recordChangeDescriptions ? new Map() : null, durations: [], commitTime: performance.now() - profilingStartTime, interactions: Array.from(root.memoizedInteractions).map( @@ -1343,6 +1435,7 @@ export function attach( // If profiling is active, store commit time and duration, and the current interactions. // The frontend may request this information after profiling has stopped. currentCommitProfilingMetadata = { + changeDescriptions: recordChangeDescriptions ? new Map() : null, durations: [], commitTime: performance.now() - profilingStartTime, interactions: Array.from(root.memoizedInteractions).map( @@ -2058,6 +2151,7 @@ export function attach( } type CommitProfilingData = {| + changeDescriptions: Map | null, commitTime: number, durations: Array, interactions: Array, @@ -2074,6 +2168,7 @@ export function attach( let initialIDToRootMap: Map | null = null; let isProfiling: boolean = false; let profilingStartTime: number = 0; + let recordChangeDescriptions: boolean = false; let rootToCommitProfilingMetadataMap: CommitProfilingMetadataMap | null = null; function getProfilingData(): ProfilingDataBackend { @@ -2111,6 +2206,7 @@ export function attach( commitProfilingMetadata.forEach((commitProfilingData, commitIndex) => { const { + changeDescriptions, durations, interactions, maxActualDuration, @@ -2144,6 +2240,10 @@ export function attach( } commitData.push({ + changeDescriptions: + changeDescriptions !== null + ? Array.from(changeDescriptions.entries()) + : null, duration: maxActualDuration, fiberActualDurations, fiberSelfDurations, @@ -2170,11 +2270,13 @@ export function attach( }; } - function startProfiling() { + function startProfiling(shouldRecordChangeDescriptions: boolean) { if (isProfiling) { return; } + recordChangeDescriptions = shouldRecordChangeDescriptions; + // Capture initial values as of the time profiling starts. // It's important we snapshot both the durations and the id-to-root map, // since either of these may change during the profiling session @@ -2198,13 +2300,17 @@ export function attach( function stopProfiling() { isProfiling = false; + recordChangeDescriptions = false; } // Automatically start profiling so that we don't miss timing info from initial "mount". if ( sessionStorageGetItem(SESSION_STORAGE_RELOAD_AND_PROFILE_KEY) === 'true' ) { - startProfiling(); + startProfiling( + sessionStorageGetItem(SESSION_STORAGE_RECORD_CHANGE_DESCRIPTIONS_KEY) === + 'true' + ); } // React will switch between these implementations depending on whether diff --git a/src/backend/types.js b/src/backend/types.js index bce7238efd..220f1358ec 100644 --- a/src/backend/types.js +++ b/src/backend/types.js @@ -112,7 +112,16 @@ export type ReactRenderer = { currentDispatcherRef?: {| current: null | Dispatcher |}, }; +// TODO (change descriptions) Is it important to handle context? +export type ChangeDescription = {| + didHooksChange: boolean, + props: Array, + state: Array, +|}; + export type CommitDataBackend = {| + // Tuple of fiber ID and change description + changeDescriptions: Array<[number, ChangeDescription]> | null, duration: number, // Tuple of fiber ID and actual duration fiberActualDurations: Array<[number, number]>, @@ -226,7 +235,7 @@ export type RendererInterface = { setInProps: (id: number, path: Array, value: any) => void, setInState: (id: number, path: Array, value: any) => void, setTrackedPath: (path: Array | null) => void, - startProfiling: () => void, + startProfiling: (recordChangeDescriptions: boolean) => void, stopProfiling: () => void, updateComponentFilters: (somponentFilters: Array) => void, }; diff --git a/src/constants.js b/src/constants.js index b06ebcfa22..9267c53e4b 100644 --- a/src/constants.js +++ b/src/constants.js @@ -14,6 +14,9 @@ export const LOCAL_STORAGE_FILTER_PREFERENCES_KEY = export const SESSION_STORAGE_LAST_SELECTION_KEY = 'React::DevTools::lastSelection'; +export const SESSION_STORAGE_RECORD_CHANGE_DESCRIPTIONS_KEY = + 'React::DevTools::recordChangeDescriptions'; + export const SESSION_STORAGE_RELOAD_AND_PROFILE_KEY = 'React::DevTools::reloadAndProfile'; diff --git a/src/devtools/ProfilerStore.js b/src/devtools/ProfilerStore.js index f4901287ea..b15726c542 100644 --- a/src/devtools/ProfilerStore.js +++ b/src/devtools/ProfilerStore.js @@ -179,7 +179,7 @@ export default class ProfilerStore extends EventEmitter { } startProfiling(): void { - this._bridge.send('startProfiling'); + this._bridge.send('startProfiling', this._store.recordChangeDescriptions); // Don't actually update the local profiling boolean yet! // Wait for onProfilingStatus() to confirm the status has changed. diff --git a/src/devtools/store.js b/src/devtools/store.js index 6ad34d7e7b..d4501273a8 100644 --- a/src/devtools/store.js +++ b/src/devtools/store.js @@ -38,6 +38,8 @@ const LOCAL_STORAGE_CAPTURE_SCREENSHOTS_KEY = 'React::DevTools::captureScreenshots'; const LOCAL_STORAGE_COLLAPSE_ROOTS_BY_DEFAULT_KEY = 'React::DevTools::collapseNodesByDefault'; +const LOCAL_STORAGE_RECORD_CHANGE_DESCRIPTIONS_KEY = + 'React::DevTools::recordChangeDescriptions'; type Config = {| isProfiling?: boolean, @@ -83,6 +85,8 @@ export default class Store extends EventEmitter { _profilerStore: ProfilerStore; + _recordChangeDescriptions: boolean = false; + // Incremented each time the store is mutated. // This enables a passive effect to detect a mutation between render and commit phase. _revision: number = 0; @@ -118,6 +122,10 @@ export default class Store extends EventEmitter { localStorageGetItem(LOCAL_STORAGE_COLLAPSE_ROOTS_BY_DEFAULT_KEY) !== 'false'; + this._recordChangeDescriptions = + localStorageGetItem(LOCAL_STORAGE_RECORD_CHANGE_DESCRIPTIONS_KEY) === + 'true'; + this._componentFilters = getSavedComponentFilters(); let isProfiling = false; @@ -248,6 +256,20 @@ export default class Store extends EventEmitter { return this._profilerStore; } + get recordChangeDescriptions(): boolean { + return this._recordChangeDescriptions; + } + set recordChangeDescriptions(value: boolean): void { + this._recordChangeDescriptions = value; + + localStorageSetItem( + LOCAL_STORAGE_RECORD_CHANGE_DESCRIPTIONS_KEY, + value ? 'true' : 'false' + ); + + this.emit('recordChangeDescriptions'); + } + get revision(): number { return this._revision; } diff --git a/src/devtools/views/Profiler/ReloadAndProfileButton.js b/src/devtools/views/Profiler/ReloadAndProfileButton.js index 110a1b61a4..933d453ab0 100644 --- a/src/devtools/views/Profiler/ReloadAndProfileButton.js +++ b/src/devtools/views/Profiler/ReloadAndProfileButton.js @@ -7,27 +7,41 @@ import { BridgeContext, StoreContext } from '../context'; import { useSubscription } from '../hooks'; import Store from 'src/devtools/store'; +type SubscriptionData = {| + recordChangeDescriptions: boolean, + supportsReloadAndProfile: boolean, +|}; + export default function ReloadAndProfileButton() { const bridge = useContext(BridgeContext); const store = useContext(StoreContext); - const supportsReloadAndProfileSubscription = useMemo( + const subscription = useMemo( () => ({ - getCurrentValue: () => store.supportsReloadAndProfile, + getCurrentValue: () => ({ + recordChangeDescriptions: store.recordChangeDescriptions, + supportsReloadAndProfile: store.supportsReloadAndProfile, + }), subscribe: (callback: Function) => { + store.addListener('recordChangeDescriptions', callback); store.addListener('supportsReloadAndProfile', callback); - return () => store.removeListener('supportsReloadAndProfile', callback); + return () => { + store.removeListener('recordChangeDescriptions', callback); + store.removeListener('supportsReloadAndProfile', callback); + }; }, }), [store] ); - const supportsReloadAndProfile = useSubscription( - supportsReloadAndProfileSubscription - ); + const { + recordChangeDescriptions, + supportsReloadAndProfile, + } = useSubscription(subscription); - const reloadAndProfile = useCallback(() => bridge.send('reloadAndProfile'), [ - bridge, - ]); + const reloadAndProfile = useCallback( + () => bridge.send('reloadAndProfile', recordChangeDescriptions), + [bridge, recordChangeDescriptions] + ); if (!supportsReloadAndProfile) { return null; diff --git a/src/devtools/views/Profiler/SidebarSelectedFiberInfo.css b/src/devtools/views/Profiler/SidebarSelectedFiberInfo.css index 30097488c7..d19f2b1743 100644 --- a/src/devtools/views/Profiler/SidebarSelectedFiberInfo.css +++ b/src/devtools/views/Profiler/SidebarSelectedFiberInfo.css @@ -56,3 +56,22 @@ .CurrentCommit:focus { outline: none; } + +.WhatChangedItem { + margin-top: 0.25rem; +} + +.WhatChangedKey { + font-family: var(--font-family-monospace); + font-size: var(--font-size-monospace-small); + line-height: 1; +} +.WhatChangedKey:first-of-type::before { + content: ' ('; +} +.WhatChangedKey::after { + content: ', '; +} +.WhatChangedKey:last-of-type::after { + content: ')'; +} diff --git a/src/devtools/views/Profiler/SidebarSelectedFiberInfo.js b/src/devtools/views/Profiler/SidebarSelectedFiberInfo.js index c83b0bc00e..2b3c1b793a 100644 --- a/src/devtools/views/Profiler/SidebarSelectedFiberInfo.js +++ b/src/devtools/views/Profiler/SidebarSelectedFiberInfo.js @@ -1,6 +1,7 @@ // @flow import React, { Fragment, useContext } from 'react'; +import ProfilerStore from 'src/devtools/ProfilerStore'; import { ProfilerContext } from './ProfilerContext'; import { formatDuration, formatTime } from './utils'; import { StoreContext } from '../context'; @@ -67,9 +68,86 @@ export default function SidebarSelectedFiberInfo(_: Props) { +
: {listItems}
); } + +type WhatChangedProps = {| + commitIndex: number, + fiberID: number, + profilerStore: ProfilerStore, + rootID: number, +|}; + +function WhatChanged({ + commitIndex, + fiberID, + profilerStore, + rootID, +}: WhatChangedProps) { + const { changeDescriptions } = profilerStore.getCommitData( + ((rootID: any): number), + commitIndex + ); + if (changeDescriptions === null) { + return null; + } + + const changeDescription = changeDescriptions.get(fiberID); + if (changeDescription == null) { + return null; + } + + const changes = []; + if (changeDescription.didHooksChange) { + changes.push( +
+ • Hooks +
+ ); + } + if (changeDescription.props.length !== 0) { + changes.push( +
+ • Props + {changeDescription.props.map(key => ( + + {key} + + ))} +
+ ); + } + if (changeDescription.state.length !== 0) { + changes.push( +
+ • State + {changeDescription.state.map(key => ( + + {key} + + ))} +
+ ); + } + + if (changes.length === 0) { + changes.push(
Nothing
); + } + + return ( +
+ + {changes} +
+ ); +} diff --git a/src/devtools/views/Profiler/types.js b/src/devtools/views/Profiler/types.js index 9af30779be..667ab54ce3 100644 --- a/src/devtools/views/Profiler/types.js +++ b/src/devtools/views/Profiler/types.js @@ -31,7 +31,17 @@ export type SnapshotNode = {| type: ElementType, |}; +// TODO (change descriptions) Is it important to handle context? +export type ChangeDescription = {| + didHooksChange: boolean, + props: Array, + state: Array, +|}; + export type CommitDataFrontend = {| + // Map of Fiber (ID) to a description of what changed in this commit. + changeDescriptions: Map | null, + // How long was this commit? duration: number, @@ -93,6 +103,7 @@ export type ProfilingDataFrontend = {| |}; export type CommitDataExport = {| + changeDescriptions: Array<[number, ChangeDescription]> | null, duration: number, // Tuple of fiber ID and actual duration fiberActualDurations: Array<[number, number]>, diff --git a/src/devtools/views/Profiler/utils.js b/src/devtools/views/Profiler/utils.js index f192520f4f..bd4d1e9f31 100644 --- a/src/devtools/views/Profiler/utils.js +++ b/src/devtools/views/Profiler/utils.js @@ -58,6 +58,10 @@ export function prepareProfilingDataFrontendFromBackendAndStore( dataForRoots.set(rootID, { commitData: commitData.map((commitDataBackend, commitIndex) => ({ + changeDescriptions: + commitDataBackend.changeDescriptions != null + ? new Map(commitDataBackend.changeDescriptions) + : null, duration: commitDataBackend.duration, fiberActualDurations: new Map( commitDataBackend.fiberActualDurations @@ -109,6 +113,7 @@ export function prepareProfilingDataFrontendFromExport( dataForRoots.set(rootID, { commitData: commitData.map( ({ + changeDescriptions, duration, fiberActualDurations, fiberSelfDurations, @@ -117,6 +122,8 @@ export function prepareProfilingDataFrontendFromExport( screenshot, timestamp, }) => ({ + changeDescriptions: + changeDescriptions != null ? new Map(changeDescriptions) : null, duration, fiberActualDurations: new Map(fiberActualDurations), fiberSelfDurations: new Map(fiberSelfDurations), @@ -159,6 +166,7 @@ export function prepareProfilingDataExport( dataForRoots.push({ commitData: commitData.map( ({ + changeDescriptions, duration, fiberActualDurations, fiberSelfDurations, @@ -167,6 +175,10 @@ export function prepareProfilingDataExport( screenshot, timestamp, }) => ({ + changeDescriptions: + changeDescriptions != null + ? Array.from(changeDescriptions.entries()) + : null, duration, fiberActualDurations: Array.from(fiberActualDurations.entries()), fiberSelfDurations: Array.from(fiberSelfDurations.entries()), diff --git a/src/devtools/views/Settings/Settings.js b/src/devtools/views/Settings/Settings.js index 645ee25422..b53dee04e2 100644 --- a/src/devtools/views/Settings/Settings.js +++ b/src/devtools/views/Settings/Settings.js @@ -1,6 +1,6 @@ // @flow -import React, { useCallback, useContext, useMemo } from 'react'; +import React, { Fragment, useCallback, useContext, useMemo } from 'react'; import { useSubscription } from '../hooks'; import { StoreContext } from '../context'; import { SettingsContext } from './SettingsContext'; @@ -43,6 +43,20 @@ function Settings(_: {||}) { collapseNodesByDefaultSubscription ); + const recordChangeDescriptionsSubscription = useMemo( + () => ({ + getCurrentValue: () => store.recordChangeDescriptions, + subscribe: (callback: Function) => { + store.addListener('recordChangeDescriptions', callback); + return () => store.removeListener('recordChangeDescriptions', callback); + }, + }), + [store] + ); + const recordChangeDescriptions = useSubscription( + recordChangeDescriptionsSubscription + ); + const updateDisplayDensity = useCallback( ({ currentTarget }) => { setDisplayDensity(currentTarget.value); @@ -69,6 +83,12 @@ function Settings(_: {||}) { }, [store] ); + const updateRecordChangeDescriptions = useCallback( + ({ currentTarget }) => { + store.recordChangeDescriptions = currentTarget.checked; + }, + [store] + ); return (
@@ -145,25 +165,37 @@ function Settings(_: {||}) {
- {store.supportsCaptureScreenshots && ( -
-
Profiler
- - {captureScreenshots && ( -
- Screenshots will be throttled in order to reduce the negative - impact on performance. -
- )} -
- )} +
+
Profiler
+ + + + {store.supportsCaptureScreenshots && ( + + + {captureScreenshots && ( +
+ Screenshots will be throttled in order to reduce the negative + impact on performance. +
+ )} + + )} +
); }