Skip to content

Commit 54cfa95

Browse files
authored
DevTools: fix initial host instance selection (facebook#31892)
Related: facebook#31342 This fixes RDT behaviour when some DOM element was pre-selected in built-in browser's Elements panel, and then Components panel of React DevTools was opened for the first time. With this change, React DevTools will correctly display the initial state of the Components Tree with the corresponding React Element (if possible) pre-selected. Previously, we would only subscribe listener when `TreeContext` is mounted, but this only happens when user opens one of React DevTools panels for the first time. With this change, we keep state inside `Store`, which is created when Browser DevTools are opened. Later, `TreeContext` will use it for initial state value. Planned next changes: 1. Merge `inspectedElementID` and `selectedElementID`, I have no idea why we need both. 2. Fix issue with `AutoSizer` rendering a blank container.
1 parent d5f3c50 commit 54cfa95

File tree

3 files changed

+37
-11
lines changed

3 files changed

+37
-11
lines changed

packages/react-devtools-extensions/src/main/index.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -345,8 +345,6 @@ function mountReactDevTools() {
345345

346346
createBridgeAndStore();
347347

348-
setReactSelectionFromBrowser(bridge);
349-
350348
createComponentsPanel();
351349
createProfilerPanel();
352350
}

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ export default class Store extends EventEmitter<{
9696
componentFilters: [],
9797
error: [Error],
9898
hookSettings: [$ReadOnly<DevToolsHookSettings>],
99+
hostInstanceSelected: [Element['id']],
99100
settingsUpdated: [$ReadOnly<DevToolsHookSettings>],
100101
mutated: [[Array<number>, Map<number, number>]],
101102
recordChangeDescriptions: [],
@@ -190,6 +191,9 @@ export default class Store extends EventEmitter<{
190191
_hookSettings: $ReadOnly<DevToolsHookSettings> | null = null;
191192
_shouldShowWarningsAndErrors: boolean = false;
192193

194+
// Only used in browser extension for synchronization with built-in Elements panel.
195+
_lastSelectedHostInstanceElementId: Element['id'] | null = null;
196+
193197
constructor(bridge: FrontendBridge, config?: Config) {
194198
super();
195199

@@ -265,6 +269,7 @@ export default class Store extends EventEmitter<{
265269
bridge.addListener('saveToClipboard', this.onSaveToClipboard);
266270
bridge.addListener('hookSettings', this.onHookSettings);
267271
bridge.addListener('backendInitialized', this.onBackendInitialized);
272+
bridge.addListener('selectElement', this.onHostInstanceSelected);
268273
}
269274

270275
// This is only used in tests to avoid memory leaks.
@@ -481,6 +486,10 @@ export default class Store extends EventEmitter<{
481486
return this._unsupportedRendererVersionDetected;
482487
}
483488

489+
get lastSelectedHostInstanceElementId(): Element['id'] | null {
490+
return this._lastSelectedHostInstanceElementId;
491+
}
492+
484493
containsElement(id: number): boolean {
485494
return this._idToElement.has(id);
486495
}
@@ -1431,6 +1440,7 @@ export default class Store extends EventEmitter<{
14311440
bridge.removeListener('backendVersion', this.onBridgeBackendVersion);
14321441
bridge.removeListener('bridgeProtocol', this.onBridgeProtocol);
14331442
bridge.removeListener('saveToClipboard', this.onSaveToClipboard);
1443+
bridge.removeListener('selectElement', this.onHostInstanceSelected);
14341444

14351445
if (this._onBridgeProtocolTimeoutID !== null) {
14361446
clearTimeout(this._onBridgeProtocolTimeoutID);
@@ -1507,6 +1517,16 @@ export default class Store extends EventEmitter<{
15071517
this._bridge.send('getHookSettings'); // Warm up cached hook settings
15081518
};
15091519

1520+
onHostInstanceSelected: (elementId: number) => void = elementId => {
1521+
if (this._lastSelectedHostInstanceElementId === elementId) {
1522+
return;
1523+
}
1524+
1525+
this._lastSelectedHostInstanceElementId = elementId;
1526+
// By the time we emit this, there is no guarantee that TreeContext is rendered.
1527+
this.emit('hostInstanceSelected', elementId);
1528+
};
1529+
15101530
getHookSettings: () => void = () => {
15111531
if (this._hookSettings != null) {
15121532
this.emit('hookSettings', this._hookSettings);

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

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ import {
3939
startTransition,
4040
} from 'react';
4141
import {createRegExp} from '../utils';
42-
import {BridgeContext, StoreContext} from '../context';
42+
import {StoreContext} from '../context';
4343
import Store from '../../store';
4444

4545
import type {Element} from 'react-devtools-shared/src/frontend/types';
@@ -836,7 +836,6 @@ function TreeContextController({
836836
defaultSelectedElementID,
837837
defaultSelectedElementIndex,
838838
}: Props): React.Node {
839-
const bridge = useContext(BridgeContext);
840839
const store = useContext(StoreContext);
841840

842841
const initialRevision = useMemo(() => store.revision, [store]);
@@ -899,9 +898,15 @@ function TreeContextController({
899898
numElements: store.numElements,
900899
ownerSubtreeLeafElementID: null,
901900
selectedElementID:
902-
defaultSelectedElementID == null ? null : defaultSelectedElementID,
901+
defaultSelectedElementID != null
902+
? defaultSelectedElementID
903+
: store.lastSelectedHostInstanceElementId,
903904
selectedElementIndex:
904-
defaultSelectedElementIndex == null ? null : defaultSelectedElementIndex,
905+
defaultSelectedElementIndex != null
906+
? defaultSelectedElementIndex
907+
: store.lastSelectedHostInstanceElementId
908+
? store.getIndexOfElementID(store.lastSelectedHostInstanceElementId)
909+
: null,
905910

906911
// Search
907912
searchIndex: null,
@@ -914,7 +919,9 @@ function TreeContextController({
914919

915920
// Inspection element panel
916921
inspectedElementID:
917-
defaultInspectedElementID == null ? null : defaultInspectedElementID,
922+
defaultInspectedElementID != null
923+
? defaultInspectedElementID
924+
: store.lastSelectedHostInstanceElementId,
918925
});
919926

920927
const dispatchWrapper = useCallback(
@@ -929,11 +936,12 @@ function TreeContextController({
929936

930937
// Listen for host element selections.
931938
useEffect(() => {
932-
const handleSelectElement = (id: number) =>
939+
const handler = (id: Element['id']) =>
933940
dispatchWrapper({type: 'SELECT_ELEMENT_BY_ID', payload: id});
934-
bridge.addListener('selectElement', handleSelectElement);
935-
return () => bridge.removeListener('selectElement', handleSelectElement);
936-
}, [bridge, dispatchWrapper]);
941+
942+
store.addListener('hostInstanceSelected', handler);
943+
return () => store.removeListener('hostInstanceSelected', handler);
944+
}, [store, dispatchWrapper]);
937945

938946
// If a newly-selected search result or inspection selection is inside of a collapsed subtree, auto expand it.
939947
// This needs to be a layout effect to avoid temporarily flashing an incorrect selection.

0 commit comments

Comments
 (0)