Don't show empty (no work) commits in Profiler

facebook · Nov 1, 2019 · 701a308 · 701a308
1 parent 9a35adc
commit 701a308
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 2 deletions.
diff --git a/packages/react-devtools-shared/src/__tests__/profilerStore-test.js b/packages/react-devtools-shared/src/__tests__/profilerStore-test.js
@@ -79,4 +79,44 @@ describe('ProfilerStore', () => {
     store.profilerStore.profilingData = fauxProfilingData;
     expect(store.profilerStore.profilingData).toBe(fauxProfilingData);
   });
+
+  // This test covers current broken behavior (arguably) with the synthetic event system.
+  it('should filter empty commits', () => {
+    const inputRef = React.createRef();
+    const ControlledInput = () => {
+      const [name, setName] = React.useState('foo');
+      const handleChange = event => setName(event.target.value);
+      return <input ref={inputRef} value={name} onChange={handleChange} />;
+    };
+
+    const container = document.createElement('div');
+
+    // This element has to be in the <body> for the event system to work.
+    document.body.appendChild(container);
+
+    // It's important that this test uses legacy sync mode.
+    // The root API does not trigger this particular failing case.
+    ReactDOM.render(<ControlledInput />, container);
+
+    utils.act(() => store.profilerStore.startProfiling());
+
+    // Sets a value in a way that React doesn't see,
+    // so that a subsequent "change" event will trigger the event handler.
+    const setUntrackedValue = Object.getOwnPropertyDescriptor(
+      HTMLInputElement.prototype,
+      'value',
+    ).set;
+
+    const target = inputRef.current;
+    setUntrackedValue.call(target, 'bar');
+    target.dispatchEvent(new Event('input', {bubbles: true, cancelable: true}));
+    expect(target.value).toBe('bar');
+
+    utils.act(() => store.profilerStore.stopProfiling());
+
+    // Only one commit should have been recorded (in response to the "change" event).
+    const root = store.roots[0];
+    const data = store.profilerStore.getDataForRoot(root);
+    expect(data.commitData).toHaveLength(1);
+  });
 });
diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js
@@ -1643,6 +1643,7 @@ export function attach(
         }
       }
     }
+
     if (shouldIncludeInTree) {
       const isProfilingSupported = nextFiber.hasOwnProperty('treeBaseDuration');
       if (isProfilingSupported) {
@@ -1742,6 +1743,15 @@ export function attach(
     const current = root.current;
     const alternate = current.alternate;
 
+    // Certain types of updates bail out at the root without doing any actual render work.
+    // React should probably not call the DevTools commit hook in this case,
+    // but if it does- we can detect it and filter them out from the profiler.
+    const didBailoutAtRoot =
+      root.current === root.current.alternate ||
+      (alternate !== null &&
+        alternate.expirationTime === 0 &&
+        alternate.childExpirationTime === 0);
+
     currentRootID = getFiberID(getPrimaryFiber(current));
 
     // Before the traversals, remember to start tracking
@@ -1758,7 +1768,7 @@ export function attach(
     // where some v16 renderers support profiling and others don't.
     const isProfilingSupported = root.memoizedInteractions != null;
 
-    if (isProfiling && isProfilingSupported) {
+    if (isProfiling && isProfilingSupported && !didBailoutAtRoot) {
       // If profiling is active, store commit time and duration, and the current interactions.
       // The frontend may request this information after profiling has stopped.
       currentCommitProfilingMetadata = {
@@ -1802,7 +1812,7 @@ export function attach(
       mountFiberRecursively(current, null, false, false);
     }
 
-    if (isProfiling && isProfilingSupported) {
+    if (isProfiling && isProfilingSupported && !didBailoutAtRoot) {
       const commitProfilingMetadata = ((rootToCommitProfilingMetadataMap: any): CommitProfilingMetadataMap).get(
         currentRootID,
       );

diff --git a/packages/react-devtools-shared/src/backend/types.js b/packages/react-devtools-shared/src/backend/types.js
@@ -284,6 +284,9 @@ export type DevToolsHook = {
   onCommitFiberRoot: (
     rendererID: RendererID,
     fiber: Object,
+    // Added in v16.9 to support Profiler priority labels
     commitPriority?: number,
+    // Added in v16.9 to support Fast Refresh
+    didError?: boolean,
   ) => void,
 };