Skip to content

Commit 2276ebd

Browse files
author
Brian Vaughn
committed
Added FB-only feature to show which hooks changed during profiling
I don't think this feature is broadly useful, but Benoit has asked for it to help with his performance investigations so I have enabled it only for Facebook DevTools builds. Note this doesn't impact either build unless the "Record why each component rendered while profiling." setting is enabled. When the feature flag and the setting are enabled, selecting an element in a profile will show which hooks have changed. Since the Profiler can't display the names of the changed hooks (it would be too heavy to compute this at runtime) the numbers will need to be matched up to the selected component. Fortunately the selection is synced between these two panels which should make matching at least somewhat faster. This is the first feature to actually use the recent DevTools feature flag system, so I also had to tweak a few things to make that work like it should.
1 parent f227e7f commit 2276ebd

File tree

11 files changed

+132
-17
lines changed

11 files changed

+132
-17
lines changed

packages/react-devtools-extensions/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
"build": "cross-env NODE_ENV=production yarn run build:chrome && yarn run build:firefox && yarn run build:edge",
77
"build:dev": "cross-env NODE_ENV=development yarn run build:chrome:dev && yarn run build:firefox:dev && yarn run build:edge:dev",
88
"build:chrome": "cross-env NODE_ENV=production node ./chrome/build",
9-
"build:chrome:crx": "cross-env NODE_ENV=production node ./chrome/build --crx",
9+
"build:chrome:crx": "cross-env NODE_ENV=production FEATURE_FLAG_TARGET=extension-fb node ./chrome/build --crx",
1010
"build:chrome:dev": "cross-env NODE_ENV=development node ./chrome/build",
1111
"build:firefox": "cross-env NODE_ENV=production node ./firefox/build",
1212
"build:firefox:dev": "cross-env NODE_ENV=development node ./firefox/build",

packages/react-devtools-extensions/webpack.backend.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
const {resolve} = require('path');
44
const {DefinePlugin} = require('webpack');
55
const {GITHUB_URL, getVersionString} = require('./utils');
6+
const {resolveFeatureFlags} = require('react-devtools-shared/buildUtils');
67

