maxjhandsome
/
react
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Added context support to change description

This commit is contained in:
Brian Vaughn 2019-06-08 09:10:54 -07:00
parent bd5c2e4621
commit 2d48401346
7 changed files with 793 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

575
src/__tests__/__snapshots__/profilingCache-test.js.snap
View File

@ -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,
},
],
],

106
src/__tests__/profilingCache-test.js
View File

@ -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 (
<ModernContext.Provider value={this.state.count}>
<React.Fragment>
<ModernContextConsumer />
<LegacyContextConsumer />
</React.Fragment>
</ModernContext.Provider>
);
}
}
const FunctionComponentWithHooks = ({ count }) => {
React.useMemo(() => count, [count]);
return null;
};
class ModernContextConsumer extends React.Component<any> {
static contextType = ModernContext;
render() {
return <FunctionComponentWithHooks count={this.context} />;
}
}
class LegacyContextConsumer extends React.Component<any> {
static contextTypes = {
count: PropTypes.number,
};
render() {
return <FunctionComponentWithHooks count={this.context.count} />;
}
}
const container = document.createElement('div');
utils.act(() => store.profilerStore.startProfiling());
utils.act(() => ReactDOM.render(<LegacyContextProvider />, 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(
<Validator
commitIndex={commitIndex}
previousCommitDetails={null}
rootID={rootID}
/>
);
});
}
expect(allCommitData).toHaveLength(2);
utils.exportImportHelper(bridge, store);
for (let commitIndex = 0; commitIndex < 2; commitIndex++) {
utils.act(() => {
TestRenderer.create(
<Validator
commitIndex={commitIndex}
previousCommitDetails={allCommitData[commitIndex]}
rootID={rootID}
/>
);
});
}
});
it('should calculate a self duration based on actual children (not filtered children)', () => {
store.componentFilters = [utils.createDisplayNameFilter('^Parent$')];

111
src/backend/renderer.js
View File

@ -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<string> {
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<string> {
const keys = [];
if (next == null) {
return keys;
function getChangedKeys(prev: any, next: any): null | Array<string> {
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<number, any> | null = null;
let initialTreeBaseDurationsMap: Map<number, number> | null = null;
let initialIDToRootMap: Map<number, number> | 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;

7
src/backend/types.js
View File

@ -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<string> | boolean | null,
didHooksChange: boolean,
props: Array<string>,
state: Array<string>,
props: Array<string> | null,
state: Array<string> | null,
|};
export type CommitDataBackend = {|

47
src/devtools/views/Profiler/SidebarSelectedFiberInfo.js
View File

@ -104,21 +104,33 @@ function WhatChanged({
const changeDescription = changeDescriptions.get(fiberID);
if (changeDescription == null) {
return null;
}
const changes = [];
if (changeDescription.didHooksChange) {
changes.push(
<div key="hooks" className={styles.WhatChangedItem}>
• Hooks
// This indicates that the component mounted during this commit
return (
<div className={styles.Content}>
<label className={styles.Label}>Why did this render?</label>
<div className={styles.WhatChangedItem}>
This is the first time the component rendered.
</div>
</div>
);
}
if (changeDescription.props.length !== 0) {
const changes = [];
if (changeDescription.didHooksChange) {
changes.push(
<div key="hooks" className={styles.WhatChangedItem}>
• Hooks changed
</div>
);
}
if (
changeDescription.props !== null &&
changeDescription.props.length !== 0
) {
changes.push(
<div key="props" className={styles.WhatChangedItem}>
• Props
• Props changed:
{changeDescription.props.map(key => (
<span key={key} className={styles.WhatChangedKey}>
{key}
@ -127,10 +139,13 @@ function WhatChanged({
</div>
);
}
if (changeDescription.state.length !== 0) {
if (
changeDescription.state !== null &&
changeDescription.state.length !== 0
) {
changes.push(
<div key="state" className={styles.WhatChangedItem}>
• State
• State changed:
{changeDescription.state.map(key => (
<span key={key} className={styles.WhatChangedKey}>
{key}
@ -141,12 +156,16 @@ function WhatChanged({
}
if (changes.length === 0) {
changes.push(<div className={styles.WhatChangedItem}>Nothing</div>);
changes.push(
<div key="nothing" className={styles.WhatChangedItem}>
The parent component rendered.
</div>
);
}
return (
<div className={styles.Content}>
<label className={styles.Label}>What changed?</label>
<label className={styles.Label}>Why did this render?</label>
{changes}
</div>
);

7
src/devtools/views/Profiler/types.js
View File

@ -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<string> | boolean | null,
didHooksChange: boolean,
props: Array<string>,
state: Array<string>,
props: Array<string> | null,
state: Array<string> | null,
|};
export type CommitDataFrontend = {|

2
src/devtools/views/Settings/Settings.js
View File

@ -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.
</label>
{store.supportsCaptureScreenshots && (