Skip to content

Commit

Permalink
DevTools: fix initial host instance selection (facebook#31892)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
hoxyq authored Jan 9, 2025
1 parent d5f3c50 commit 54cfa95
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 11 deletions.
2 changes: 0 additions & 2 deletions packages/react-devtools-extensions/src/main/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -345,8 +345,6 @@ function mountReactDevTools() {

createBridgeAndStore();

setReactSelectionFromBrowser(bridge);

createComponentsPanel();
createProfilerPanel();
}
Expand Down
20 changes: 20 additions & 0 deletions packages/react-devtools-shared/src/devtools/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ export default class Store extends EventEmitter<{
componentFilters: [],
error: [Error],
hookSettings: [$ReadOnly<DevToolsHookSettings>],
hostInstanceSelected: [Element['id']],
settingsUpdated: [$ReadOnly<DevToolsHookSettings>],
mutated: [[Array<number>, Map<number, number>]],
recordChangeDescriptions: [],
Expand Down Expand Up @@ -190,6 +191,9 @@ export default class Store extends EventEmitter<{
_hookSettings: $ReadOnly<DevToolsHookSettings> | null = null;
_shouldShowWarningsAndErrors: boolean = false;

// Only used in browser extension for synchronization with built-in Elements panel.
_lastSelectedHostInstanceElementId: Element['id'] | null = null;

constructor(bridge: FrontendBridge, config?: Config) {
super();

Expand Down Expand Up @@ -265,6 +269,7 @@ export default class Store extends EventEmitter<{
bridge.addListener('saveToClipboard', this.onSaveToClipboard);
bridge.addListener('hookSettings', this.onHookSettings);
bridge.addListener('backendInitialized', this.onBackendInitialized);
bridge.addListener('selectElement', this.onHostInstanceSelected);
}

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

get lastSelectedHostInstanceElementId(): Element['id'] | null {
return this._lastSelectedHostInstanceElementId;
}

containsElement(id: number): boolean {
return this._idToElement.has(id);
}
Expand Down Expand Up @@ -1431,6 +1440,7 @@ export default class Store extends EventEmitter<{
bridge.removeListener('backendVersion', this.onBridgeBackendVersion);
bridge.removeListener('bridgeProtocol', this.onBridgeProtocol);
bridge.removeListener('saveToClipboard', this.onSaveToClipboard);
bridge.removeListener('selectElement', this.onHostInstanceSelected);

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

onHostInstanceSelected: (elementId: number) => void = elementId => {
if (this._lastSelectedHostInstanceElementId === elementId) {
return;
}

this._lastSelectedHostInstanceElementId = elementId;
// By the time we emit this, there is no guarantee that TreeContext is rendered.
this.emit('hostInstanceSelected', elementId);
};

getHookSettings: () => void = () => {
if (this._hookSettings != null) {
this.emit('hookSettings', this._hookSettings);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ import {
startTransition,
} from 'react';
import {createRegExp} from '../utils';
import {BridgeContext, StoreContext} from '../context';
import {StoreContext} from '../context';
import Store from '../../store';

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

const initialRevision = useMemo(() => store.revision, [store]);
Expand Down Expand Up @@ -899,9 +898,15 @@ function TreeContextController({
numElements: store.numElements,
ownerSubtreeLeafElementID: null,
selectedElementID:
defaultSelectedElementID == null ? null : defaultSelectedElementID,
defaultSelectedElementID != null
? defaultSelectedElementID
: store.lastSelectedHostInstanceElementId,
selectedElementIndex:
defaultSelectedElementIndex == null ? null : defaultSelectedElementIndex,
defaultSelectedElementIndex != null
? defaultSelectedElementIndex
: store.lastSelectedHostInstanceElementId
? store.getIndexOfElementID(store.lastSelectedHostInstanceElementId)
: null,

// Search
searchIndex: null,
Expand All @@ -914,7 +919,9 @@ function TreeContextController({

// Inspection element panel
inspectedElementID:
defaultInspectedElementID == null ? null : defaultInspectedElementID,
defaultInspectedElementID != null
? defaultInspectedElementID
: store.lastSelectedHostInstanceElementId,
});

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

// Listen for host element selections.
useEffect(() => {
const handleSelectElement = (id: number) =>
const handler = (id: Element['id']) =>
dispatchWrapper({type: 'SELECT_ELEMENT_BY_ID', payload: id});
bridge.addListener('selectElement', handleSelectElement);
return () => bridge.removeListener('selectElement', handleSelectElement);
}, [bridge, dispatchWrapper]);

store.addListener('hostInstanceSelected', handler);
return () => store.removeListener('hostInstanceSelected', handler);
}, [store, dispatchWrapper]);

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

0 comments on commit 54cfa95

Please sign in to comment.