Don't show empty (no work) commits in Profiler (#17253)
Brian Vaughn committed
Nov 1, 2019 at 15:51 UTC
36fd29f09fae82766207899ac31e462d8ee284e5
3 files changed
+54
-2
packages/react-devtools-shared/src/__tests__/profilerStore-test.js
+40
@@ -79,4 +79,44 @@ describe('ProfilerStore', () => {
79
store.profilerStore.profilingData = fauxProfilingData;
80
expect(store.profilerStore.profilingData).toBe(fauxProfilingData);
81
});
82
+
83
+ // This test covers current broken behavior (arguably) with the synthetic event system.
84
+ it('should filter empty commits', () => {
85
+ const inputRef = React.createRef();
86
+ const ControlledInput = () => {
87
+ const [name, setName] = React.useState('foo');
88
+ const handleChange = event => setName(event.target.value);
89
+ return <input ref={inputRef} value={name} onChange={handleChange} />;
90
+ };
91
+
92
+ const container = document.createElement('div');
93
+
94
+ // This element has to be in the <body> for the event system to work.
95
+ document.body.appendChild(container);
96
+
97
+ // It's important that this test uses legacy sync mode.
98
+ // The root API does not trigger this particular failing case.
99
+ ReactDOM.render(<ControlledInput />, container);
100
+
101
+ utils.act(() => store.profilerStore.startProfiling());
102
+
103
+ // Sets a value in a way that React doesn't see,
104
+ // so that a subsequent "change" event will trigger the event handler.
105
+ const setUntrackedValue = Object.getOwnPropertyDescriptor(
106
+ HTMLInputElement.prototype,
107
+ 'value',
108
+ ).set;
109
+
110
+ const target = inputRef.current;
111
+ setUntrackedValue.call(target, 'bar');
112
+ target.dispatchEvent(new Event('input', {bubbles: true, cancelable: true}));
113
+ expect(target.value).toBe('bar');
114
+
115
+ utils.act(() => store.profilerStore.stopProfiling());
116
+
117
+ // Only one commit should have been recorded (in response to the "change" event).
118
+ const root = store.roots[0];
119
+ const data = store.profilerStore.getDataForRoot(root);
120
+ expect(data.commitData).toHaveLength(1);
121
+ });
122
});
packages/react-devtools-shared/src/backend/renderer.js
+11
-2
@@ -1643,6 +1643,7 @@ export function attach(
1643
}
1644
}
1645
}
1646
+
1647
if (shouldIncludeInTree) {
1648
const isProfilingSupported = nextFiber.hasOwnProperty('treeBaseDuration');
1649
if (isProfilingSupported) {
@@ -1742,6 +1743,14 @@ export function attach(
1743
const current = root.current;
1744
const alternate = current.alternate;
1745
1746
+ // Certain types of updates bail out at the root without doing any actual render work.
1747
+ // React should probably not call the DevTools commit hook in this case,
1748
+ // but if it does- we can detect it and filter them out from the profiler.
1749
+ const didBailoutAtRoot =
1750
+ alternate !== null &&
1751
+ alternate.expirationTime === 0 &&
1752
+ alternate.childExpirationTime === 0;
1753
+
1754
currentRootID = getFiberID(getPrimaryFiber(current));
1755
1756
// Before the traversals, remember to start tracking
@@ -1758,7 +1767,7 @@ export function attach(
1767
// where some v16 renderers support profiling and others don't.
1768
const isProfilingSupported = root.memoizedInteractions != null;
1769
1761
- if (isProfiling && isProfilingSupported) {
1770
+ if (isProfiling && isProfilingSupported && !didBailoutAtRoot) {
1771
// If profiling is active, store commit time and duration, and the current interactions.
1772
// The frontend may request this information after profiling has stopped.
1773
currentCommitProfilingMetadata = {
@@ -1802,7 +1811,7 @@ export function attach(
1811
mountFiberRecursively(current, null, false, false);
1812
}
1813
1805
- if (isProfiling && isProfilingSupported) {
1814
+ if (isProfiling && isProfilingSupported && !didBailoutAtRoot) {
1815
const commitProfilingMetadata = ((rootToCommitProfilingMetadataMap: any): CommitProfilingMetadataMap).get(
1816
currentRootID,
1817
);
packages/react-devtools-shared/src/backend/types.js
+3
@@ -284,6 +284,9 @@ export type DevToolsHook = {
284
onCommitFiberRoot: (
285
rendererID: RendererID,
286
fiber: Object,
287
+ // Added in v16.9 to support Profiler priority labels
288
commitPriority?: number,
289
+ // Added in v16.9 to support Fast Refresh
290
+ didError?: boolean,
291
) => void,
292
};