DevTools: Replace legacy Suspense cache with unstable_getCacheForType

facebook · Jan 5, 2021 · 35f2ab7 · 35f2ab7
1 parent 2765955
commit 35f2ab7
Show file tree

Hide file tree

Showing 4 changed files with 131 additions and 77 deletions.
diff --git a/packages/react-devtools-shared/src/devtools/cache.js b/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
 //

diff --git a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElement.js b/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}

diff --git a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementContext.js b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementContext.js
@@ -10,15 +10,16 @@
 import * as React from 'react';
 import {
   createContext,
+  unstable_getCacheForType as getCacheForType,
+  unstable_startTransition as startTransition,
+  unstable_useCacheRefresh as useCacheRefresh,
   useCallback,
   useContext,
   useEffect,
   useMemo,
   useRef,
   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 +34,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 +51,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 +69,68 @@ 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,
-  Element,
-  InspectedElementFrontend,
-> = createResource(
-  (element: Element) => {
-    const request = inProgressRequests.get(element);
-    if (request != null) {
-      return request.promise;
-    }
+const inspectedElementThenables: WeakMap<Element, Thenable> = new WeakMap();
 
-    let resolveFn = ((null: any): ResolveFn);
-    const promise = new Promise(resolve => {
-      resolveFn = resolve;
-    });
+type InspectedElementCache = WeakMap<Element, InspectedElementFrontend>;
 
-    inProgressRequests.set(element, {promise, resolveFn});
+function createInspectedElementCache(): InspectedElementCache {
+  return new WeakMap();
+}
 
-    return promise;
-  },
-  (element: Element) => element,
-  {useWeakMap: true},
-);
+function getInspectedElementCache(): InspectedElementCache {
+  return getCacheForType(createInspectedElementCache);
+}
+
+function setInspectedElement(
+  element: Element,
+  inspectedElement: InspectedElementFrontend,
+  inspectedElementCache: InspectedElementCache,
+): void {
+  // TODO (cache) This mutation seems sketchy.
+  // Probably need to refresh the cache with a new seed.
+  inspectedElementCache.set(element, inspectedElement);
+
+  const maybeThenable = inspectedElementThenables.get(element);
+  if (maybeThenable != null) {
+    inspectedElementThenables.delete(element);
+
+    maybeThenable.resolve(inspectedElement);
+  }
+}
+
+function getInspectedElement(element: Element): InspectedElementFrontend {
+  const inspectedElementCache = getInspectedElementCache();
+  const maybeInspectedElement = inspectedElementCache.get(element);
+  if (maybeInspectedElement !== undefined) {
+    return maybeInspectedElement;
+  }
+
+  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;
+}
 
 type Props = {|
   children: React$Node,
@@ -145,14 +180,13 @@ function InspectedElementContextController({children}: Props) {
     [bridge, store],
   );
 
-  const getInspectedElement = useCallback<GetInspectedElement>(
+  const getInspectedElementWrapper = useCallback<GetInspectedElement>(
     (id: number) => {
       const element = store.getElementByID(id);
       if (element !== null) {
-        return resource.read(element);
-      } else {
-        return null;
+        return getInspectedElement(element);
       }
+      return null;
     },
     [store],
   );
@@ -162,11 +196,32 @@ function InspectedElementContextController({children}: Props) {
   // 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]);
@@ -176,6 +231,8 @@ function InspectedElementContextController({children}: Props) {
     setCurrentlyInspectedElement,
   ] = useState<InspectedElementFrontend | null>(null);
 
+  const inspectedElementCache = getInspectedElementCache();
+
   // This effect handler invalidates the suspense cache and schedules rendering updates with React.
   useEffect(() => {
     const onInspectedElement = (data: InspectedElementPayload) => {
@@ -198,7 +255,11 @@ function InspectedElementContextController({children}: Props) {
 
               fillInPath(inspectedElement, data.value, data.path, value);
 
-              resource.write(element, inspectedElement);
+              setInspectedElement(
+                element,
+                inspectedElement,
+                inspectedElementCache,
+              );
 
               // Schedule update with React if the currently-selected element has been invalidated.
               if (id === selectedElementID) {
@@ -277,20 +338,15 @@ function InspectedElementContextController({children}: Props) {
 
           element = store.getElementByID(id);
           if (element !== null) {
-            const request = inProgressRequests.get(element);
-            if (request != null) {
-              inProgressRequests.delete(element);
-              batchedUpdates(() => {
-                request.resolveFn(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);
-              }
+            setInspectedElement(
+              element,
+              inspectedElement,
+              inspectedElementCache,
+            );
+
+            // Schedule update with React if the currently-selected element has been invalidated.
+            if (id === selectedElementID) {
+              setCurrentlyInspectedElement(inspectedElement);
             }
           }
           break;
@@ -356,19 +412,21 @@ function InspectedElementContextController({children}: Props) {
 
   const value = useMemo(
     () => ({
+      clearErrorsForInspectedElement,
+      clearWarningsForInspectedElement,
       copyInspectedElementPath,
-      getInspectedElement,
+      getInspectedElement: getInspectedElementWrapper,
       getInspectedElementPath,
-      refreshInspectedElement,
       storeAsGlobal,
     }),
     // InspectedElement is used to invalidate the cache and schedule an update with React.
     [
+      clearErrorsForInspectedElement,
+      clearWarningsForInspectedElement,
       copyInspectedElementPath,
       currentlyInspectedElement,
       getInspectedElement,
       getInspectedElementPath,
-      refreshInspectedElement,
       storeAsGlobal,
     ],
   );

diff --git a/...ct-devtools-shared/src/devtools/views/Components/InspectedElementErrorsAndWarningsTree.js b/...ct-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>