DevTools: refactor NativeStyleEditor, don't use custom cache implementation (#32298)

We have this really old (5+ years) feature for inspecting native styles
of React Native Host components.

We also have a custom Cache implementation in React DevTools, which was
forked from React at some point. We know that this should be removed,
but it spans through critical parts of the application, like fetching
and caching inspected element.

Before this PR, this was also used for caching native style and layouts
of RN Host components. This approach is out of date, and was based on
the presence of Suspense boundary around inspected element View, which
we have removed to speed up element inspection -
https://github.com/facebook/react/pull/30555.
 
Looks like I've introduced a regression in
https://github.com/facebook/react/pull/31956:
- Custom Cache implementation will throw thenables and suspend.
- Because of this, some descendant Suspense boundaries will not resolve
for a long time, and React will throw an error
https://react.dev/errors/482.

I've switched from a usage of this custom Cache implementation to a
naive fetching in effect and keeping the layout and style in a local
state of a Context, which will be propagated downwards. The race should
be impossible, this is guaranteed by the mechanism for queueing messages
through microtasks queue.

The only downside is the UI. If you quickly switch between 2 elements,
and one of them has native style, while the other doesn't, UI will feel
jumpy. We can address this later with a Suspense boundary, if needed.

packages/react-devtools-inline/__tests__/__e2e__/components.test.js

