[DevTools] Make Element Inspection Feel Snappy

packages/react-devtools-shared/src/__tests__/inspectedElement-test.js

@@ -117,12 +117,11 @@ describe('InspectedElement', () => {
         <SettingsContextController>
           <TreeContextController
             defaultSelectedElementID={defaultSelectedElementID}
-            defaultSelectedElementIndex={defaultSelectedElementIndex}>
-            <React.Suspense fallback="Loading...">
-              <InspectedElementContextController>
-                {children}
-              </InspectedElementContextController>
-            </React.Suspense>
+            defaultSelectedElementIndex={defaultSelectedElementIndex}
+            defaultInspectedElementID={defaultSelectedElementID}>
+            <InspectedElementContextController>
+              {children}
+            </InspectedElementContextController>
           </TreeContextController>
         </SettingsContextController>
       </StoreContext.Provider>

packages/react-devtools-shared/src/__tests__/setupTests.js

@@ -128,6 +128,11 @@ beforeEach(() => {
   // Fake timers let us flush Bridge operations between setup and assertions.
   jest.useFakeTimers();
 
+  // We use fake timers heavily in tests but the bridge batching now uses microtasks.
+  global.devtoolsJestTestScheduler = callback => {
+    setTimeout(callback, 0);
+  };
+
   // Use utils.js#withErrorsOrWarningsIgnored instead of directly mutating this array.
   global._ignoredErrorOrWarningMessages = [
     'react-test-renderer is deprecated.',

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

@@ -19,8 +19,6 @@ import type {
 } from 'react-devtools-shared/src/backend/types';
 import type {StyleAndLayout as StyleAndLayoutPayload} from 'react-devtools-shared/src/backend/NativeStyleEditor/types';
 
-const BATCH_DURATION = 100;
-
 // This message specifies the version of the DevTools protocol currently supported by the backend,
 // as well as the earliest NPM version (e.g. "4.13.0") that protocol is supported by on the frontend.
 // This enables an older frontend to display an upgrade message to users for a newer, unsupported backend.
@@ -276,7 +274,7 @@ class Bridge<
 }> {
   _isShutdown: boolean = false;
   _messageQueue: Array<any> = [];
-  _timeoutID: TimeoutID | null = null;
+  _scheduledFlush: boolean = false;
   _wall: Wall;
   _wallUnlisten: Function | null = null;
 
@@ -324,8 +322,19 @@ class Bridge<
     //   (or we're waiting for our setTimeout-0 to fire), then _timeoutID will
     //   be set, and we'll simply add to the queue and wait for that
     this._messageQueue.push(event, payload);
-    if (!this._timeoutID) {
-      this._timeoutID = setTimeout(this._flush, 0);
+    if (!this._scheduledFlush) {
+      this._scheduledFlush = true;
+      // $FlowFixMe
+      if (typeof devtoolsJestTestScheduler === 'function') {
+        // This exists just for our own jest tests.
+        // They're written in such a way that we can neither mock queueMicrotask
+        // because then we break React DOM and we can't not mock it because then
+        // we can't synchronously flush it. So they need to be rewritten.
+        // $FlowFixMe
+        devtoolsJestTestScheduler(this._flush); // eslint-disable-line no-undef
+      } else {
+        queueMicrotask(this._flush);
+      }
     }
   }
 
@@ -363,34 +372,23 @@ class Bridge<
     do {
       this._flush();
     } while (this._messageQueue.length);
-
-    // Make sure once again that there is no dangling timer.
-    if (this._timeoutID !== null) {
-      clearTimeout(this._timeoutID);
-      this._timeoutID = null;
-    }
   }
 
   _flush: () => void = () => {
     // This method is used after the bridge is marked as destroyed in shutdown sequence,
     // so we do not bail out if the bridge marked as destroyed.
     // It is a private method that the bridge ensures is only called at the right times.
-
-    if (this._timeoutID !== null) {
-      clearTimeout(this._timeoutID);
-      this._timeoutID = null;
-    }
-
-    if (this._messageQueue.length) {
-      for (let i = 0; i < this._messageQueue.length; i += 2) {
-        this._wall.send(this._messageQueue[i], ...this._messageQueue[i + 1]);
+    try {
+      if (this._messageQueue.length) {
+        for (let i = 0; i < this._messageQueue.length; i += 2) {
+          this._wall.send(this._messageQueue[i], ...this._messageQueue[i + 1]);
+        }
+        this._messageQueue.length = 0;
       }
-      this._messageQueue.length = 0;
-
-      // Check again for queued messages in BATCH_DURATION ms. This will keep
-      // flushing in a loop as long as messages continue to be added. Once no
-      // more are, the timer expires.
-      this._timeoutID = setTimeout(this._flush, BATCH_DURATION);
+    } finally {
+      // We set this at the end in case new messages are added synchronously above.
+      // They're already handled so they shouldn't queue more flushes.
+      this._scheduledFlush = false;
     }
   };
 

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

@@ -8,14 +8,7 @@
  */
 
 import * as React from 'react';
-import {
-  Fragment,
-  Suspense,
-  useEffect,
-  useLayoutEffect,
-  useReducer,
-  useRef,
-} from 'react';
+import {Fragment, useEffect, useLayoutEffect, useReducer, useRef} from 'react';
 import Tree from './Tree';
 import {OwnersListContextController} from './OwnersListContext';
 import portaledContent from '../portaledContent';
@@ -169,11 +162,9 @@ function Components(_: {}) {
             <div className={styles.InspectedElementWrapper}>
               <NativeStyleContextController>
                 <InspectedElementErrorBoundary>
-                  <Suspense fallback={<Loading />}>
-                    <InspectedElementContextController>
-                      <InspectedElement />
-                    </InspectedElementContextController>
-                  </Suspense>
+                  <InspectedElementContextController>
+                    <InspectedElement />
+                  </InspectedElementContextController>
                 </InspectedElementErrorBoundary>
               </NativeStyleContextController>
             </div>
@@ -186,10 +177,6 @@ function Components(_: {}) {
   );
 }
 
-function Loading() {
-  return <div className={styles.Loading}>Loading...</div>;
-}
-
 const LOCAL_STORAGE_KEY = 'React::DevTools::createResizeReducer';
 const VERTICAL_MODE_MAX_WIDTH = 600;
 const MINIMUM_SIZE = 50;

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

@@ -66,7 +66,7 @@ export type Props = {
 export function InspectedElementContextController({
   children,
 }: Props): React.Node {
-  const {selectedElementID} = useContext(TreeStateContext);
+  const {inspectedElementID} = useContext(TreeStateContext);
   const fetchFileWithCaching = useContext(FetchFileWithCachingContext);
   const bridge = useContext(BridgeContext);
   const store = useContext(StoreContext);
@@ -93,7 +93,9 @@ export function InspectedElementContextController({
   });
 
   const element =
-    selectedElementID !== null ? store.getElementByID(selectedElementID) : null;
+    inspectedElementID !== null
+      ? store.getElementByID(inspectedElementID)
+      : null;
 
   const alreadyLoadedHookNames =
     element != null && hasAlreadyLoadedHookNames(element);

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

@@ -27,7 +27,7 @@ export default function InspectedElementErrorBoundaryWrapper({
 }: WrapperProps): React.Node {
   // Key on the selected element ID so that changing the selected element automatically hides the boundary.
   // This seems best since an error inspecting one element isn't likely to be relevant to another element.
-  const {selectedElementID} = useContext(TreeStateContext);
+  const {inspectedElementID} = useContext(TreeStateContext);
 
   const refresh = useCacheRefresh();
   const handleDsmiss = useCallback(() => {
@@ -37,7 +37,7 @@ export default function InspectedElementErrorBoundaryWrapper({
   return (
     <div className={styles.Wrapper}>
       <ErrorBoundary
-        key={selectedElementID}
+        key={inspectedElementID}
         canDismiss={true}
         onBeforeDismissCallback={handleDsmiss}>
         {children}