Skip to content

Commit 9af74e0

Browse files
committed
[DevTools] Support VirtualInstances in findAllCurrentHostInstances (#30853)
This lets us highlight Server Components. However, there is a problem with this because if the actual nearest Fiber is filtered, there's no FiberInstance and so we might skip past it and maybe never find a child while walking the whole tree. This is very common in the case where you have just Server Components and Host Components which are filtered by default. Note how the DOM nodes that are just plain host instances without client component wrappers are not highlighted here: <img width="1102" alt="Screenshot 2024-08-30 at 4 33 55 PM" src="https://github.com/user-attachments/assets/c9a7b91e-5faf-4c60-99a8-1195539ff8b5"> Fixing that needs a separate refactor though and related to several other features that already have a similar issue without VirtualInstances like Suspense/Error Boundaries (triggering suspense/error on a filtered Suspense/ErrorBoundary doesn't work correctly). So this first PR just adds the feature for the common case where there's at least some Fibers. [ghstack-poisoned]
1 parent b658948 commit 9af74e0

File tree

1 file changed

+45
-35
lines changed
  • packages/react-devtools-shared/src/backend/fiber

1 file changed

+45
-35
lines changed

packages/react-devtools-shared/src/backend/fiber/renderer.js

Lines changed: 45 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -3393,6 +3393,18 @@ export function attach(
33933393
// I.e. we just restore them by undoing what we did above.
33943394
fiberInstance.firstChild = remainingReconcilingChildren;
33953395
remainingReconcilingChildren = null;
3396+
3397+
if (traceUpdatesEnabled) {
3398+
// If we're tracing updates and we've bailed out before reaching a host node,
3399+
// we should fall back to recursively marking the nearest host descendants for highlight.
3400+
if (traceNearestHostComponentUpdate) {
3401+
const hostInstances =
3402+
findAllCurrentHostInstances(fiberInstance);
3403+
hostInstances.forEach(hostInstance => {
3404+
traceUpdatesForNodes.add(hostInstance);
3405+
});
3406+
}
3407+
}
33963408
} else {
33973409
// If this fiber is filtered there might be changes to this set elsewhere so we have
33983410
// to visit each child to place it back in the set. We let the child bail out instead.
@@ -3404,19 +3416,6 @@ export function attach(
34043416
);
34053417
}
34063418
}
3407-
3408-
if (traceUpdatesEnabled) {
3409-
// If we're tracing updates and we've bailed out before reaching a host node,
3410-
// we should fall back to recursively marking the nearest host descendants for highlight.
3411-
if (traceNearestHostComponentUpdate) {
3412-
const hostInstances = findAllCurrentHostInstances(
3413-
getFiberInstanceThrows(nextFiber),
3414-
);
3415-
hostInstances.forEach(hostInstance => {
3416-
traceUpdatesForNodes.add(hostInstance);
3417-
});
3418-
}
3419-
}
34203419
}
34213420
}
34223421

@@ -3690,15 +3689,31 @@ export function attach(
36903689
return null;
36913690
}
36923691

3693-
function findAllCurrentHostInstances(
3694-
fiberInstance: FiberInstance,
3695-
): $ReadOnlyArray<HostInstance> {
3696-
const hostInstances = [];
3697-
const fiber = fiberInstance.data;
3698-
if (!fiber) {
3699-
return hostInstances;
3692+
function appendHostInstancesByDevToolsInstance(
3693+
devtoolsInstance: DevToolsInstance,
3694+
hostInstances: Array<HostInstance>,
3695+
) {
3696+
if (devtoolsInstance.kind === FIBER_INSTANCE) {
3697+
const fiber = devtoolsInstance.data;
3698+
appendHostInstancesByFiber(fiber, hostInstances);
3699+
return;
37003700
}
3701+
// Search the tree for the nearest child Fiber and add all its host instances.
3702+
// TODO: If the true nearest Fiber is filtered, we might skip it and instead include all
3703+
// the children below it. In the extreme case, searching the whole tree.
3704+
for (
3705+
let child = devtoolsInstance.firstChild;
3706+
child !== null;
3707+
child = child.nextSibling
3708+
) {
3709+
appendHostInstancesByDevToolsInstance(child, hostInstances);
3710+
}
3711+
}
37013712

3713+
function appendHostInstancesByFiber(
3714+
fiber: Fiber,
3715+
hostInstances: Array<HostInstance>,
3716+
): void {
37023717
// Next we'll drill down this component to find all HostComponent/Text.
37033718
let node: Fiber = fiber;
37043719
while (true) {
@@ -3718,19 +3733,24 @@ export function attach(
37183733
continue;
37193734
}
37203735
if (node === fiber) {
3721-
return hostInstances;
3736+
return;
37223737
}
37233738
while (!node.sibling) {
37243739
if (!node.return || node.return === fiber) {
3725-
return hostInstances;
3740+
return;
37263741
}
37273742
node = node.return;
37283743
}
37293744
node.sibling.return = node.return;
37303745
node = node.sibling;
37313746
}
3732-
// Flow needs the return here, but ESLint complains about it.
3733-
// eslint-disable-next-line no-unreachable
3747+
}
3748+
3749+
function findAllCurrentHostInstances(
3750+
devtoolsInstance: DevToolsInstance,
3751+
): $ReadOnlyArray<HostInstance> {
3752+
const hostInstances: Array<HostInstance> = [];
3753+
appendHostInstancesByDevToolsInstance(devtoolsInstance, hostInstances);
37343754
return hostInstances;
37353755
}
37363756

@@ -3741,17 +3761,7 @@ export function attach(
37413761
console.warn(`Could not find DevToolsInstance with id "${id}"`);
37423762
return null;
37433763
}
3744-
if (devtoolsInstance.kind !== FIBER_INSTANCE) {
3745-
// TODO: Handle VirtualInstance.
3746-
return null;
3747-
}
3748-
const fiber = devtoolsInstance.data;
3749-
if (fiber === null) {
3750-
return null;
3751-
}
3752-
3753-
const hostInstances = findAllCurrentHostInstances(devtoolsInstance);
3754-
return hostInstances;
3764+
return findAllCurrentHostInstances(devtoolsInstance);
37553765
} catch (err) {
37563766
// The fiber might have unmounted by now.
37573767
return null;

0 commit comments

Comments
 (0)