[DevTools] Only show boundaries with unique suspenders by default in the timeline (facebook#34397)

Author: eps1lon

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

@@ -88,6 +88,7 @@ import {
   SUSPENSE_TREE_OPERATION_REMOVE,
   SUSPENSE_TREE_OPERATION_REORDER_CHILDREN,
   SUSPENSE_TREE_OPERATION_RESIZE,
+  SUSPENSE_TREE_OPERATION_SUSPENDERS,
   UNKNOWN_SUSPENDERS_NONE,
   UNKNOWN_SUSPENDERS_REASON_PRODUCTION,
   UNKNOWN_SUSPENDERS_REASON_OLD_VERSION,
@@ -2016,6 +2017,7 @@ export function attach(
   const pendingOperations: OperationsArray = [];
   const pendingRealUnmountedIDs: Array<FiberInstance['id']> = [];
   const pendingRealUnmountedSuspenseIDs: Array<FiberInstance['id']> = [];
+  const pendingSuspenderChanges: Set<FiberInstance['id']> = new Set();
   let pendingOperationsQueue: Array<OperationsArray> | null = [];
   const pendingStringTable: Map<string, StringTableEntry> = new Map();
   let pendingStringTableLength: number = 0;
@@ -2047,6 +2049,7 @@ export function attach(
       pendingOperations.length === 0 &&
       pendingRealUnmountedIDs.length === 0 &&
       pendingRealUnmountedSuspenseIDs.length === 0 &&
+      pendingSuspenderChanges.size === 0 &&
       pendingUnmountedRootID === null
     );
   }
@@ -2113,6 +2116,7 @@ export function attach(
       pendingRealUnmountedIDs.length +
       (pendingUnmountedRootID === null ? 0 : 1);
     const numUnmountSuspenseIDs = pendingRealUnmountedSuspenseIDs.length;
+    const numSuspenderChanges = pendingSuspenderChanges.size;
 
     const operations = new Array<number>(
       // Identify which renderer this update is coming from.
@@ -2128,7 +2132,10 @@ export function attach(
         // [TREE_OPERATION_REMOVE, removedIDLength, ...ids]
         (numUnmountIDs > 0 ? 2 + numUnmountIDs : 0) +
         // Regular operations
-        pendingOperations.length,
+        pendingOperations.length +
+        // All suspender changes are batched in a single message.
+        // [SUSPENSE_TREE_OPERATION_SUSPENDERS, suspenderChangesLength, ...[id, hasUniqueSuspenders]]
+        (numSuspenderChanges > 0 ? 2 + numSuspenderChanges * 2 : 0),
     );
 
     // Identify which renderer this update is coming from.
@@ -2191,19 +2198,39 @@ export function attach(
         i++;
       }
     }
-    // Fill in the rest of the operations.
+
+    // Fill in pending operations.
     for (let j = 0; j < pendingOperations.length; j++) {
       operations[i + j] = pendingOperations[j];
     }
     i += pendingOperations.length;
 
+    // Suspender changes might affect newly mounted nodes that we already recorded
+    // in pending operations.
+    if (numSuspenderChanges > 0) {
+      operations[i++] = SUSPENSE_TREE_OPERATION_SUSPENDERS;
+      operations[i++] = numSuspenderChanges;
+      pendingSuspenderChanges.forEach(fiberIdWithChanges => {
+        const suspense = idToSuspenseNodeMap.get(fiberIdWithChanges);
+        if (suspense === undefined) {
+          // Probably forgot to cleanup pendingSuspenderChanges when this node was removed.
+          throw new Error(
+            `Could not send suspender changes for "${fiberIdWithChanges}" since the Fiber no longer exists.`,
+          );
+        }
+        operations[i++] = fiberIdWithChanges;
+        operations[i++] = suspense.hasUniqueSuspenders ? 1 : 0;
+      });
+    }
+
     // Let the frontend know about tree operations.
     flushOrQueueOperations(operations);
 
     // Reset all of the pending state now that we've told the frontend about it.
     pendingOperations.length = 0;
     pendingRealUnmountedIDs.length = 0;
     pendingRealUnmountedSuspenseIDs.length = 0;
+    pendingSuspenderChanges.clear();
     pendingUnmountedRootID = null;
     pendingStringTable.clear();
     pendingStringTableLength = 0;
@@ -2688,6 +2715,19 @@ export function attach(
     }
   }
 
+  function recordSuspenseSuspenders(suspenseNode: SuspenseNode): void {
+    if (__DEBUG__) {
+      console.log('recordSuspenseSuspenders()', suspenseNode);
+    }
+    const fiberInstance = suspenseNode.instance;
+    if (fiberInstance.kind !== FIBER_INSTANCE) {
+      // TODO: Suspender updates of filtered Suspense nodes are currently dropped.
+      return;
+    }
+
+    pendingSuspenderChanges.add(fiberInstance.id);
+  }
+
   function recordSuspenseUnmount(suspenseInstance: SuspenseNode): void {
     if (__DEBUG__) {
       console.log(
@@ -2709,6 +2749,7 @@ export function attach(
     // and later arrange them in the correct order.
     pendingRealUnmountedSuspenseIDs.push(id);
 
+    pendingSuspenderChanges.delete(id);
     idToSuspenseNodeMap.delete(id);
   }
 
@@ -2779,6 +2820,7 @@ export function attach(
       ) {
         // This didn't exist in the parent before, so let's mark this boundary as having a unique suspender.
         parentSuspenseNode.hasUniqueSuspenders = true;
+        recordSuspenseSuspenders(parentSuspenseNode);
       }
     }
     // We have observed at least one known reason this might have been suspended.
@@ -2820,6 +2862,9 @@ export function attach(
         // We have found a child boundary that depended on the unblocked I/O.
         // It can now be marked as having unique suspenders. We can skip its children
         // since they'll still be blocked by this one.
+        if (!node.hasUniqueSuspenders) {
+          recordSuspenseSuspenders(node);
+        }
         node.hasUniqueSuspenders = true;
         node.hasUnknownSuspenders = false;
       } else if (node.firstChild !== null) {
@@ -3522,6 +3567,9 @@ export function attach(
       // Unfortunately if we don't have any DEV time debug info or debug thenables then
       // we have no meta data to show. However, we still mark this Suspense boundary as
       // participating in the loading sequence since apparently it can suspend.
+      if (!suspenseNode.hasUniqueSuspenders) {
+        recordSuspenseSuspenders(suspenseNode);
+      }
       suspenseNode.hasUniqueSuspenders = true;
       // We have not seen any reason yet for why this suspense node might have been
       // suspended but it clearly has been at some point. If we later discover a reason

packages/react-devtools-shared/src/constants.js

@@ -28,6 +28,7 @@ export const SUSPENSE_TREE_OPERATION_ADD = 8;
 export const SUSPENSE_TREE_OPERATION_REMOVE = 9;
 export const SUSPENSE_TREE_OPERATION_REORDER_CHILDREN = 10;
 export const SUSPENSE_TREE_OPERATION_RESIZE = 11;
+export const SUSPENSE_TREE_OPERATION_SUSPENDERS = 12;
 
 export const PROFILING_FLAG_BASIC_SUPPORT = 0b01;
 export const PROFILING_FLAG_TIMELINE_SUPPORT = 0b10;

packages/react-devtools-shared/src/devtools/store.js

@@ -24,6 +24,7 @@ import {
   SUSPENSE_TREE_OPERATION_REMOVE,
   SUSPENSE_TREE_OPERATION_REORDER_CHILDREN,
   SUSPENSE_TREE_OPERATION_RESIZE,
+  SUSPENSE_TREE_OPERATION_SUSPENDERS,
 } from '../constants';
 import {ElementTypeRoot} from '../frontend/types';
 import {
@@ -879,8 +880,13 @@ export default class Store extends EventEmitter<{
     return null;
   }
 
+  /**
+   * @param rootID
+   * @param uniqueSuspendersOnly Filters out boundaries without unique suspenders
+   */
   getSuspendableDocumentOrderSuspense(
     rootID: Element['id'] | void,
+    uniqueSuspendersOnly: boolean,
   ): $ReadOnlyArray<SuspenseNode['id']> {
     if (rootID === undefined) {
       return [];
@@ -892,7 +898,7 @@ export default class Store extends EventEmitter<{
     if (!this.supportsTogglingSuspense(root.id)) {
       return [];
     }
-    const suspenseTreeList: SuspenseNode['id'][] = [];
+    const list: SuspenseNode['id'][] = [];
     const suspense = this.getSuspenseByID(root.id);
     if (suspense !== null) {
       const stack = [suspense];
@@ -901,9 +907,11 @@ export default class Store extends EventEmitter<{
         if (current === undefined) {
           continue;
         }
-        // Include the root even if we won't suspend it.
+        // Include the root even if we won't show it suspended (because that's just blank).
         // You should be able to see what suspended the shell.
-        suspenseTreeList.push(current.id);
+        if (!uniqueSuspendersOnly || current.hasUniqueSuspenders) {
+          list.push(current.id);
+        }
         // Add children in reverse order to maintain document order
         for (let j = current.children.length - 1; j >= 0; j--) {
           const childSuspense = this.getSuspenseByID(current.children[j]);
@@ -914,7 +922,7 @@ export default class Store extends EventEmitter<{
       }
     }
 
-    return suspenseTreeList;
+    return list;
   }
 
   getRendererIDForElement(id: number): number | null {
@@ -1580,6 +1588,7 @@ export default class Store extends EventEmitter<{
             children: [],
             name,
             rects,
+            hasUniqueSuspenders: false,
           });
 
           hasSuspenseTreeChanged = true;
@@ -1749,6 +1758,42 @@ export default class Store extends EventEmitter<{
 
           break;
         }
+        case SUSPENSE_TREE_OPERATION_SUSPENDERS: {
+          const changeLength = operations[i + 1];
+          i += 2;
+
+          for (let changeIndex = 0; changeIndex < changeLength; changeIndex++) {
+            const id = operations[i];
+            const hasUniqueSuspenders = operations[i + 1] === 1;
+            const suspense = this._idToSuspense.get(id);
+
+            if (suspense === undefined) {
+              this._throwAndEmitError(
+                Error(
+                  `Cannot update suspenders of suspense node "${id}" because no matching node was found in the Store.`,
+                ),
+              );
+
+              break;
+            }
+
+            i += 2;
+
+            if (__DEBUG__) {
+              const previousHasUniqueSuspenders = suspense.hasUniqueSuspenders;
+              debug(
+                'Suspender changes',
+                `Suspense node ${id} unique suspenders set to ${String(hasUniqueSuspenders)} (was ${String(previousHasUniqueSuspenders)})`,
+              );
+            }
+
+            suspense.hasUniqueSuspenders = hasUniqueSuspenders;
+          }
+
+          hasSuspenseTreeChanged = true;
+
+          break;
+        }
         default:
           this._throwAndEmitError(
             new UnsupportedBridgeOperationError(

packages/react-devtools-shared/src/devtools/views/ButtonIcon.js

@@ -52,7 +52,7 @@ type Props = {
   type: IconType,
 };
 
-const materialIconsViewBox = '0 -960 960 960';
+const panelIcons = '0 -960 960 820';
 export default function ButtonIcon({className = '', type}: Props): React.Node {
   let pathData = null;
   let viewBox = '0 0 24 24';
@@ -131,27 +131,27 @@ export default function ButtonIcon({className = '', type}: Props): React.Node {
       break;
     case 'panel-left-close':
       pathData = PATH_MATERIAL_PANEL_LEFT_CLOSE;
-      viewBox = materialIconsViewBox;
+      viewBox = panelIcons;
       break;
     case 'panel-left-open':
       pathData = PATH_MATERIAL_PANEL_LEFT_OPEN;
-      viewBox = materialIconsViewBox;
+      viewBox = panelIcons;
       break;
     case 'panel-right-close':
       pathData = PATH_MATERIAL_PANEL_RIGHT_CLOSE;
-      viewBox = materialIconsViewBox;
+      viewBox = panelIcons;
       break;
     case 'panel-right-open':
       pathData = PATH_MATERIAL_PANEL_RIGHT_OPEN;
-      viewBox = materialIconsViewBox;
+      viewBox = panelIcons;
       break;
     case 'panel-bottom-open':
       pathData = PATH_MATERIAL_PANEL_BOTTOM_OPEN;
-      viewBox = materialIconsViewBox;
+      viewBox = panelIcons;
       break;
     case 'panel-bottom-close':
       pathData = PATH_MATERIAL_PANEL_BOTTOM_CLOSE;
-      viewBox = materialIconsViewBox;
+      viewBox = panelIcons;
       break;
     case 'suspend':
       pathData = PATH_SUSPEND;

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

@@ -20,6 +20,7 @@ import {
   SUSPENSE_TREE_OPERATION_REMOVE,
   SUSPENSE_TREE_OPERATION_REORDER_CHILDREN,
   SUSPENSE_TREE_OPERATION_RESIZE,
+  SUSPENSE_TREE_OPERATION_SUSPENDERS,
 } from 'react-devtools-shared/src/constants';
 import {
   parseElementDisplayNameFromBackend,
@@ -452,6 +453,18 @@ function updateTree(
         break;
       }
 
+      case SUSPENSE_TREE_OPERATION_SUSPENDERS: {
+        const changesLength = ((operations[i + 1]: any): number);
+
+        if (__DEBUG__) {
+          const changes = operations.slice(i + 2, i + 2 + changesLength * 2);
+          debug('Suspender changes', `[${changes.join(',')}]`);
+        }
+
+        i += 2 + changesLength * 2;
+        break;
+      }
+
       default:
         throw Error(`Unsupported Bridge operation "${operation}"`);
     }

packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTimeline.css

@@ -3,6 +3,7 @@
   display: flex;
   flex-direction: row;
   padding: 0.25rem;
+  align-items: center;
 }
 
 .SuspenseTimelineInput {