@@ -212,8 +212,8 @@ test.describe('Components', () => {
   });
 
   test('should allow searching for component by name', async () => {
-    async function getComponentSearchResultsCount() {
-      return await page.evaluate(() => {
+    async function waitForComponentSearchResultsCount(text) {
+      return await page.waitForFunction(expectedElementText => {
         const {createTestNameSelector, findAllNodes} =
           window.REACT_DOM_DEVTOOLS;
         const container = document.getElementById('devtools');
@@ -221,8 +221,10 @@ test.describe('Components', () => {
         const element = findAllNodes(container, [
           createTestNameSelector('ComponentSearchInput-ResultsCount'),
         ])[0];
-        return element.innerText;
-      });
+        return element !== undefined
+          ? element.innerText === expectedElementText
+          : false;
+      }, text);
     }
 
     async function focusComponentSearch() {
@@ -238,35 +240,27 @@ test.describe('Components', () => {
 
     await focusComponentSearch();
     await page.keyboard.insertText('List');
-    let count = await getComponentSearchResultsCount();
-    expect(count).toBe('1 | 4');
+    await waitForComponentSearchResultsCount('1 | 4');
 
     await page.keyboard.insertText('Item');
-    count = await getComponentSearchResultsCount();
-    expect(count).toBe('1 | 3');
+    await waitForComponentSearchResultsCount('1 | 3');
 
     await page.keyboard.press('Enter');
-    count = await getComponentSearchResultsCount();
-    expect(count).toBe('2 | 3');
+    await waitForComponentSearchResultsCount('2 | 3');
 
     await page.keyboard.press('Enter');
-    count = await getComponentSearchResultsCount();
-    expect(count).toBe('3 | 3');
+    await waitForComponentSearchResultsCount('3 | 3');
 
     await page.keyboard.press('Enter');
-    count = await getComponentSearchResultsCount();
-    expect(count).toBe('1 | 3');
+    await waitForComponentSearchResultsCount('1 | 3');
 
     await page.keyboard.press('Shift+Enter');
-    count = await getComponentSearchResultsCount();
-    expect(count).toBe('3 | 3');
+    await waitForComponentSearchResultsCount('3 | 3');
 
     await page.keyboard.press('Shift+Enter');
-    count = await getComponentSearchResultsCount();
-    expect(count).toBe('2 | 3');
+    await waitForComponentSearchResultsCount('2 | 3');
 
     await page.keyboard.press('Shift+Enter');
-    count = await getComponentSearchResultsCount();
-    expect(count).toBe('1 | 3');
+    await waitForComponentSearchResultsCount('1 | 3');
   });
 });

packages/react-devtools-shared/src/devtools/views/Components/NativeStyleEditor/LayoutViewer.css

@@ -1,6 +1,5 @@
 .LayoutViewer {
   padding: 0.25rem;
-  border-top: 1px solid var(--color-border);
   font-family: var(--font-family-monospace);
   font-size: var(--font-size-monospace-small);
 }

packages/react-devtools-shared/src/devtools/views/Components/NativeStyleEditor/StyleEditor.css

@@ -2,7 +2,6 @@
   font-family: var(--font-family-monospace);
   font-size: var(--font-size-monospace-normal);
   padding: 0.25rem;
-  border-top: 1px solid var(--color-border);
 }
 
 .HeaderRow {

packages/react-devtools-shared/src/devtools/views/Components/NativeStyleEditor/StyleEditor.js

@@ -81,7 +81,7 @@ export default function StyleEditor({id, style}: Props): React.Node {
       {keys.length > 0 &&
         keys.map(attribute => (
           <Row
-            key={attribute}
+            key={`${attribute}/${style[attribute]}`}
             attribute={attribute}
             changeAttribute={changeAttribute}
             changeValue={changeValue}

packages/react-devtools-shared/src/devtools/views/Components/NativeStyleEditor/context.js

@@ -10,75 +10,26 @@
 import type {ReactContext} from 'shared/ReactTypes';
 
 import * as React from 'react';
-import {
-  createContext,
-  useCallback,
-  useContext,
-  useEffect,
-  useMemo,
-  useState,
-} from 'react';
-import {createResource} from 'react-devtools-shared/src/devtools/cache';
+import {createContext, useContext, useEffect, useState} from 'react';
 import {
   BridgeContext,
   StoreContext,
 } from 'react-devtools-shared/src/devtools/views/context';
-import {TreeStateContext} from '../TreeContext';
+import {TreeStateContext} from 'react-devtools-shared/src/devtools/views/Components/TreeContext';
 
-import type {StateContext} from '../TreeContext';
+import type {StateContext} from 'react-devtools-shared/src/devtools/views/Components/TreeContext';
 import type {FrontendBridge} from 'react-devtools-shared/src/bridge';
 import type Store from 'react-devtools-shared/src/devtools/store';
 import type {StyleAndLayout as StyleAndLayoutBackend} from 'react-devtools-shared/src/backend/NativeStyleEditor/types';
 import type {StyleAndLayout as StyleAndLayoutFrontend} from './types';
-import type {Element} from 'react-devtools-shared/src/frontend/types';
-import type {
-  Resource,
-  Thenable,
-} from 'react-devtools-shared/src/devtools/cache';
 
-export type GetStyleAndLayout = (id: number) => StyleAndLayoutFrontend | null;
-
-type Context = {
-  getStyleAndLayout: GetStyleAndLayout,
-};
+type Context = StyleAndLayoutFrontend | null;
 
 const NativeStyleContext: ReactContext<Context> = createContext<Context>(
   ((null: any): Context),
 );
 NativeStyleContext.displayName = 'NativeStyleContext';
 
-type ResolveFn = (styleAndLayout: StyleAndLayoutFrontend) => void;
-type InProgressRequest = {
-  promise: Thenable<StyleAndLayoutFrontend>,
-  resolveFn: ResolveFn,
-};
-
-const inProgressRequests: WeakMap<Element, InProgressRequest> = new WeakMap();
-const resource: Resource<Element, Element, StyleAndLayoutFrontend> =
-  createResource(
-    (element: Element) => {
-      const request = inProgressRequests.get(element);
-      if (request != null) {
-        return request.promise;
-      }
-
-      let resolveFn:
-        | ResolveFn
-        | ((
-            result: Promise<StyleAndLayoutFrontend> | StyleAndLayoutFrontend,
-          ) => void) = ((null: any): ResolveFn);
-      const promise = new Promise(resolve => {
-        resolveFn = resolve;
-      });
-
-      inProgressRequests.set(element, ({promise, resolveFn}: $FlowFixMe));
-
-      return (promise: $FlowFixMe);
-    },
-    (element: Element) => element,
-    {useWeakMap: true},
-  );
-
 type Props = {
   children: React$Node,
 };
@@ -86,72 +37,22 @@ type Props = {
 function NativeStyleContextController({children}: Props): React.Node {
   const bridge = useContext<FrontendBridge>(BridgeContext);
   const store = useContext<Store>(StoreContext);
-
-  const getStyleAndLayout = useCallback<GetStyleAndLayout>(
-    (id: number) => {
-      const element = store.getElementByID(id);
-      if (element !== null) {
-        return resource.read(element);
-      } else {
-        return null;
-      }
-    },
-    [store],
-  );
-
-  // It's very important that this context consumes inspectedElementID and not NativeStyleID.
-  // 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 {inspectedElementID} = useContext<StateContext>(TreeStateContext);
 
   const [currentStyleAndLayout, setCurrentStyleAndLayout] =
     useState<StyleAndLayoutFrontend | null>(null);
 
-  // This effect handler invalidates the suspense cache and schedules rendering updates with React.
-  useEffect(() => {
-    const onStyleAndLayout = ({id, layout, style}: StyleAndLayoutBackend) => {
-      const element = store.getElementByID(id);
-      if (element !== null) {
-        const styleAndLayout: StyleAndLayoutFrontend = {
-          layout,
-          style,
-        };
-        const request = inProgressRequests.get(element);
-        if (request != null) {
-          inProgressRequests.delete(element);
-          request.resolveFn(styleAndLayout);
-          setCurrentStyleAndLayout(styleAndLayout);
-        } else {
-          resource.write(element, styleAndLayout);
-
-          // Schedule update with React if the currently-selected element has been invalidated.
-          if (id === inspectedElementID) {
-            setCurrentStyleAndLayout(styleAndLayout);
-          }
-        }
-      }
-    };
-
-    bridge.addListener('NativeStyleEditor_styleAndLayout', onStyleAndLayout);
-    return () =>
-      bridge.removeListener(
-        'NativeStyleEditor_styleAndLayout',
-        onStyleAndLayout,
-      );
-  }, [bridge, currentStyleAndLayout, inspectedElementID, store]);
-
   // This effect handler polls for updates on the currently selected element.
   useEffect(() => {
     if (inspectedElementID === null) {
+      setCurrentStyleAndLayout(null);
       return () => {};
     }
 
-    const rendererID = store.getRendererIDForElement(inspectedElementID);
-
-    let timeoutID: TimeoutID | null = null;
-
+    let requestTimeoutId: TimeoutID | null = null;
     const sendRequest = () => {
-      timeoutID = null;
+      requestTimeoutId = null;
+      const rendererID = store.getRendererIDForElement(inspectedElementID);
 
       if (rendererID !== null) {
         bridge.send('NativeStyleEditor_measure', {
@@ -165,38 +66,37 @@ function NativeStyleContextController({children}: Props): React.Node {
     // We'll poll for an update in the response handler below.
     sendRequest();
 
-    const onStyleAndLayout = ({id}: StyleAndLayoutBackend) => {
+    const onStyleAndLayout = ({id, layout, style}: StyleAndLayoutBackend) => {
       // If this is the element we requested, wait a little bit and then ask for another update.
       if (id === inspectedElementID) {
-        if (timeoutID !== null) {
-          clearTimeout(timeoutID);
+        if (requestTimeoutId !== null) {
+          clearTimeout(requestTimeoutId);
         }
-        timeoutID = setTimeout(sendRequest, 1000);
+        requestTimeoutId = setTimeout(sendRequest, 1000);
       }
+
+      const styleAndLayout: StyleAndLayoutFrontend = {
+        layout,
+        style,
+      };
+      setCurrentStyleAndLayout(styleAndLayout);
     };
 
     bridge.addListener('NativeStyleEditor_styleAndLayout', onStyleAndLayout);
-
     return () => {
       bridge.removeListener(
         'NativeStyleEditor_styleAndLayout',
         onStyleAndLayout,
       );
 
-      if (timeoutID !== null) {
-        clearTimeout(timeoutID);
+      if (requestTimeoutId !== null) {
+        clearTimeout(requestTimeoutId);
       }
     };
   }, [bridge, inspectedElementID, store]);
 
-  const value = useMemo(
-    () => ({getStyleAndLayout}),
-    // NativeStyle is used to invalidate the cache and schedule an update with React.
-    [currentStyleAndLayout, getStyleAndLayout],
-  );
-
   return (
-    <NativeStyleContext.Provider value={value}>
+    <NativeStyleContext.Provider value={currentStyleAndLayout}>
       {children}
     </NativeStyleContext.Provider>
   );

packages/react-devtools-shared/src/devtools/views/Components/NativeStyleEditor/index.css

@@ -0,0 +1,3 @@
+.Stack > *:not(:first-child) {
+  border-top: 1px solid var(--color-border);
+}

packages/react-devtools-shared/src/devtools/views/Components/NativeStyleEditor/index.js

@@ -8,19 +8,19 @@
  */
 
 import * as React from 'react';
-import {Fragment, useContext, useMemo} from 'react';
+import {useContext, useMemo} from 'react';
+
 import {StoreContext} from 'react-devtools-shared/src/devtools/views/context';
 import {useSubscription} from 'react-devtools-shared/src/devtools/views/hooks';
+import {TreeStateContext} from 'react-devtools-shared/src/devtools/views/Components/TreeContext';
+
 import {NativeStyleContext} from './context';
 import LayoutViewer from './LayoutViewer';
 import StyleEditor from './StyleEditor';
-import {TreeStateContext} from '../TreeContext';
+import styles from './index.css';
 
-type Props = {};
-
-export default function NativeStyleEditorWrapper(_: Props): React.Node {
+export default function NativeStyleEditorWrapper(): React.Node {
   const store = useContext(StoreContext);
-
   const subscription = useMemo(
     () => ({
       getCurrentValue: () => store.supportsNativeStyleEditor,
@@ -33,8 +33,8 @@ export default function NativeStyleEditorWrapper(_: Props): React.Node {
     }),
     [store],
   );
-  const supportsNativeStyleEditor = useSubscription<boolean>(subscription);
 
+  const supportsNativeStyleEditor = useSubscription<boolean>(subscription);
   if (!supportsNativeStyleEditor) {
     return null;
   }
@@ -42,32 +42,27 @@ export default function NativeStyleEditorWrapper(_: Props): React.Node {
   return <NativeStyleEditor />;
 }
 
-function NativeStyleEditor(_: Props) {
-  const {getStyleAndLayout} = useContext(NativeStyleContext);
-
+function NativeStyleEditor() {
   const {inspectedElementID} = useContext(TreeStateContext);
+  const inspectedElementStyleAndLayout = useContext(NativeStyleContext);
   if (inspectedElementID === null) {
     return null;
   }
-
-  const maybeStyleAndLayout = getStyleAndLayout(inspectedElementID);
-  if (maybeStyleAndLayout === null) {
+  if (inspectedElementStyleAndLayout === null) {
     return null;
   }
 
-  const {layout, style} = maybeStyleAndLayout;
+  const {layout, style} = inspectedElementStyleAndLayout;
+  if (layout === null && style === null) {
+    return null;
+  }
 
   return (
-    <Fragment>
+    <div className={styles.Stack}>
       {layout !== null && (
         <LayoutViewer id={inspectedElementID} layout={layout} />
       )}
-      {style !== null && (
-        <StyleEditor
-          id={inspectedElementID}
-          style={style !== null ? style : {}}
-        />
-      )}
-    </Fragment>
+      {style !== null && <StyleEditor id={inspectedElementID} style={style} />}
+    </div>
   );
 }

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

@@ -35,6 +35,7 @@ import {
   useMemo,
   useReducer,
   useRef,
+  startTransition,
 } from 'react';
 import {createRegExp} from '../utils';
 import {StoreContext} from '../context';
@@ -890,15 +891,19 @@ function TreeContextController({
           ? store.getIndexOfElementID(store.lastSelectedHostInstanceElementId)
           : null,
   });
+  const dispatchWrapper = useMemo(
+    () => (action: Action) => startTransition(() => dispatch(action)),
+    [dispatch],
+  );
 
   // Listen for host element selections.
   useEffect(() => {
     const handler = (id: Element['id']) =>
-      dispatch({type: 'SELECT_ELEMENT_BY_ID', payload: id});
+      dispatchWrapper({type: 'SELECT_ELEMENT_BY_ID', payload: id});
 
     store.addListener('hostInstanceSelected', handler);
     return () => store.removeListener('hostInstanceSelected', handler);
-  }, [store, dispatch]);
+  }, [store, dispatchWrapper]);
 
   // If a newly-selected search result or inspection selection is inside of a collapsed subtree, auto expand it.
   // This needs to be a layout effect to avoid temporarily flashing an incorrect selection.
@@ -922,7 +927,7 @@ function TreeContextController({
       Array<number>,
       Map<number, number>,
     ]) => {
-      dispatch({
+      dispatchWrapper({
         type: 'HANDLE_STORE_MUTATION',
         payload: [addedElementIDs, removedElementIDs],
       });
@@ -933,7 +938,7 @@ function TreeContextController({
       // 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({
+      dispatchWrapper({
         type: 'HANDLE_STORE_MUTATION',
         payload: [[], new Map()],
       });
@@ -941,11 +946,11 @@ function TreeContextController({
 
     store.addListener('mutated', handleStoreMutated);
     return () => store.removeListener('mutated', handleStoreMutated);
-  }, [dispatch, initialRevision, store]);
+  }, [dispatchWrapper, initialRevision, store]);
 
   return (
     <TreeStateContext.Provider value={state}>
-      <TreeDispatcherContext.Provider value={dispatch}>
+      <TreeDispatcherContext.Provider value={dispatchWrapper}>
         {children}
       </TreeDispatcherContext.Provider>
     </TreeStateContext.Provider>