78
const NODE_ENV = process.env.NODE_ENV;
89
if (!NODE_ENV) {
@@ -16,6 +17,8 @@ const __DEV__ = NODE_ENV === 'development';
1617

1718
const DEVTOOLS_VERSION = getVersionString();
1819

20+
const featureFlagTarget = process.env.FEATURE_FLAG_TARGET || 'extension-oss';
21+
1922
module.exports = {
2023
mode: __DEV__ ? 'development' : 'production',
2124
devtool: __DEV__ ? 'cheap-module-eval-source-map' : false,
@@ -34,6 +37,7 @@ module.exports = {
3437
alias: {
3538
react: resolve(builtModulesDir, 'react'),
3639
'react-debug-tools': resolve(builtModulesDir, 'react-debug-tools'),
40+
'react-devtools-feature-flags': resolveFeatureFlags(featureFlagTarget),
3741
'react-dom': resolve(builtModulesDir, 'react-dom'),
3842
'react-is': resolve(builtModulesDir, 'react-is'),
3943
scheduler: resolve(builtModulesDir, 'scheduler'),

packages/react-devtools-extensions/webpack.config.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ const __DEV__ = NODE_ENV === 'development';
1717

1818
const DEVTOOLS_VERSION = getVersionString();
1919

20+
const featureFlagTarget = process.env.FEATURE_FLAG_TARGET || 'extension-oss';
21+
2022
module.exports = {
2123
mode: __DEV__ ? 'development' : 'production',
2224
devtool: __DEV__ ? 'cheap-module-eval-source-map' : false,
@@ -40,7 +42,7 @@ module.exports = {
4042
alias: {
4143
react: resolve(builtModulesDir, 'react'),
4244
'react-debug-tools': resolve(builtModulesDir, 'react-debug-tools'),
43-
'react-devtools-feature-flags': resolveFeatureFlags('extension'),
45+
'react-devtools-feature-flags': resolveFeatureFlags(featureFlagTarget),
4446
'react-dom': resolve(builtModulesDir, 'react-dom'),
4547
'react-is': resolve(builtModulesDir, 'react-is'),
4648
scheduler: resolve(builtModulesDir, 'scheduler'),

packages/react-devtools-shared/buildUtils.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,11 @@ function resolveFeatureFlags(target) {
1717
case 'shell':
1818
flagsPath = 'DevToolsFeatureFlags.default';
1919
break;
20-
case 'extension':
21-
flagsPath = 'DevToolsFeatureFlags.extension';
20+
case 'extension-oss':
21+
flagsPath = 'DevToolsFeatureFlags.extension-oss';
22+
break;
23+
case 'extension-fb':
24+
flagsPath = 'DevToolsFeatureFlags.extension-fb';
2225
break;
2326
default:
2427
console.error(`Invalid target "${target}"`);

packages/react-devtools-shared/src/backend/renderer.js

Lines changed: 50 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ import {
7979
MEMO_SYMBOL_STRING,
8080
} from './ReactSymbols';
8181
import {format} from './utils';
82+
import {enableProfilerChangedHookIndices} from 'react-devtools-feature-flags';
8283

8384
import type {Fiber} from 'react-reconciler/src/ReactInternalTypes';
8485
import type {
@@ -978,12 +979,9 @@ export function attach(
978979
state: null,
979980
};
980981
} else {
981-
return {
982+
const data: ChangeDescription = {
982983
context: getContextChangedKeys(nextFiber),
983-
didHooksChange: didHooksChange(
984-
prevFiber.memoizedState,
985-
nextFiber.memoizedState,
986-
),
984+
didHooksChange: false,
987985
isFirstMount: false,
988986
props: getChangedKeys(
989987
prevFiber.memoizedProps,
@@ -994,6 +992,23 @@ export function attach(
994992
nextFiber.memoizedState,
995993
),
996994
};
995+
996+
// Only traverse the hooks list once, depending on what info we're returning.
997+
if (enableProfilerChangedHookIndices) {
998+
const indices = getChangedHooksIndices(
999+
prevFiber.memoizedState,
1000+
nextFiber.memoizedState,
1001+
);
1002+
data.hooks = indices;
1003+
data.didHooksChange = indices !== null && indices.length > 0;
1004+
} else {
1005+
data.didHooksChange = didHooksChange(
1006+
prevFiber.memoizedState,
1007+
nextFiber.memoizedState,
1008+
);
1009+
}
1010+
1011+
return data;
9971012
}
9981013
default:
9991014
return null;
@@ -1154,6 +1169,36 @@ export function attach(
11541169
return false;
11551170
}
11561171

1172+
function getChangedHooksIndices(prev: any, next: any): null | Array<number> {
1173+
if (enableProfilerChangedHookIndices) {
1174+
if (prev == null || next == null) {
1175+
return null;
1176+
}
1177+
1178+
const indices = [];
1179+
let index = 0;
1180+
if (
1181+
next.hasOwnProperty('baseState') &&
1182+
next.hasOwnProperty('memoizedState') &&
1183+
next.hasOwnProperty('next') &&
1184+
next.hasOwnProperty('queue')
1185+
) {
1186+
while (next !== null) {
1187+
if (didHookChange(prev, next)) {
1188+
indices.push(index);
1189+
}
1190+
next = next.next;
1191+
prev = prev.next;
1192+
index++;
1193+
}
1194+
}
1195+
1196+
return indices;
1197+
}
1198+
1199+
return null;
1200+
}
1201+
11571202
function getChangedKeys(prev: any, next: any): null | Array<string> {
11581203
if (prev == null || next == null) {
11591204
return null;

packages/react-devtools-shared/src/backend/types.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,7 @@ export type ChangeDescription = {|
150150
isFirstMount: boolean,
151151
props: Array<string> | null,
152152
state: Array<string> | null,
153+
hooks?: Array<number> | null,
153154
|};
154155

155156
export type CommitDataBackend = {|

packages/react-devtools-shared/src/config/DevToolsFeatureFlags.default.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,4 @@
1313
* It should always be imported from "react-devtools-feature-flags".
1414
************************************************************************/
1515

16-
// TODO Add feature flags here...
16+
export const enableProfilerChangedHookIndices = false;

packages/react-devtools-shared/src/config/DevToolsFeatureFlags.extension.js renamed to packages/react-devtools-shared/src/config/DevToolsFeatureFlags.extension-fb.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,15 @@
1313
* It should always be imported from "react-devtools-feature-flags".
1414
************************************************************************/
1515

16-
// TODO Add feature flags here...
16+
export const enableProfilerChangedHookIndices = true;
1717

1818
/************************************************************************
1919
* Do not edit the code below.
2020
* It ensures this fork exports the same types as the default flags file.
2121
************************************************************************/
2222

2323
import typeof * as FeatureFlagsType from './DevToolsFeatureFlags.default';
24-
import typeof * as ExportsType from './DevToolsFeatureFlags.extension';
24+
import typeof * as ExportsType from './DevToolsFeatureFlags.extension-fb';
2525

2626
// eslint-disable-next-line no-unused-vars
2727
type Check<_X, Y: _X, X: Y = _X> = null;
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/**
2+
* Copyright (c) Facebook, Inc. and its affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*
7+
* @flow
8+
*/
9+
10+
/************************************************************************
11+
* This file is forked between different DevTools implementations.
12+
* It should never be imported directly!
13+
* It should always be imported from "react-devtools-feature-flags".
14+
************************************************************************/
15+
16+
export const enableProfilerChangedHookIndices = false;
17+
18+
/************************************************************************
19+
* Do not edit the code below.
20+
* It ensures this fork exports the same types as the default flags file.
21+
************************************************************************/
22+
23+
import typeof * as FeatureFlagsType from './DevToolsFeatureFlags.default';
24+
import typeof * as ExportsType from './DevToolsFeatureFlags.extension-oss';
25+
26+
// eslint-disable-next-line no-unused-vars
27+
type Check<_X, Y: _X, X: Y = _X> = null;
28+
// eslint-disable-next-line no-unused-expressions
29+
(null: Check<ExportsType, FeatureFlagsType>);

packages/react-devtools-shared/src/devtools/views/Profiler/WhatChanged.js

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,30 @@
99

1010
import * as React from 'react';
1111
import {useContext} from 'react';
12+
import {enableProfilerChangedHookIndices} from 'react-devtools-feature-flags';
1213
import {ProfilerContext} from '../Profiler/ProfilerContext';
1314
import {StoreContext} from '../context';
1415

1516
import styles from './WhatChanged.css';
1617

18+
function hookIndicesToString(indices: Array<number>): string {
19+
// This is debatable but I think 1-based might ake for a nicer UX.
20+
const numbers = indices.map(value => value + 1);
21+
22+
switch (numbers.length) {
23+
case 0:
24+
return 'No hooks changed';
25+
case 1:
26+
return `Hook ${numbers[0]} changed`;
27+
case 2:
28+
return `Hooks ${numbers[0]} and ${numbers[1]} changed`;
29+
default:
30+
return `Hooks ${numbers.slice(0, numbers.length - 1).join(', ')} and ${
31+
numbers[numbers.length - 1]
32+
} changed`;
33+
}
34+
}
35+
1736
type Props = {|
1837
fiberID: number,
1938
|};
@@ -81,11 +100,22 @@ export default function WhatChanged({fiberID}: Props) {
81100
}
82101

83102
if (changeDescription.didHooksChange) {
84-
changes.push(
85-
<div key="hooks" className={styles.Item}>
86-
• Hooks changed
87-
</div>,
88-
);
103+
if (
104+
enableProfilerChangedHookIndices &&
105+
Array.isArray(changeDescription.hooks)
106+
) {
107+
changes.push(
108+
<div key="hooks" className={styles.Item}>
109+
{hookIndicesToString(changeDescription.hooks)}
110+
</div>,
111+
);
112+
} else {
113+
changes.push(
114+
<div key="hooks" className={styles.Item}>
115+
• Hooks changed
116+
</div>,
117+
);
118+
}
89119
}
90120

91121
if (

packages/react-devtools-shared/src/devtools/views/Profiler/types.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ export type ChangeDescription = {|
4646
isFirstMount: boolean,
4747
props: Array<string> | null,
4848
state: Array<string> | null,
49+
hooks?: Array<number> | null,
4950
|};
5051

5152
export type CommitDataFrontend = {|

0 commit comments

Comments
 (0)