Skip to content

Commit b4c3801

Browse files
authored
[DevTools] Remove lodash.throttle (#30657)
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.
1 parent 68dbd84 commit b4c3801

File tree

4 files changed

+36
-41
lines changed

4 files changed

+36
-41
lines changed

Diff for: packages/react-devtools-shared/package.json

-2
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,6 @@
2222
"jsc-safe-url": "^0.2.4",
2323
"json5": "^2.1.3",
2424
"local-storage-fallback": "^4.1.1",
25-
"lodash.throttle": "^4.1.1",
26-
"memoize-one": "^3.1.1",
2725
"react-virtualized-auto-sizer": "^1.0.23",
2826
"react-window": "^1.8.10"
2927
}

Diff for: packages/react-devtools-shared/src/backend/agent.js

+28-20
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
*/
99

1010
import EventEmitter from '../events';
11-
import throttle from 'lodash.throttle';
1211
import {
1312
SESSION_STORAGE_LAST_SELECTION_KEY,
1413
SESSION_STORAGE_RELOAD_AND_PROFILE_KEY,
@@ -483,7 +482,13 @@ export default class Agent extends EventEmitter<{
483482
this._persistedSelection = null;
484483
this._persistedSelectionMatch = null;
485484
renderer.setTrackedPath(null);
486-
this._throttledPersistSelection(rendererID, id);
485+
// Throttle persisting the selection.
486+
this._lastSelectedElementID = id;
487+
this._lastSelectedRendererID = rendererID;
488+
if (!this._persistSelectionTimerScheduled) {
489+
this._persistSelectionTimerScheduled = true;
490+
setTimeout(this._persistSelection, 1000);
491+
}
487492
}
488493

489494
// TODO: If there was a way to change the selected DOM element
@@ -893,22 +898,25 @@ export default class Agent extends EventEmitter<{
893898
this._bridge.send('unsupportedRendererVersion', rendererID);
894899
}
895900

896-
_throttledPersistSelection: any = throttle(
897-
(rendererID: number, id: number) => {
898-
// This is throttled, so both renderer and selected ID
899-
// might not be available by the time we read them.
900-
// This is why we need the defensive checks here.
901-
const renderer = this._rendererInterfaces[rendererID];
902-
const path = renderer != null ? renderer.getPathForElement(id) : null;
903-
if (path !== null) {
904-
sessionStorageSetItem(
905-
SESSION_STORAGE_LAST_SELECTION_KEY,
906-
JSON.stringify(({rendererID, path}: PersistedSelection)),
907-
);
908-
} else {
909-
sessionStorageRemoveItem(SESSION_STORAGE_LAST_SELECTION_KEY);
910-
}
911-
},
912-
1000,
913-
);
901+
_persistSelectionTimerScheduled: boolean = false;
902+
_lastSelectedRendererID: number = -1;
903+
_lastSelectedElementID: number = -1;
904+
_persistSelection: any = () => {
905+
this._persistSelectionTimerScheduled = false;
906+
const rendererID = this._lastSelectedRendererID;
907+
const id = this._lastSelectedElementID;
908+
// This is throttled, so both renderer and selected ID
909+
// might not be available by the time we read them.
910+
// This is why we need the defensive checks here.
911+
const renderer = this._rendererInterfaces[rendererID];
912+
const path = renderer != null ? renderer.getPathForElement(id) : null;
913+
if (path !== null) {
914+
sessionStorageSetItem(
915+
SESSION_STORAGE_LAST_SELECTION_KEY,
916+
JSON.stringify(({rendererID, path}: PersistedSelection)),
917+
);
918+
} else {
919+
sessionStorageRemoveItem(SESSION_STORAGE_LAST_SELECTION_KEY);
920+
}
921+
};
914922
}

Diff for: packages/react-devtools-shared/src/backend/views/Highlighter/index.js

+6-14
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@
77
* @flow
88
*/
99

10-
import memoize from 'memoize-one';
11-
import throttle from 'lodash.throttle';
1210
import Agent from 'react-devtools-shared/src/backend/agent';
1311
import {hideOverlay, showOverlay} from './Highlighter';
1412

@@ -189,18 +187,12 @@ export default function setupHighlighter(
189187
event.stopPropagation();
190188
}
191189

192-
const selectElementForNode = throttle(
193-
memoize((node: HTMLElement) => {
194-
const id = agent.getIDForHostInstance(node);
195-
if (id !== null) {
196-
bridge.send('selectElement', id);
197-
}
198-
}),
199-
200,
200-
// Don't change the selection in the very first 200ms
201-
// because those are usually unintentional as you lift the cursor.
202-
{leading: false},
203-
);
190+
const selectElementForNode = (node: HTMLElement) => {
191+
const id = agent.getIDForHostInstance(node);
192+
if (id !== null) {
193+
bridge.send('selectElement', id);
194+
}
195+
};
204196

205197
function getEventTarget(event: MouseEvent): HTMLElement {
206198
if (event.composed) {

Diff for: packages/react-devtools-shared/src/devtools/views/hooks.js

+2-5
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
* @flow
88
*/
99

10-
import throttle from 'lodash.throttle';
1110
import {
1211
useCallback,
1312
useEffect,
@@ -125,10 +124,8 @@ export function useIsOverflowing(
125124

126125
const container = ((containerRef.current: any): HTMLDivElement);
127126

128-
const handleResize = throttle(
129-
() => setIsOverflowing(container.clientWidth <= totalChildWidth),
130-
100,
131-
);
127+
const handleResize = () =>
128+
setIsOverflowing(container.clientWidth <= totalChildWidth);
132129

133130
handleResize();
134131

0 commit comments

Comments
 (0)