[DevTools] Avoid renders of stale Suspense store (#34396)

Author: eps1lon

packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseBreadcrumbs.js

@@ -8,20 +8,20 @@
  */
 
 import type {SuspenseNode} from 'react-devtools-shared/src/frontend/types';
+import typeof {SyntheticMouseEvent} from 'react-dom-bindings/src/events/SyntheticEvent';
 
 import * as React from 'react';
 import {useContext} from 'react';
 import {
   TreeDispatcherContext,
   TreeStateContext,
 } from '../Components/TreeContext';
-import {StoreContext} from '../context';
 import {useHighlightHostInstance} from '../hooks';
 import styles from './SuspenseBreadcrumbs.css';
-import typeof {SyntheticMouseEvent} from 'react-dom-bindings/src/events/SyntheticEvent';
+import {useSuspenseStore} from './SuspenseTreeContext';
 
 export default function SuspenseBreadcrumbs(): React$Node {
-  const store = useContext(StoreContext);
+  const store = useSuspenseStore();
   const dispatch = useContext(TreeDispatcherContext);
   const {inspectedElementID} = useContext(TreeStateContext);
 

packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseRects.js

@@ -19,10 +19,9 @@ import {
   TreeDispatcherContext,
   TreeStateContext,
 } from '../Components/TreeContext';
-import {StoreContext} from '../context';
 import {useHighlightHostInstance} from '../hooks';
 import styles from './SuspenseRects.css';
-import {SuspenseTreeStateContext} from './SuspenseTreeContext';
+import {useSuspenseStore} from './SuspenseTreeContext';
 import typeof {
   SyntheticMouseEvent,
   SyntheticPointerEvent,
@@ -46,7 +45,7 @@ function SuspenseRects({
   suspenseID: SuspenseNode['id'],
 }): React$Node {
   const dispatch = useContext(TreeDispatcherContext);
-  const store = useContext(StoreContext);
+  const store = useSuspenseStore();
 
   const {inspectedElementID} = useContext(TreeStateContext);
 
@@ -109,9 +108,9 @@ function SuspenseRects({
 
 function getDocumentBoundingRect(
   store: Store,
-  shells: $ReadOnlyArray<SuspenseNode['id']>,
+  roots: $ReadOnlyArray<SuspenseNode['id']>,
 ): Rect {
-  if (shells.length === 0) {
+  if (roots.length === 0) {
     return {x: 0, y: 0, width: 0, height: 0};
   }
 
@@ -120,14 +119,14 @@ function getDocumentBoundingRect(
   let maxX = Number.NEGATIVE_INFINITY;
   let maxY = Number.NEGATIVE_INFINITY;
 
-  for (let i = 0; i < shells.length; i++) {
-    const shellID = shells[i];
-    const shell = store.getSuspenseByID(shellID);
-    if (shell === null) {
+  for (let i = 0; i < roots.length; i++) {
+    const rootID = roots[i];
+    const root = store.getSuspenseByID(rootID);
+    if (root === null) {
       continue;
     }
 
-    const rects = shell.rects;
+    const rects = root.rects;
     if (rects === null) {
       continue;
     }
@@ -154,32 +153,32 @@ function getDocumentBoundingRect(
 }
 
 function SuspenseRectsShell({
-  shellID,
+  rootID,
 }: {
-  shellID: SuspenseNode['id'],
+  rootID: SuspenseNode['id'],
 }): React$Node {
-  const store = useContext(StoreContext);
-  const shell = store.getSuspenseByID(shellID);
-  if (shell === null) {
-    console.warn(`<Element> Could not find suspense node id ${shellID}`);
+  const store = useSuspenseStore();
+  const root = store.getSuspenseByID(rootID);
+  if (root === null) {
+    console.warn(`<Element> Could not find suspense node id ${rootID}`);
     return null;
   }
 
   return (
     <g>
-      {shell.children.map(childID => {
+      {root.children.map(childID => {
         return <SuspenseRects key={childID} suspenseID={childID} />;
       })}
     </g>
   );
 }
 
 function SuspenseRectsContainer(): React$Node {
-  const store = useContext(StoreContext);
+  const store = useSuspenseStore();
   // TODO: This relies on a full re-render of all children when the Suspense tree changes.
-  const {shells} = useContext(SuspenseTreeStateContext);
+  const roots = store.roots;
 
-  const boundingRect = getDocumentBoundingRect(store, shells);
+  const boundingRect = getDocumentBoundingRect(store, roots);
 
   const width = '100%';
   const boundingRectWidth = boundingRect.width;
@@ -193,8 +192,8 @@ function SuspenseRectsContainer(): React$Node {
       <svg
         style={{width, height}}
         viewBox={`${boundingRect.x} ${boundingRect.y} ${boundingRect.width} ${boundingRect.height}`}>
-        {shells.map(shellID => {
-          return <SuspenseRectsShell key={shellID} shellID={shellID} />;
+        {roots.map(rootID => {
+          return <SuspenseRectsShell key={rootID} rootID={rootID} />;
         })}
       </svg>
     </div>

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

@@ -12,10 +12,10 @@ import type Store from '../../store';
 
 import * as React from 'react';
 import {useContext, useLayoutEffect, useMemo, useRef, useState} from 'react';
-import {BridgeContext, StoreContext} from '../context';
+import {BridgeContext} from '../context';
 import {TreeDispatcherContext} from '../Components/TreeContext';
 import {useHighlightHostInstance} from '../hooks';
-import {SuspenseTreeStateContext} from './SuspenseTreeContext';
+import {useSuspenseStore} from './SuspenseTreeContext';
 import styles from './SuspenseTimeline.css';
 import typeof {
   SyntheticEvent,
@@ -63,14 +63,14 @@ function getSuspendableDocumentOrderSuspense(
 
 function SuspenseTimelineInput({rootID}: {rootID: Element['id'] | void}) {
   const bridge = useContext(BridgeContext);
-  const store = useContext(StoreContext);
+  const store = useSuspenseStore();
   const dispatch = useContext(TreeDispatcherContext);
   const {highlightHostInstance, clearHighlightHostInstance} =
     useHighlightHostInstance();
 
   const timeline = useMemo(() => {
     return getSuspendableDocumentOrderSuspense(store, rootID);
-  }, [store, rootID]);
+  }, [store, store.revisionSuspense, rootID]);
 
   const inputRef = useRef<HTMLElement | null>(null);
   const inputBBox = useRef<ClientRect | null>(null);
@@ -161,6 +161,7 @@ function SuspenseTimelineInput({rootID}: {rootID: Element['id'] | void}) {
   function handleFocus() {
     const suspense = timeline[value];
 
+    dispatch({type: 'SELECT_ELEMENT_BY_ID', payload: suspense.id});
     highlightHostInstance(suspense.id);
   }
 
@@ -213,10 +214,10 @@ function SuspenseTimelineInput({rootID}: {rootID: Element['id'] | void}) {
 }
 
 export default function SuspenseTimeline(): React$Node {
-  const store = useContext(StoreContext);
-  const {shells} = useContext(SuspenseTreeStateContext);
+  const store = useSuspenseStore();
 
-  const defaultSelectedRootID = shells.find(rootID => {
+  const roots = store.roots;
+  const defaultSelectedRootID = roots.find(rootID => {
     const suspense = store.getSuspenseByID(rootID);
     return (
       store.supportsTogglingSuspense(rootID) &&
@@ -239,20 +240,18 @@ export default function SuspenseTimeline(): React$Node {
   return (
     <div className={styles.SuspenseTimelineContainer}>
       <SuspenseTimelineInput key={selectedRootID} rootID={selectedRootID} />
-      {shells.length > 0 && (
+      {roots.length > 0 && (
         <select
           aria-label="Select Suspense Root"
           className={styles.SuspenseTimelineRootSwitcher}
-          onChange={handleChange}>
-          {shells.map(rootID => {
+          onChange={handleChange}
+          value={selectedRootID}>
+          {roots.map(rootID => {
             // TODO: Use name
             const name = '#' + rootID;
             // TODO: Highlight host on hover
             return (
-              <option
-                key={rootID}
-                selected={rootID === selectedRootID}
-                value={rootID}>
+              <option key={rootID} value={rootID}>
                 {name}
               </option>
             );

packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTreeContext.js

@@ -7,6 +7,7 @@
  * @flow
  */
 import type {ReactContext} from 'shared/ReactTypes';
+import type Store from '../../store';
 
 import * as React from 'react';
 import {
@@ -17,17 +18,12 @@ import {
   useMemo,
   useReducer,
 } from 'react';
-import type {SuspenseNode} from '../../../frontend/types';
 import {StoreContext} from '../context';
 
-export type SuspenseTreeState = {
-  shells: $ReadOnlyArray<SuspenseNode['id']>,
-};
+export type SuspenseTreeState = {};
 
-type ACTION_HANDLE_SUSPENSE_TREE_MUTATION = {
-  type: 'HANDLE_SUSPENSE_TREE_MUTATION',
-};
-export type SuspenseTreeAction = ACTION_HANDLE_SUSPENSE_TREE_MUTATION;
+// unused for now
+export type SuspenseTreeAction = {type: 'unused'};
 export type SuspenseTreeDispatch = (action: SuspenseTreeAction) => void;
 
 const SuspenseTreeStateContext: ReactContext<SuspenseTreeState> =
@@ -42,11 +38,39 @@ type Props = {
   children: React$Node,
 };
 
-function SuspenseTreeContextController({children}: Props): React.Node {
+/**
+ * The Store is mutable. This Hook ensures renders read the latest Suspense related
+ * data.
+ */
+function useSuspenseStore(): Store {
   const store = useContext(StoreContext);
-
+  const [, storeUpdated] = useReducer<number, number, void>(
+    (x: number) => (x + 1) % Number.MAX_SAFE_INTEGER,
+    0,
+  );
   const initialRevision = useMemo(() => store.revisionSuspense, [store]);
+  // We're currently storing everything Suspense related in the same Store as
+  // Components. However, most reads are currently stateless. This ensures
+  // the latest state is always read from the Store.
+  useEffect(() => {
+    const handleSuspenseTreeMutated = () => {
+      storeUpdated();
+    };
+
+    // Since this is a passive effect, the tree may have been mutated before our initial subscription.
+    if (store.revisionSuspense !== initialRevision) {
+      // At the moment, we can treat this as a mutation.
+      handleSuspenseTreeMutated();
+    }
+
+    store.addListener('suspenseTreeMutated', handleSuspenseTreeMutated);
+    return () =>
+      store.removeListener('suspenseTreeMutated', handleSuspenseTreeMutated);
+  }, [initialRevision, store]);
+  return store;
+}
 
+function SuspenseTreeContextController({children}: Props): React.Node {
   // This reducer is created inline because it needs access to the Store.
   // The store is mutable, but the Store itself is global and lives for the lifetime of the DevTools,
   // so it's okay for the reducer to have an empty dependencies array.
@@ -58,18 +82,14 @@ function SuspenseTreeContextController({children}: Props): React.Node {
       ): SuspenseTreeState => {
         const {type} = action;
         switch (type) {
-          case 'HANDLE_SUSPENSE_TREE_MUTATION':
-            return {...state, shells: store.roots};
           default:
             throw new Error(`Unrecognized action "${type}"`);
         }
       },
     [],
   );
 
-  const initialState: SuspenseTreeState = {
-    shells: store.roots,
-  };
+  const initialState: SuspenseTreeState = {};
   const [state, dispatch] = useReducer(reducer, initialState);
   const transitionDispatch = useMemo(
     () => (action: SuspenseTreeAction) =>
@@ -79,28 +99,6 @@ function SuspenseTreeContextController({children}: Props): React.Node {
     [dispatch],
   );
 
-  useEffect(() => {
-    const handleSuspenseTreeMutated = () => {
-      dispatch({
-        type: 'HANDLE_SUSPENSE_TREE_MUTATION',
-      });
-    };
-
-    // Since this is a passive effect, the tree may have been mutated before our initial subscription.
-    if (store.revisionSuspense !== initialRevision) {
-      // At the moment, we can treat this as a mutation.
-      // We don't know which Elements were newly added/removed, but that should be okay in this case.
-      // It would only impact the search state, which is unlikely to exist yet at this point.
-      dispatch({
-        type: 'HANDLE_SUSPENSE_TREE_MUTATION',
-      });
-    }
-
-    store.addListener('suspenseTreeMutated', handleSuspenseTreeMutated);
-    return () =>
-      store.removeListener('suspenseTreeMutated', handleSuspenseTreeMutated);
-  }, [dispatch, initialRevision, store]);
-
   return (
     <SuspenseTreeStateContext.Provider value={state}>
       <SuspenseTreeDispatcherContext.Provider value={transitionDispatch}>
@@ -114,4 +112,5 @@ export {
   SuspenseTreeDispatcherContext,
   SuspenseTreeStateContext,
   SuspenseTreeContextController,
+  useSuspenseStore,
 };