[DevTools] Implement "best renderer" by taking the inner most matched node (#30494)
Stacked on #30491. When going from DOM Node to select a component or highlight a component we find the nearest mounted ancestor. However, when multiple renderers are nested there can be multiple ancestors. The original fix #24665 did this by taking the inner renderer if it was an exact match but if it wasn't it just took the first renderer. Instead, we can track the inner most node we've found so far. Then get the ID from that node (which will be fast since it's now a perfect match). This is a better match. However, the main reason I'm doing this is because the old mechanism leaked the `Fiber` type outside the `RendererInterface` which is supposed to abstract all of that. With the new algorithm this doesn't leak. I've tested this with a new build against the repro in the old issue #24539 and it seems to work.
Sebastian Markbåge committed
Jul 30, 2024 at 14:57 UTC
f963c80d2179924380bcdd80e3c067df4a3e2ce3
8 files changed
+98
-46
packages/react-devtools-shared/src/__tests__/legacy/storeLegacy-v15-test.js
+1
-1
@@ -905,7 +905,7 @@ describe('Store (legacy)', () => {
905
[root]
906
▸ <Wrapper>
907
`);
908
- const deepestedNodeID = global.agent.getIDForNode(ref);
908
+ const deepestedNodeID = global.agent.getIDForHostInstance(ref);
909
act(() => store.toggleIsCollapsed(deepestedNodeID, false));
910
expect(store).toMatchInlineSnapshot(`
911
[root]
packages/react-devtools-shared/src/__tests__/store-test.js
+1
-1
@@ -1217,7 +1217,7 @@ describe('Store', () => {
1217
▸ <Wrapper>
1218
`);
1219
1220
- const deepestedNodeID = agent.getIDForNode(ref.current);
1220
+ const deepestedNodeID = agent.getIDForHostInstance(ref.current);
1221
1222
act(() => store.toggleIsCollapsed(deepestedNodeID, false));
1223
expect(store).toMatchInlineSnapshot(`
packages/react-devtools-shared/src/backend/agent.js
+66
-18
@@ -43,7 +43,7 @@ import type {
43
ComponentFilter,
44
BrowserTheme,
45
} from 'react-devtools-shared/src/frontend/types';
46
-import {isSynchronousXHRSupported} from './utils';
46
+import {isSynchronousXHRSupported, isReactNativeEnvironment} from './utils';
47
48
const debug = (methodName: string, ...args: Array<string>) => {
49
if (__DEBUG__) {
@@ -343,31 +343,79 @@ export default class Agent extends EventEmitter<{
343
return renderer.getInstanceAndStyle(id);
344
}
345
346
- getBestMatchingRendererInterface(node: Object): RendererInterface | null {
347
- let bestMatch = null;
346
+ getIDForHostInstance(target: HostInstance): number | null {
347
+ let bestMatch: null | HostInstance = null;
348
+ let bestRenderer: null | RendererInterface = null;
349
+ // Find the nearest ancestor which is mounted by a React.
350
for (const rendererID in this._rendererInterfaces) {
351
const renderer = ((this._rendererInterfaces[
352
(rendererID: any)
353
]: any): RendererInterface);
352
- const fiber = renderer.getFiberForNative(node);
353
- if (fiber !== null) {
354
- // check if fiber.stateNode is matching the original hostInstance
355
- if (fiber.stateNode === node) {
356
- return renderer;
357
- } else if (bestMatch === null) {
358
- bestMatch = renderer;
354
+ const nearestNode: null = renderer.getNearestMountedHostInstance(target);
355
+ if (nearestNode !== null) {
356
+ if (nearestNode === target) {
357
+ // Exact match we can exit early.
358
+ bestMatch = nearestNode;
359
+ bestRenderer = renderer;
360
+ break;
361
+ }
362
+ if (
363
+ bestMatch === null ||
364
+ (!isReactNativeEnvironment() && bestMatch.contains(nearestNode))
365
+ ) {
366
+ // If this is the first match or the previous match contains the new match,
367
+ // so the new match is a deeper and therefore better match.
368
+ bestMatch = nearestNode;
369
+ bestRenderer = renderer;
370
}
371
}
372
}
362
- // if an exact match is not found, return the first valid renderer as fallback
363
- return bestMatch;
373
+ if (bestRenderer != null && bestMatch != null) {
374
+ try {
375
+ return bestRenderer.getElementIDForHostInstance(bestMatch, true);
376
+ } catch (error) {
377
+ // Some old React versions might throw if they can't find a match.
378
+ // If so we should ignore it...
379
+ }
380
+ }
381
+ return null;
382
}
383
366
- getIDForNode(node: Object): number | null {
367
- const rendererInterface = this.getBestMatchingRendererInterface(node);
368
- if (rendererInterface != null) {
384
+ getComponentNameForHostInstance(target: HostInstance): string | null {
385
+ // We duplicate this code from getIDForHostInstance to avoid an object allocation.
386
+ let bestMatch: null | HostInstance = null;
387
+ let bestRenderer: null | RendererInterface = null;
388
+ // Find the nearest ancestor which is mounted by a React.
389
+ for (const rendererID in this._rendererInterfaces) {
390
+ const renderer = ((this._rendererInterfaces[
391
+ (rendererID: any)
392
+ ]: any): RendererInterface);
393
+ const nearestNode = renderer.getNearestMountedHostInstance(target);
394
+ if (nearestNode !== null) {
395
+ if (nearestNode === target) {
396
+ // Exact match we can exit early.
397
+ bestMatch = nearestNode;
398
+ bestRenderer = renderer;
399
+ break;
400
+ }
401
+ if (
402
+ bestMatch === null ||
403
+ (!isReactNativeEnvironment() && bestMatch.contains(nearestNode))
404
+ ) {
405
+ // If this is the first match or the previous match contains the new match,
406
+ // so the new match is a deeper and therefore better match.
407
+ bestMatch = nearestNode;
408
+ bestRenderer = renderer;
409
+ }
410
+ }
411
+ }
412
+
413
+ if (bestRenderer != null && bestMatch != null) {
414
try {
370
- return rendererInterface.getElementIDForHostInstance(node, true);
415
+ const id = bestRenderer.getElementIDForHostInstance(bestMatch, true);
416
+ if (id) {
417
+ return bestRenderer.getDisplayNameForElementID(id);
418
+ }
419
} catch (error) {
420
// Some old React versions might throw if they can't find a match.
421
// If so we should ignore it...
@@ -616,8 +664,8 @@ export default class Agent extends EventEmitter<{
664
}
665
};
666
619
- selectNode(target: Object): void {
620
- const id = this.getIDForNode(target);
667
+ selectNode(target: HostInstance): void {
668
+ const id = this.getIDForHostInstance(target);
669
if (id !== null) {
670
this._bridge.send('selectElement', id);
671
}
packages/react-devtools-shared/src/backend/fiber/renderer.js
+9
-3
@@ -2882,8 +2882,14 @@ export function attach(
2882
return fiber != null ? getDisplayNameForFiber(fiber) : null;
2883
}
2884
2885
- function getFiberForNative(hostInstance: HostInstance) {
2886
- return renderer.findFiberByHostInstance(hostInstance);
2885
+ function getNearestMountedHostInstance(
2886
+ hostInstance: HostInstance,
2887
+ ): null | HostInstance {
2888
+ const mountedHostInstance = renderer.findFiberByHostInstance(hostInstance);
2889
+ if (mountedHostInstance != null) {
2890
+ return mountedHostInstance.stateNode;
2891
+ }
2892
+ return null;
2893
}
2894
2895
function getElementIDForHostInstance(
@@ -4659,7 +4665,7 @@ export function attach(
4665
flushInitialOperations,
4666
getBestMatchForTrackedPath,
4667
getDisplayNameForElementID,
4662
- getFiberForNative,
4668
+ getNearestMountedHostInstance,
4669
getElementIDForHostInstance,
4670
getInstanceAndStyle,
4671
getOwnersList,
packages/react-devtools-shared/src/backend/legacy/renderer.js
+13
-4
@@ -145,7 +145,9 @@ export function attach(
145
let getElementIDForHostInstance: GetElementIDForHostInstance =
146
((null: any): GetElementIDForHostInstance);
147
let findHostInstanceForInternalID: (id: number) => ?HostInstance;
148
- let getFiberForNative = (node: HostInstance) => {
148
+ let getNearestMountedHostInstance = (
149
+ node: HostInstance,
150
+ ): null | HostInstance => {
151
// Not implemented.
152
return null;
153
};
@@ -160,8 +162,15 @@ export function attach(
162
const internalInstance = idToInternalInstanceMap.get(id);
163
return renderer.ComponentTree.getNodeFromInstance(internalInstance);
164
};
163
- getFiberForNative = (node: HostInstance) => {
164
- return renderer.ComponentTree.getClosestInstanceFromNode(node);
165
+ getNearestMountedHostInstance = (
166
+ node: HostInstance,
167
+ ): null | HostInstance => {
168
+ const internalInstance =
169
+ renderer.ComponentTree.getClosestInstanceFromNode(node);
170
+ if (internalInstance != null) {
171
+ return renderer.ComponentTree.getNodeFromInstance(internalInstance);
172
+ }
173
+ return null;
174
};
175
} else if (renderer.Mount.getID && renderer.Mount.getNode) {
176
getElementIDForHostInstance = (node, findNearestUnfilteredAncestor) => {
@@ -1111,7 +1120,7 @@ export function attach(
1120
flushInitialOperations,
1121
getBestMatchForTrackedPath,
1122
getDisplayNameForElementID,
1114
- getFiberForNative,
1123
+ getNearestMountedHostInstance,
1124
getElementIDForHostInstance,
1125
getInstanceAndStyle,
1126
findHostInstancesForElementID: (id: number) => {
packages/react-devtools-shared/src/backend/types.js
+4
-5
@@ -86,10 +86,7 @@ type SharedInternalsSubset = {
86
};
87
export type CurrentDispatcherRef = SharedInternalsSubset;
88
89
-export type GetDisplayNameForElementID = (
90
- id: number,
91
- findNearestUnfilteredAncestor?: boolean,
92
-) => string | null;
89
+export type GetDisplayNameForElementID = (id: number) => string | null;
90
91
export type GetElementIDForHostInstance = (
92
component: HostInstance,
@@ -363,7 +360,9 @@ export type RendererInterface = {
360
findHostInstancesForElementID: FindHostInstancesForElementID,
361
flushInitialOperations: () => void,
362
getBestMatchForTrackedPath: () => PathMatch | null,
366
- getFiberForNative: (component: HostInstance) => Fiber | null,
363
+ getNearestMountedHostInstance: (
364
+ component: HostInstance,
365
+ ) => HostInstance | null,
366
getElementIDForHostInstance: GetElementIDForHostInstance,
367
getDisplayNameForElementID: GetDisplayNameForElementID,
368
getInstanceAndStyle(id: number): InstanceAndStyle,
packages/react-devtools-shared/src/backend/views/Highlighter/Overlay.js
+3
-13
@@ -233,19 +233,9 @@ export default class Overlay {
233
name = elements[0].nodeName.toLowerCase();
234
235
const node = elements[0];
236
- const rendererInterface =
237
- this.agent.getBestMatchingRendererInterface(node);
238
- if (rendererInterface) {
239
- const id = rendererInterface.getElementIDForHostInstance(node, true);
240
- if (id) {
241
- const ownerName = rendererInterface.getDisplayNameForElementID(
242
- id,
243
- true,
244
- );
245
- if (ownerName) {
246
- name += ' (in ' + ownerName + ')';
247
- }
248
- }
236
+ const ownerName = this.agent.getComponentNameForHostInstance(node);
237
+ if (ownerName) {
238
+ name += ' (in ' + ownerName + ')';
239
}
240
}
241
packages/react-devtools-shared/src/backend/views/Highlighter/index.js
+1
-1
@@ -193,7 +193,7 @@ export default function setupHighlighter(
193
194
const selectElementForNode = throttle(
195
memoize((node: HTMLElement) => {
196
- const id = agent.getIDForNode(node);
196
+ const id = agent.getIDForHostInstance(node);
197
if (id !== null) {
198
bridge.send('selectElement', id);
199
}