DevTools: Replace legacy Suspense cache with unstable_getCacheForType

Author: Brian Vaughn

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

@@ -12,6 +12,8 @@ import type {Thenable} from 'shared/ReactTypes';
 import * as React from 'react';
 import {createContext} from 'react';
 
+// TODO (cache) Remove this cache; it is outdated and will not work with newer APIs like startTransition.
+
 // Cache implementation was forked from the React repo:
 // https://github.com/facebook/react/blob/master/packages/react-cache/src/ReactCache.js
 //

packages/react-devtools-shared/src/devtools/views/Components/InspectedElement.js

@@ -39,6 +39,8 @@ export default function InspectedElementWrapper(_: Props) {
   const {dispatch: modalDialogDispatch} = useContext(ModalDialogContext);
 
   const {
+    clearErrorsForInspectedElement,
+    clearWarningsForInspectedElement,
     copyInspectedElementPath,
     getInspectedElementPath,
     getInspectedElement,
@@ -228,6 +230,8 @@ export default function InspectedElementWrapper(_: Props) {
           key={
             inspectedElementID /* Force reset when selected Element changes */
           }
+          clearErrorsForInspectedElement={clearErrorsForInspectedElement}
+          clearWarningsForInspectedElement={clearWarningsForInspectedElement}
           copyInspectedElementPath={copyInspectedElementPath}
           element={element}
           getInspectedElementPath={getInspectedElementPath}

packages/react-devtools-shared/src/devtools/views/Components/InspectedElementContext.js

@@ -10,6 +10,9 @@
 import * as React from 'react';
 import {
   createContext,
+  unstable_getCacheForType as getCacheForType,
+  unstable_startTransition as startTransition,
+  unstable_useCacheRefresh as useCacheRefresh,
   useCallback,
   useContext,
   useEffect,
@@ -18,7 +21,6 @@ import {
   useState,
 } from 'react';
 import {unstable_batchedUpdates as batchedUpdates} from 'react-dom';
-import {createResource} from '../../cache';
 import {BridgeContext, StoreContext} from '../context';
 import {hydrate, fillInPath} from 'react-devtools-shared/src/hydration';
 import {TreeStateContext} from './TreeContext';
@@ -33,7 +35,6 @@ import type {
   Element,
   InspectedElement as InspectedElementFrontend,
 } from 'react-devtools-shared/src/devtools/views/Components/types';
-import type {Resource, Thenable} from '../../cache';
 
 export type StoreAsGlobal = (id: number, path: Array<string | number>) => void;
 
@@ -51,13 +52,15 @@ export type GetInspectedElement = (
   id: number,
 ) => InspectedElementFrontend | null;
 
-type RefreshInspectedElement = () => void;
+type ClearErrorsForInspectedElement = () => void;
+type ClearWarningsForInspectedElement = () => void;
 
 export type InspectedElementContextType = {|
+  clearErrorsForInspectedElement: ClearErrorsForInspectedElement,
+  clearWarningsForInspectedElement: ClearWarningsForInspectedElement,
   copyInspectedElementPath: CopyInspectedElementPath,
   getInspectedElementPath: GetInspectedElementPath,
   getInspectedElement: GetInspectedElement,
-  refreshInspectedElement: RefreshInspectedElement,
   storeAsGlobal: StoreAsGlobal,
 |};
 
@@ -67,35 +70,21 @@ const InspectedElementContext = createContext<InspectedElementContextType>(
 InspectedElementContext.displayName = 'InspectedElementContext';
 
 type ResolveFn = (inspectedElement: InspectedElementFrontend) => void;
-type InProgressRequest = {|
-  promise: Thenable<InspectedElementFrontend>,
-  resolveFn: ResolveFn,
+type Callback = (inspectedElement: InspectedElementFrontend) => void;
+type Thenable = {|
+  callbacks: Set<Callback>,
+  then: (callback: Callback) => void,
+  resolve: ResolveFn,
 |};
 
-const inProgressRequests: WeakMap<Element, InProgressRequest> = new WeakMap();
-const resource: Resource<
-  Element,
+const inspectedElementThenables: WeakMap<Element, Thenable> = new WeakMap();
+
+function createInspectedElementCache(): WeakMap<
   Element,
   InspectedElementFrontend,
-> = createResource(
-  (element: Element) => {
-    const request = inProgressRequests.get(element);
-    if (request != null) {
-      return request.promise;
-    }
-
-    let resolveFn = ((null: any): ResolveFn);
-    const promise = new Promise(resolve => {
-      resolveFn = resolve;
-    });
-
-    inProgressRequests.set(element, {promise, resolveFn});
-
-    return promise;
-  },
-  (element: Element) => element,
-  {useWeakMap: true},
-);
+> {
+  return new WeakMap();
+}
 
 type Props = {|
   children: React$Node,
@@ -145,28 +134,75 @@ function InspectedElementContextController({children}: Props) {
     [bridge, store],
   );
 
+  // TODO (cache) Better encapsulate the cache and read/write methods.
+  const inspectedElementCache = getCacheForType(createInspectedElementCache);
+
   const getInspectedElement = useCallback<GetInspectedElement>(
     (id: number) => {
       const element = store.getElementByID(id);
       if (element !== null) {
-        return resource.read(element);
-      } else {
-        return null;
+        const maybeInspectedElement = inspectedElementCache.get(element);
+        if (maybeInspectedElement !== undefined) {
+          return maybeInspectedElement;
+        } else {
+          const maybeThenable = inspectedElementThenables.get(element);
+          if (maybeThenable != null) {
+            throw maybeThenable;
+          }
+
+          const thenable: Thenable = {
+            callbacks: new Set(),
+            then: callback => {
+              thenable.callbacks.add(callback);
+            },
+            resolve: inspectedElement => {
+              thenable.callbacks.forEach(callback =>
+                callback(inspectedElement),
+              );
+            },
+          };
+
+          inspectedElementThenables.set(element, thenable);
+
+          throw thenable;
+        }
       }
+      return null;
     },
-    [store],
+    [inspectedElementCache, store],
   );
 
   // It's very important that this context consumes selectedElementID and not inspectedElementID.
   // Otherwise the effect that sends the "inspect" message across the bridge-
   // would itself be blocked by the same render that suspends (waiting for the data).
   const {selectedElementID} = useContext(TreeStateContext);
 
-  const refreshInspectedElement = useCallback<RefreshInspectedElement>(() => {
+  const refresh = useCacheRefresh();
+
+  const clearErrorsForInspectedElement = useCallback<ClearErrorsForInspectedElement>(() => {
     if (selectedElementID !== null) {
       const rendererID = store.getRendererIDForElement(selectedElementID);
       if (rendererID !== null) {
         bridge.send('inspectElement', {id: selectedElementID, rendererID});
+
+        startTransition(() => {
+          store.clearErrorsForElement(selectedElementID);
+          refresh();
+        });
+      }
+    }
+  }, [bridge, selectedElementID]);
+
+  const clearWarningsForInspectedElement = useCallback<ClearWarningsForInspectedElement>(() => {
+    if (selectedElementID !== null) {
+      const rendererID = store.getRendererIDForElement(selectedElementID);
+      if (rendererID !== null) {
+        bridge.send('inspectElement', {id: selectedElementID, rendererID});
+
+        startTransition(() => {
+          store.clearWarningsForElement(selectedElementID);
+          refresh();
+        });
       }
     }
   }, [bridge, selectedElementID]);
@@ -198,7 +234,9 @@ function InspectedElementContextController({children}: Props) {
 
               fillInPath(inspectedElement, data.value, data.path, value);
 
-              resource.write(element, inspectedElement);
+              // TODO (cache) This mutation seems sketchy.
+              // Probably need to refresh the cache with a new seed.
+              inspectedElementCache.set(element, inspectedElement);
 
               // Schedule update with React if the currently-selected element has been invalidated.
               if (id === selectedElementID) {
@@ -277,16 +315,17 @@ function InspectedElementContextController({children}: Props) {
 
           element = store.getElementByID(id);
           if (element !== null) {
-            const request = inProgressRequests.get(element);
-            if (request != null) {
-              inProgressRequests.delete(element);
+            inspectedElementCache.set(element, inspectedElement);
+
+            const thenable = inspectedElementThenables.get(element);
+            if (thenable != null) {
+              inspectedElementThenables.delete(element);
+
               batchedUpdates(() => {
-                request.resolveFn(inspectedElement);
+                thenable.resolve(inspectedElement);
                 setCurrentlyInspectedElement(inspectedElement);
               });
             } else {
-              resource.write(element, inspectedElement);
-
               // Schedule update with React if the currently-selected element has been invalidated.
               if (id === selectedElementID) {
                 setCurrentlyInspectedElement(inspectedElement);
@@ -356,19 +395,21 @@ function InspectedElementContextController({children}: Props) {
 
   const value = useMemo(
     () => ({
+      clearErrorsForInspectedElement,
+      clearWarningsForInspectedElement,
       copyInspectedElementPath,
       getInspectedElement,
       getInspectedElementPath,
-      refreshInspectedElement,
       storeAsGlobal,
     }),
     // InspectedElement is used to invalidate the cache and schedule an update with React.
     [
+      clearErrorsForInspectedElement,
+      clearWarningsForInspectedElement,
       copyInspectedElementPath,
       currentlyInspectedElement,
       getInspectedElement,
       getInspectedElementPath,
-      refreshInspectedElement,
       storeAsGlobal,
     ],
   );

packages/react-devtools-shared/src/devtools/views/Components/InspectedElementErrorsAndWarningsTree.css

@@ -58,3 +58,7 @@
   background-color: var(--color-console-warning-icon);
   color: var(--color-console-warning-badge-text);
 }
+
+.Pending {
+  filter: grayscale(0.75);
+}

packages/react-devtools-shared/src/devtools/views/Components/InspectedElementErrorsAndWarningsTree.js

@@ -8,7 +8,7 @@
  */
 
 import * as React from 'react';
-import {useContext} from 'react';
+import {useContext, unstable_useTransition as useTransition} from 'react';
 import Button from '../Button';
 import ButtonIcon from '../ButtonIcon';
 import Store from '../../store';
@@ -31,7 +31,10 @@ export default function InspectedElementErrorsAndWarningsTree({
   inspectedElement,
   store,
 }: Props) {
-  const {refreshInspectedElement} = useContext(InspectedElementContext);
+  const {
+    clearErrorsForInspectedElement,
+    clearWarningsForInspectedElement,
+  } = useContext(InspectedElementContext);
 
   const {showInlineWarningsAndErrors} = useContext(SettingsContext);
   if (!showInlineWarningsAndErrors) {
@@ -40,34 +43,14 @@ export default function InspectedElementErrorsAndWarningsTree({
 
   const {errors, warnings} = inspectedElement;
 
-  const clearErrors = () => {
-    const {id} = inspectedElement;
-    store.clearErrorsForElement(id);
-
-    // Immediately poll for updated data.
-    // This avoids a delay between clicking the clear button and refreshing errors.
-    // Ideally this would be done with useTranstion but that requires updating to a newer Cache strategy.
-    refreshInspectedElement();
-  };
-
-  const clearWarnings = () => {
-    const {id} = inspectedElement;
-    store.clearWarningsForElement(id);
-
-    // Immediately poll for updated data.
-    // This avoids a delay between clicking the clear button and refreshing warnings.
-    // Ideally this would be done with useTranstion but that requires updating to a newer Cache strategy.
-    refreshInspectedElement();
-  };
-
   return (
     <React.Fragment>
       {errors.length > 0 && (
         <Tree
           badgeClassName={styles.ErrorBadge}
           bridge={bridge}
           className={styles.ErrorTree}
-          clearMessages={clearErrors}
+          clearMessages={clearErrorsForInspectedElement}
           entries={errors}
           label="errors"
           messageClassName={styles.Error}
@@ -78,7 +61,7 @@ export default function InspectedElementErrorsAndWarningsTree({
           badgeClassName={styles.WarningBadge}
           bridge={bridge}
           className={styles.WarningTree}
-          clearMessages={clearWarnings}
+          clearMessages={clearWarningsForInspectedElement}
           entries={warnings}
           label="warnings"
           messageClassName={styles.Warning}
@@ -107,6 +90,8 @@ function Tree({
   label,
   messageClassName,
 }: TreeProps) {
+  const [startTransition, isPending] = useTransition();
+
   if (entries.length === 0) {
     return null;
   }
@@ -115,7 +100,12 @@ function Tree({
       <div className={`${sharedStyles.HeaderRow} ${styles.HeaderRow}`}>
         <div className={sharedStyles.Header}>{label}</div>
         <Button
-          onClick={clearMessages}
+          disabled={isPending}
+          onClick={() =>
+            startTransition(() => {
+              clearMessages();
+            })
+          }
           title={`Clear all ${label} for this component`}>
           <ButtonIcon type="clear" />
         </Button>
@@ -126,6 +116,7 @@ function Tree({
           badgeClassName={badgeClassName}
           className={messageClassName}
           count={count}
+          isPending={isPending}
           message={message}
         />
       ))}
@@ -137,17 +128,19 @@ type ErrorOrWarningViewProps = {|
   badgeClassName: string,
   className: string,
   count: number,
+  isPending: boolean,
   message: string,
 |};
 
 function ErrorOrWarningView({
   className,
   badgeClassName,
   count,
+  isPending,
   message,
 }: ErrorOrWarningViewProps) {
   return (
-    <div className={className}>
+    <div className={`${className} ${isPending ? styles.Pending : ''}`}>
       {count > 1 && <div className={badgeClassName}>{count}</div>}
       <div className={styles.Message} title={message}>
         {message}