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 -
#30555.
 
Looks like I've introduced a regression in
#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.

Author: hoxyq

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

@@ -212,17 +212,19 @@ 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');
 
         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,148 +10,49 @@
 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,
 };
 
 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';
-
-type Props = {};
+import styles from './index.css';
 
-export default function NativeStyleEditorWrapper(_: Props): React.Node {
+export default function NativeStyleEditorWrapper(): React.Node {
   const store = useContext(StoreContext);
-
   const subscription = useMemo(
     () => ({
       getCurrentValue: () => store.supportsNativeStyleEditor,
@@ -33,41 +33,36 @@ export default function NativeStyleEditorWrapper(_: Props): React.Node {
     }),
     [store],
   );
-  const supportsNativeStyleEditor = useSubscription<boolean>(subscription);
 
+  const supportsNativeStyleEditor = useSubscription<boolean>(subscription);
   if (!supportsNativeStyleEditor) {
     return null;
   }
 
   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>
   );
 }