Skip to content

Commit

Permalink
[DevTools] Support VirtualInstances in findAllCurrentHostInstances (#…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
sebmarkbage authored Sep 3, 2024
1 parent 04ec50e commit e0a07e9
Showing 1 changed file with 45 additions and 35 deletions.
80 changes: 45 additions & 35 deletions packages/react-devtools-shared/src/backend/fiber/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -3393,6 +3393,18 @@ export function attach(
// I.e. we just restore them by undoing what we did above.
fiberInstance.firstChild = remainingReconcilingChildren;
remainingReconcilingChildren = null;

if (traceUpdatesEnabled) {
// If we're tracing updates and we've bailed out before reaching a host node,
// we should fall back to recursively marking the nearest host descendants for highlight.
if (traceNearestHostComponentUpdate) {
const hostInstances =
findAllCurrentHostInstances(fiberInstance);
hostInstances.forEach(hostInstance => {
traceUpdatesForNodes.add(hostInstance);
});
}
}
} else {
// If this fiber is filtered there might be changes to this set elsewhere so we have
// to visit each child to place it back in the set. We let the child bail out instead.
Expand All @@ -3404,19 +3416,6 @@ export function attach(
);
}
}

if (traceUpdatesEnabled) {
// If we're tracing updates and we've bailed out before reaching a host node,
// we should fall back to recursively marking the nearest host descendants for highlight.
if (traceNearestHostComponentUpdate) {
const hostInstances = findAllCurrentHostInstances(
getFiberInstanceThrows(nextFiber),
);
hostInstances.forEach(hostInstance => {
traceUpdatesForNodes.add(hostInstance);
});
}
}
}
}

Expand Down Expand Up @@ -3690,15 +3689,31 @@ export function attach(
return null;
}

function findAllCurrentHostInstances(
fiberInstance: FiberInstance,
): $ReadOnlyArray<HostInstance> {
const hostInstances = [];
const fiber = fiberInstance.data;
if (!fiber) {
return hostInstances;
function appendHostInstancesByDevToolsInstance(
devtoolsInstance: DevToolsInstance,
hostInstances: Array<HostInstance>,
) {
if (devtoolsInstance.kind === FIBER_INSTANCE) {
const fiber = devtoolsInstance.data;
appendHostInstancesByFiber(fiber, hostInstances);
return;
}
// Search the tree for the nearest child Fiber and add all its host instances.
// TODO: If the true nearest Fiber is filtered, we might skip it and instead include all
// the children below it. In the extreme case, searching the whole tree.
for (
let child = devtoolsInstance.firstChild;
child !== null;
child = child.nextSibling
) {
appendHostInstancesByDevToolsInstance(child, hostInstances);
}
}

function appendHostInstancesByFiber(
fiber: Fiber,
hostInstances: Array<HostInstance>,
): void {
// Next we'll drill down this component to find all HostComponent/Text.
let node: Fiber = fiber;
while (true) {
Expand All @@ -3718,19 +3733,24 @@ export function attach(
continue;
}
if (node === fiber) {
return hostInstances;
return;
}
while (!node.sibling) {
if (!node.return || node.return === fiber) {
return hostInstances;
return;
}
node = node.return;
}
node.sibling.return = node.return;
node = node.sibling;
}
// Flow needs the return here, but ESLint complains about it.
// eslint-disable-next-line no-unreachable
}

function findAllCurrentHostInstances(
devtoolsInstance: DevToolsInstance,
): $ReadOnlyArray<HostInstance> {
const hostInstances: Array<HostInstance> = [];
appendHostInstancesByDevToolsInstance(devtoolsInstance, hostInstances);
return hostInstances;
}

Expand All @@ -3741,17 +3761,7 @@ export function attach(
console.warn(`Could not find DevToolsInstance with id "${id}"`);
return null;
}
if (devtoolsInstance.kind !== FIBER_INSTANCE) {
// TODO: Handle VirtualInstance.
return null;
}
const fiber = devtoolsInstance.data;
if (fiber === null) {
return null;
}

const hostInstances = findAllCurrentHostInstances(devtoolsInstance);
return hostInstances;
return findAllCurrentHostInstances(devtoolsInstance);
} catch (err) {
// The fiber might have unmounted by now.
return null;
Expand Down

0 comments on commit e0a07e9

Please sign in to comment.