diff --git a/src/__tests__/__snapshots__/profilingCache-test.js.snap b/src/__tests__/__snapshots__/profilingCache-test.js.snap index 6d1e6ade29..6251b6e3bb 100644 --- a/src/__tests__/__snapshots__/profilingCache-test.js.snap +++ b/src/__tests__/__snapshots__/profilingCache-test.js.snap @@ -94,21 +94,24 @@ exports[`ProfilingCache should collect data for each commit: CommitDetails commi Object { "changeDescriptions": Map { 3 => Object { + "context": null, "didHooksChange": false, "props": Array [], - "state": Array [], + "state": null, }, 4 => Object { + "context": null, "didHooksChange": false, "props": Array [], - "state": Array [], + "state": null, }, 2 => Object { + "context": null, "didHooksChange": false, "props": Array [ "count", ], - "state": Array [], + "state": null, }, }, "duration": 13, @@ -137,16 +140,18 @@ exports[`ProfilingCache should collect data for each commit: CommitDetails commi Object { "changeDescriptions": Map { 3 => Object { + "context": null, "didHooksChange": false, "props": Array [], - "state": Array [], + "state": null, }, 2 => Object { + "context": null, "didHooksChange": false, "props": Array [ "count", ], - "state": Array [], + "state": null, }, }, "duration": 10, @@ -171,11 +176,12 @@ exports[`ProfilingCache should collect data for each commit: CommitDetails commi Object { "changeDescriptions": Map { 2 => Object { + "context": null, "didHooksChange": false, "props": Array [ "count", ], - "state": Array [], + "state": null, }, }, "duration": 10, @@ -256,27 +262,30 @@ Object { Array [ 3, Object { + "context": null, "didHooksChange": false, "props": Array [], - "state": Array [], + "state": null, }, ], Array [ 4, Object { + "context": null, "didHooksChange": false, "props": Array [], - "state": Array [], + "state": null, }, ], Array [ 2, Object { + "context": null, "didHooksChange": false, "props": Array [ "count", ], - "state": Array [], + "state": null, }, ], ], @@ -335,19 +344,21 @@ Object { Array [ 3, Object { + "context": null, "didHooksChange": false, "props": Array [], - "state": Array [], + "state": null, }, ], Array [ 2, Object { + "context": null, "didHooksChange": false, "props": Array [ "count", ], - "state": Array [], + "state": null, }, ], ], @@ -390,11 +401,12 @@ Object { Array [ 2, Object { + "context": null, "didHooksChange": false, "props": Array [ "count", ], - "state": Array [], + "state": null, }, ], ], @@ -662,19 +674,21 @@ Object { Array [ 3, Object { + "context": null, "didHooksChange": false, "props": Array [], - "state": Array [], + "state": null, }, ], Array [ 2, Object { + "context": null, "didHooksChange": false, "props": Array [ "count", ], - "state": Array [], + "state": null, }, ], ], @@ -725,27 +739,30 @@ Object { Array [ 3, Object { + "context": null, "didHooksChange": false, "props": Array [], - "state": Array [], + "state": null, }, ], Array [ 5, Object { + "context": null, "didHooksChange": false, "props": Array [], - "state": Array [], + "state": null, }, ], Array [ 2, Object { + "context": null, "didHooksChange": false, "props": Array [ "count", ], - "state": Array [], + "state": null, }, ], ], @@ -949,21 +966,24 @@ Object { Object { "changeDescriptions": Map { 3 => Object { + "context": null, "didHooksChange": false, "props": Array [], - "state": Array [], + "state": null, }, 4 => Object { + "context": null, "didHooksChange": false, "props": Array [], - "state": Array [], + "state": null, }, 2 => Object { + "context": null, "didHooksChange": false, "props": Array [ "count", ], - "state": Array [], + "state": null, }, }, "duration": 13, @@ -989,16 +1009,18 @@ Object { Object { "changeDescriptions": Map { 3 => Object { + "context": null, "didHooksChange": false, "props": Array [], - "state": Array [], + "state": null, }, 2 => Object { + "context": null, "didHooksChange": false, "props": Array [ "count", ], - "state": Array [], + "state": null, }, }, "duration": 10, @@ -1020,11 +1042,12 @@ Object { Object { "changeDescriptions": Map { 2 => Object { + "context": null, "didHooksChange": false, "props": Array [ "count", ], - "state": Array [], + "state": null, }, }, "duration": 10, @@ -1343,27 +1366,30 @@ Object { Array [ 3, Object { + "context": null, "didHooksChange": false, "props": Array [], - "state": Array [], + "state": null, }, ], Array [ 4, Object { + "context": null, "didHooksChange": false, "props": Array [], - "state": Array [], + "state": null, }, ], Array [ 2, Object { + "context": null, "didHooksChange": false, "props": Array [ "count", ], - "state": Array [], + "state": null, }, ], ], @@ -1422,19 +1448,21 @@ Object { Array [ 3, Object { + "context": null, "didHooksChange": false, "props": Array [], - "state": Array [], + "state": null, }, ], Array [ 2, Object { + "context": null, "didHooksChange": false, "props": Array [ "count", ], - "state": Array [], + "state": null, }, ], ], @@ -1477,11 +1505,12 @@ Object { Array [ 2, Object { + "context": null, "didHooksChange": false, "props": Array [ "count", ], - "state": Array [], + "state": null, }, ], ], @@ -1878,6 +1907,488 @@ Object { } `; +exports[`ProfilingCache should record when props/state/hooks change: CommitDetails commitIndex: 0 1`] = ` +Object { + "changeDescriptions": Map {}, + "duration": 0, + "fiberActualDurations": Map { + 1 => 0, + 2 => 0, + 3 => 0, + 4 => 0, + 5 => 0, + 6 => 0, + 7 => 0, + }, + "fiberSelfDurations": Map { + 1 => 0, + 2 => 0, + 3 => 0, + 4 => 0, + 5 => 0, + 6 => 0, + 7 => 0, + }, + "interactionIDs": Array [], + "priorityLevel": "Immediate", + "screenshot": null, + "timestamp": 0, +} +`; + +exports[`ProfilingCache should record when props/state/hooks change: CommitDetails commitIndex: 1 1`] = ` +Object { + "changeDescriptions": Map { + 5 => Object { + "context": null, + "didHooksChange": true, + "props": Array [ + "count", + ], + "state": null, + }, + 4 => Object { + "context": true, + "didHooksChange": false, + "props": Array [], + "state": null, + }, + 7 => Object { + "context": null, + "didHooksChange": true, + "props": Array [ + "count", + ], + "state": null, + }, + 6 => Object { + "context": Array [ + "count", + ], + "didHooksChange": false, + "props": Array [], + "state": null, + }, + 2 => Object { + "context": null, + "didHooksChange": false, + "props": Array [], + "state": Array [ + "count", + ], + }, + }, + "duration": 0, + "fiberActualDurations": Map { + 5 => 0, + 4 => 0, + 7 => 0, + 6 => 0, + 3 => 0, + 2 => 0, + }, + "fiberSelfDurations": Map { + 5 => 0, + 4 => 0, + 7 => 0, + 6 => 0, + 3 => 0, + 2 => 0, + }, + "interactionIDs": Array [], + "priorityLevel": "Immediate", + "screenshot": null, + "timestamp": 0, +} +`; + +exports[`ProfilingCache should record when props/state/hooks change: imported data 1`] = ` +Object { + "dataForRoots": Array [ + Object { + "commitData": Array [ + Object { + "changeDescriptions": Array [], + "duration": 0, + "fiberActualDurations": Array [ + Array [ + 1, + 0, + ], + Array [ + 2, + 0, + ], + Array [ + 3, + 0, + ], + Array [ + 4, + 0, + ], + Array [ + 5, + 0, + ], + Array [ + 6, + 0, + ], + Array [ + 7, + 0, + ], + ], + "fiberSelfDurations": Array [ + Array [ + 1, + 0, + ], + Array [ + 2, + 0, + ], + Array [ + 3, + 0, + ], + Array [ + 4, + 0, + ], + Array [ + 5, + 0, + ], + Array [ + 6, + 0, + ], + Array [ + 7, + 0, + ], + ], + "interactionIDs": Array [], + "priorityLevel": "Immediate", + "screenshot": null, + "timestamp": 0, + }, + Object { + "changeDescriptions": Array [ + Array [ + 5, + Object { + "context": null, + "didHooksChange": true, + "props": Array [ + "count", + ], + "state": null, + }, + ], + Array [ + 4, + Object { + "context": true, + "didHooksChange": false, + "props": Array [], + "state": null, + }, + ], + Array [ + 7, + Object { + "context": null, + "didHooksChange": true, + "props": Array [ + "count", + ], + "state": null, + }, + ], + Array [ + 6, + Object { + "context": Array [ + "count", + ], + "didHooksChange": false, + "props": Array [], + "state": null, + }, + ], + Array [ + 2, + Object { + "context": null, + "didHooksChange": false, + "props": Array [], + "state": Array [ + "count", + ], + }, + ], + ], + "duration": 0, + "fiberActualDurations": Array [ + Array [ + 5, + 0, + ], + Array [ + 4, + 0, + ], + Array [ + 7, + 0, + ], + Array [ + 6, + 0, + ], + Array [ + 3, + 0, + ], + Array [ + 2, + 0, + ], + ], + "fiberSelfDurations": Array [ + Array [ + 5, + 0, + ], + Array [ + 4, + 0, + ], + Array [ + 7, + 0, + ], + Array [ + 6, + 0, + ], + Array [ + 3, + 0, + ], + Array [ + 2, + 0, + ], + ], + "interactionIDs": Array [], + "priorityLevel": "Immediate", + "screenshot": null, + "timestamp": 0, + }, + ], + "displayName": "LegacyContextProvider", + "initialTreeBaseDurations": Array [], + "interactionCommits": Array [], + "interactions": Array [], + "operations": Array [ + Array [ + 1, + 1, + 110, + 21, + 76, + 101, + 103, + 97, + 99, + 121, + 67, + 111, + 110, + 116, + 101, + 120, + 116, + 80, + 114, + 111, + 118, + 105, + 100, + 101, + 114, + 16, + 67, + 111, + 110, + 116, + 101, + 120, + 116, + 46, + 80, + 114, + 111, + 118, + 105, + 100, + 101, + 114, + 21, + 77, + 111, + 100, + 101, + 114, + 110, + 67, + 111, + 110, + 116, + 101, + 120, + 116, + 67, + 111, + 110, + 115, + 117, + 109, + 101, + 114, + 26, + 70, + 117, + 110, + 99, + 116, + 105, + 111, + 110, + 67, + 111, + 109, + 112, + 111, + 110, + 101, + 110, + 116, + 87, + 105, + 116, + 104, + 72, + 111, + 111, + 107, + 115, + 21, + 76, + 101, + 103, + 97, + 99, + 121, + 67, + 111, + 110, + 116, + 101, + 120, + 116, + 67, + 111, + 110, + 115, + 117, + 109, + 101, + 114, + 1, + 1, + 11, + 1, + 1, + 1, + 2, + 1, + 1, + 0, + 1, + 0, + 4, + 2, + 0, + 1, + 3, + 2, + 2, + 2, + 2, + 0, + 4, + 3, + 0, + 1, + 4, + 1, + 3, + 2, + 3, + 0, + 4, + 4, + 0, + 1, + 5, + 5, + 4, + 4, + 4, + 0, + 4, + 5, + 0, + 1, + 6, + 1, + 3, + 2, + 5, + 0, + 4, + 6, + 0, + 1, + 7, + 5, + 6, + 6, + 4, + 0, + 4, + 7, + 0, + ], + Array [ + 1, + 1, + 0, + ], + ], + "rootID": 1, + "snapshots": Array [], + }, + ], + "version": 4, +} +`; + exports[`ProfilingCache should report every traced interaction: Interactions 1`] = ` Array [ Object { @@ -1951,19 +2462,21 @@ Object { Array [ 3, Object { + "context": null, "didHooksChange": false, "props": Array [], - "state": Array [], + "state": null, }, ], Array [ 2, Object { + "context": null, "didHooksChange": false, "props": Array [ "count", ], - "state": Array [], + "state": null, }, ], ], diff --git a/src/__tests__/profilingCache-test.js b/src/__tests__/profilingCache-test.js index 83c1c056d1..0b76937138 100644 --- a/src/__tests__/profilingCache-test.js +++ b/src/__tests__/profilingCache-test.js @@ -5,6 +5,7 @@ import type Bridge from 'src/bridge'; import type Store from 'src/devtools/store'; describe('ProfilingCache', () => { + let PropTypes; let React; let ReactDOM; let Scheduler; @@ -23,6 +24,7 @@ describe('ProfilingCache', () => { store.collapseNodesByDefault = false; store.recordChangeDescriptions = true; + PropTypes = require('prop-types'); React = require('react'); ReactDOM = require('react-dom'); Scheduler = require('scheduler'); @@ -188,6 +190,110 @@ describe('ProfilingCache', () => { } }); + it('should record when props/state/hooks change', () => { + let instance = null; + + const ModernContext = React.createContext(0); + + class LegacyContextProvider extends React.Component< + any, + {| count: number |} + > { + static childContextTypes = { + count: PropTypes.number, + }; + state = { count: 0 }; + getChildContext() { + return this.state; + } + render() { + instance = this; + return ( + + + + + + + ); + } + } + + const FunctionComponentWithHooks = ({ count }) => { + React.useMemo(() => count, [count]); + return null; + }; + + class ModernContextConsumer extends React.Component { + static contextType = ModernContext; + render() { + return ; + } + } + + class LegacyContextConsumer extends React.Component { + static contextTypes = { + count: PropTypes.number, + }; + render() { + return ; + } + } + + const container = document.createElement('div'); + + utils.act(() => store.profilerStore.startProfiling()); + utils.act(() => ReactDOM.render(, container)); + expect(instance).not.toBeNull(); + utils.act(() => (instance: any).setState({ count: 1 })); + utils.act(() => store.profilerStore.stopProfiling()); + + const allCommitData = []; + + 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}` + ); + } + return null; + } + + const rootID = store.roots[0]; + + for (let commitIndex = 0; commitIndex < 2; commitIndex++) { + utils.act(() => { + TestRenderer.create( + + ); + }); + } + + expect(allCommitData).toHaveLength(2); + + utils.exportImportHelper(bridge, store); + + for (let commitIndex = 0; commitIndex < 2; commitIndex++) { + utils.act(() => { + TestRenderer.create( + + ); + }); + } + }); + it('should calculate a self duration based on actual children (not filtered children)', () => { store.componentFilters = [utils.createDisplayNameFilter('^Parent$')]; diff --git a/src/backend/renderer.js b/src/backend/renderer.js index 6f2bea1d20..90accb8feb 100644 --- a/src/backend/renderer.js +++ b/src/backend/renderer.js @@ -662,6 +662,7 @@ export function attach( prevFiber.memoizedState, nextFiber.memoizedState ), + context: getContextChangedKeys(nextFiber), props: getChangedKeys( prevFiber.memoizedProps, nextFiber.memoizedProps @@ -676,6 +677,88 @@ export function attach( } } + function updateContextsForFiber(fiber: Fiber) { + switch (getElementTypeForFiber(fiber)) { + case ElementTypeClass: + if (idToContextsMap !== null) { + const id = getFiberID(getPrimaryFiber(fiber)); + const contexts = getContextsForFiber(fiber); + if (contexts !== null) { + idToContextsMap.set(id, contexts); + } + } + break; + default: + break; + } + } + + const NO_CONTEXT = {}; + + function getContextsForFiber(fiber: Fiber): [Object, any] | null { + switch (getElementTypeForFiber(fiber)) { + case ElementTypeClass: + const instance = fiber.stateNode; + let legacyContext = NO_CONTEXT; + let modernContext = NO_CONTEXT; + if (instance != null) { + if ( + instance.constructor && + instance.constructor.contextType != null + ) { + modernContext = instance.context; + } else { + legacyContext = instance.context; + if (legacyContext && Object.keys(legacyContext).length === 0) { + legacyContext = NO_CONTEXT; + } + } + } + return [legacyContext, modernContext]; + default: + return null; + } + } + + function crawlToInitializeContextsMap(fiber: Fiber) { + updateContextsForFiber(fiber); + let current = fiber.child; + while (current !== null) { + crawlToInitializeContextsMap(current); + current = current.sibling; + } + } + + function getContextChangedKeys(fiber: Fiber): null | boolean | Array { + switch (getElementTypeForFiber(fiber)) { + case ElementTypeClass: + if (idToContextsMap !== null) { + const id = getFiberID(getPrimaryFiber(fiber)); + const prevContexts = idToContextsMap.has(id) + ? idToContextsMap.get(id) + : null; + const nextContexts = getContextsForFiber(fiber); + + if (prevContexts == null || nextContexts == null) { + return null; + } + + const [prevLegacyContext, prevModernContext] = prevContexts; + const [nextLegacyContext, nextModernContext] = nextContexts; + + if (nextLegacyContext !== NO_CONTEXT) { + return getChangedKeys(prevLegacyContext, nextLegacyContext); + } else if (nextModernContext !== NO_CONTEXT) { + return prevModernContext !== nextModernContext; + } + } + break; + default: + break; + } + return null; + } + function didHooksChange(prev: any, next: any): boolean { if (next == null) { return false; @@ -701,11 +784,9 @@ export function attach( return false; } - function getChangedKeys(prev: any, next: any): Array { - const keys = []; - - if (next == null) { - return keys; + function getChangedKeys(prev: any, next: any): null | Array { + if (prev == null || next == null) { + return null; } // We can't report anything meaningful for hooks changes. @@ -715,16 +796,18 @@ export function attach( next.hasOwnProperty('next') && next.hasOwnProperty('queue') ) { - return keys; + return null; } - // TODO (change descriptions) This does not account for props that were added or removed. - for (let key in prev) { + const keys = new Set([...Object.keys(prev), ...Object.keys(next)]); + const changedKeys = []; + for (let key of keys) { if (prev[key] !== next[key]) { - keys.push(key); + changedKeys.push(key); } } - return keys; + + return changedKeys; } // eslint-disable-next-line no-unused-vars @@ -1139,6 +1222,8 @@ export function attach( if (changeDescription !== null) { metadata.changeDescriptions.set(id, changeDescription); } + + updateContextsForFiber(fiber); } } } @@ -2164,6 +2249,7 @@ export function attach( let currentCommitProfilingMetadata: CommitProfilingData | null = null; let displayNamesByRootID: DisplayNamesByRootID | null = null; + let idToContextsMap: Map | null = null; let initialTreeBaseDurationsMap: Map | null = null; let initialIDToRootMap: Map | null = null; let isProfiling: boolean = false; @@ -2284,6 +2370,7 @@ export function attach( displayNamesByRootID = new Map(); initialTreeBaseDurationsMap = new Map(idToTreeBaseDurationMap); initialIDToRootMap = new Map(idToRootMap); + idToContextsMap = new Map(); hook.getFiberRoots(rendererID).forEach(root => { const rootID = getFiberID(getPrimaryFiber(root.current)); @@ -2291,6 +2378,10 @@ export function attach( rootID, getDisplayNameForRoot(root.current) ); + + if (shouldRecordChangeDescriptions) { + crawlToInitializeContextsMap(root.current); + } }); isProfiling = true; diff --git a/src/backend/types.js b/src/backend/types.js index 220f1358ec..5b55945742 100644 --- a/src/backend/types.js +++ b/src/backend/types.js @@ -112,11 +112,12 @@ export type ReactRenderer = { currentDispatcherRef?: {| current: null | Dispatcher |}, }; -// TODO (change descriptions) Is it important to handle context? +// TODO (change descriptions) Should we report changed hooks keys? export type ChangeDescription = {| + context: Array | boolean | null, didHooksChange: boolean, - props: Array, - state: Array, + props: Array | null, + state: Array | null, |}; export type CommitDataBackend = {| diff --git a/src/devtools/views/Profiler/SidebarSelectedFiberInfo.js b/src/devtools/views/Profiler/SidebarSelectedFiberInfo.js index 2b3c1b793a..bfaab32e3f 100644 --- a/src/devtools/views/Profiler/SidebarSelectedFiberInfo.js +++ b/src/devtools/views/Profiler/SidebarSelectedFiberInfo.js @@ -104,21 +104,33 @@ function WhatChanged({ const changeDescription = changeDescriptions.get(fiberID); if (changeDescription == null) { - return null; - } - - const changes = []; - if (changeDescription.didHooksChange) { - changes.push( -
- • Hooks + // This indicates that the component mounted during this commit + return ( +
+ +
+ This is the first time the component rendered. +
); } - if (changeDescription.props.length !== 0) { + + const changes = []; + + if (changeDescription.didHooksChange) { + changes.push( +
+ • Hooks changed +
+ ); + } + if ( + changeDescription.props !== null && + changeDescription.props.length !== 0 + ) { changes.push(
- • Props + • Props changed: {changeDescription.props.map(key => ( {key} @@ -127,10 +139,13 @@ function WhatChanged({
); } - if (changeDescription.state.length !== 0) { + if ( + changeDescription.state !== null && + changeDescription.state.length !== 0 + ) { changes.push(
- • State + • State changed: {changeDescription.state.map(key => ( {key} @@ -141,12 +156,16 @@ function WhatChanged({ } if (changes.length === 0) { - changes.push(
Nothing
); + changes.push( +
+ The parent component rendered. +
+ ); } return (
- + {changes}
); diff --git a/src/devtools/views/Profiler/types.js b/src/devtools/views/Profiler/types.js index 667ab54ce3..514ff815cf 100644 --- a/src/devtools/views/Profiler/types.js +++ b/src/devtools/views/Profiler/types.js @@ -31,11 +31,12 @@ export type SnapshotNode = {| type: ElementType, |}; -// TODO (change descriptions) Is it important to handle context? +// TODO (change descriptions) Should we report changed hooks keys? export type ChangeDescription = {| + context: Array | boolean | null, didHooksChange: boolean, - props: Array, - state: Array, + props: Array | null, + state: Array | null, |}; export type CommitDataFrontend = {| diff --git a/src/devtools/views/Settings/Settings.js b/src/devtools/views/Settings/Settings.js index b53dee04e2..3e29c19d3b 100644 --- a/src/devtools/views/Settings/Settings.js +++ b/src/devtools/views/Settings/Settings.js @@ -174,7 +174,7 @@ function Settings(_: {||}) { checked={recordChangeDescriptions} onChange={updateRecordChangeDescriptions} />{' '} - Record which props/state/hooks changed while profiling + Record why each component rendered while profiling. {store.supportsCaptureScreenshots && (