Repository: facebook/react
Pull request: [DevTools] Remove lodash.throttle

Same principle as #30555. We shouldn't be throttling the UI to make it feel less snappy. Instead, we should use back-pressure to handle it. Normally the browser handles it automatically with frame aligned events. E.g. if the thread can't keep up with sync updates it doesn't send each event but the next one. E.g. pointermove or resize.

However, it is possible that we end up queuing too many events if the frontend can't keep up but the solution to this is the same as mentioned in #30555. I.e. to track the last message and only send after we get a response.

I still keep the throttle to persist the selection since that affects the disk usage and doesn't have direct UX effects.

The main motivation for this change though is that lodash throttle doesn't rely on timers but Date.now which makes it incompatible with most jest helpers which means I can't write tests against these functions properly.


Changed files:
filename: packages/react-devtools-shared/package.json
patch: @@ -22,8 +22,6 @@
     "jsc-safe-url": "^0.2.4",
     "json5": "^2.1.3",
     "local-storage-fallback": "^4.1.1",
-    "lodash.throttle": "^4.1.1",
-    "memoize-one": "^3.1.1",
     "react-virtualized-auto-sizer": "^1.0.23",
     "react-window": "^1.8.10"
   }

filename: packages/react-devtools-shared/src/backend/agent.js
patch: @@ -8,7 +8,6 @@
  */
 
 import EventEmitter from '../events';
-import throttle from 'lodash.throttle';
 import {
   SESSION_STORAGE_LAST_SELECTION_KEY,
   SESSION_STORAGE_RELOAD_AND_PROFILE_KEY,
@@ -483,7 +482,13 @@ export default class Agent extends EventEmitter<{
         this._persistedSelection = null;
         this._persistedSelectionMatch = null;
         renderer.setTrackedPath(null);
-        this._throttledPersistSelection(rendererID, id);
+        // Throttle persisting the selection.
+        this._lastSelectedElementID = id;
+        this._lastSelectedRendererID = rendererID;
+        if (!this._persistSelectionTimerScheduled) {
+          this._persistSelectionTimerScheduled = true;
+          setTimeout(this._persistSelection, 1000);
+        }
       }
 
       // TODO: If there was a way to change the selected DOM element
@@ -893,22 +898,25 @@ export default class Agent extends EventEmitter<{
     this._bridge.send('unsupportedRendererVersion', rendererID);
   }
 
-  _throttledPersistSelection: any = throttle(
-    (rendererID: number, id: number) => {
-      // This is throttled, so both renderer and selected ID
-      // might not be available by the time we read them.
-      // This is why we need the defensive checks here.
-      const renderer = this._rendererInterfaces[rendererID];
-      const path = renderer != null ? renderer.getPathForElement(id) : null;
-      if (path !== null) {
-        sessionStorageSetItem(
-          SESSION_STORAGE_LAST_SELECTION_KEY,
-          JSON.stringify(({rendererID, path}: PersistedSelection)),
-        );
-      } else {
-        sessionStorageRemoveItem(SESSION_STORAGE_LAST_SELECTION_KEY);
-      }
-    },
-    1000,
-  );
+  _persistSelectionTimerScheduled: boolean = false;
+  _lastSelectedRendererID: number = -1;
+  _lastSelectedElementID: number = -1;
+  _persistSelection: any = () => {
+    this._persistSelectionTimerScheduled = false;
+    const rendererID = this._lastSelectedRendererID;
+    const id = this._lastSelectedElementID;
+    // This is throttled, so both renderer and selected ID
+    // might not be available by the time we read them.
+    // This is why we need the defensive checks here.
+    const renderer = this._rendererInterfaces[rendererID];
+    const path = renderer != null ? renderer.getPathForElement(id) : null;
+    if (path !== null) {
+      sessionStorageSetItem(
+        SESSION_STORAGE_LAST_SELECTION_KEY,
+        JSON.stringify(({rendererID, path}: PersistedSelection)),
+      );
+    } else {
+      sessionStorageRemoveItem(SESSION_STORAGE_LAST_SELECTION_KEY);
+    }
+  };
 }

filename: packages/react-devtools-shared/src/backend/views/Highlighter/index.js
patch: @@ -7,8 +7,6 @@
  * @flow
  */
 
-import memoize from 'memoize-one';
-import throttle from 'lodash.throttle';
 import Agent from 'react-devtools-shared/src/backend/agent';
 import {hideOverlay, showOverlay} from './Highlighter';
 
@@ -189,18 +187,12 @@ export default function setupHighlighter(
     event.stopPropagation();
   }
 
-  const selectElementForNode = throttle(
-    memoize((node: HTMLElement) => {
-      const id = agent.getIDForHostInstance(node);
-      if (id !== null) {
-        bridge.send('selectElement', id);
-      }
-    }),
-    200,
-    // Don't change the selection in the very first 200ms
-    // because those are usually unintentional as you lift the cursor.
-    {leading: false},
-  );
+  const selectElementForNode = (node: HTMLElement) => {
+    const id = agent.getIDForHostInstance(node);
+    if (id !== null) {
+      bridge.send('selectElement', id);
+    }
+  };
 
   function getEventTarget(event: MouseEvent): HTMLElement {
     if (event.composed) {

filename: packages/react-devtools-shared/src/devtools/views/hooks.js
patch: @@ -7,7 +7,6 @@
  * @flow
  */
 
-import throttle from 'lodash.throttle';
 import {
   useCallback,
   useEffect,
@@ -125,10 +124,8 @@ export function useIsOverflowing(
 
     const container = ((containerRef.current: any): HTMLDivElement);
 
-    const handleResize = throttle(
-      () => setIsOverflowing(container.clientWidth <= totalChildWidth),
-      100,
-    );
+    const handleResize = () =>
+      setIsOverflowing(container.clientWidth <= totalChildWidth);
 
     handleResize();