Fixed edge case bug in Profiler

facebook · Mar 4, 2022 · aa1bfaf · aa1bfaf
1 parent f6c130f
commit aa1bfaf
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 28 deletions.
diff --git a/packages/react-devtools-shared/src/__tests__/__snapshots__/profilingCache-test.js.snap b/packages/react-devtools-shared/src/__tests__/__snapshots__/profilingCache-test.js.snap
@@ -2778,8 +2778,12 @@ Object {
       1,
       0,
       2,
-      1,
+      5,
       4,
+      11,
+      10,
+      8,
+      7,
     ],
   ],
   "rootID": 1,
@@ -3279,8 +3283,12 @@ Object {
           1,
           0,
           2,
-          1,
+          5,
           4,
+          11,
+          10,
+          8,
+          7,
         ],
       ],
       "rootID": 1,

diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js
@@ -1622,18 +1622,25 @@ export function attach(
     pendingOperations.push(op);
   }
 
-  function flushOrQueueOperations(operations: OperationsArray): void {
-    if (operations.length === 3) {
-      // This operations array is a no op: [renderer ID, root ID, string table size (0)]
-      // We can usually skip sending updates like this across the bridge, unless we're Profiling.
-      // In that case, even though the tree didn't change– some Fibers may have still rendered.
-      if (
-        !isProfiling ||
+  function shouldBailoutWithPendingOperations() {
+    if (!isProfiling) {
+      return (
+        pendingOperations.length === 0 &&
+        pendingRealUnmountedIDs.length === 0 &&
+        pendingSimulatedUnmountedIDs.length === 0 &&
+        pendingUnmountedRootID === null
+      );
+    } else {
+      return (
         currentCommitProfilingMetadata == null ||
         currentCommitProfilingMetadata.durations.length === 0
-      ) {
-        return;
-      }
+      );
+    }
+  }
+
+  function flushOrQueueOperations(operations: OperationsArray): void {
+    if (shouldBailoutWithPendingOperations()) {
+      return;
     }
 
     if (pendingOperationsQueue !== null) {
@@ -1666,7 +1673,7 @@ export function attach(
 
       recordPendingErrorsAndWarnings();
 
-      if (pendingOperations.length === 0) {
+      if (shouldBailoutWithPendingOperations()) {
         // No warnings or errors to flush; we can bail out early here too.
         return;
       }
@@ -1789,12 +1796,7 @@ export function attach(
     // We do this just before flushing, so we can ignore errors for no-longer-mounted Fibers.
     recordPendingErrorsAndWarnings();
 
-    if (
-      pendingOperations.length === 0 &&
-      pendingRealUnmountedIDs.length === 0 &&
-      pendingSimulatedUnmountedIDs.length === 0 &&
-      pendingUnmountedRootID === null
-    ) {
+    if (shouldBailoutWithPendingOperations()) {
       // If we aren't profiling, we can just bail out here.
       // No use sending an empty update over the bridge.
       //
@@ -1803,9 +1805,7 @@ export function attach(
       // (2) the operations array for each commit
       // Because of this, it's important that the operations and metadata arrays align,
       // So it's important not to omit even empty operations while profiling is active.
-      if (!isProfiling) {
-        return;
-      }
+      return;
     }
 
     const numUnmountIDs =
@@ -2722,12 +2722,7 @@ export function attach(
     }
 
     if (isProfiling && isProfilingSupported) {
-      // Make sure at least one Fiber performed work during this commit.
-      // If not, don't send it to the frontend; showing an empty commit in the Profiler is confusing.
-      if (
-        currentCommitProfilingMetadata != null &&
-        currentCommitProfilingMetadata.durations.length > 0
-      ) {
+      if (!shouldBailoutWithPendingOperations()) {
         const commitProfilingMetadata = ((rootToCommitProfilingMetadataMap: any): CommitProfilingMetadataMap).get(
           currentRootID,
         );