Skip to content

Commit

Permalink
[DevTools] Unmount instance by walking the instance tree instead of t…
Browse files Browse the repository at this point in the history 
…he fiber tree (#30665)

There was a comment that it's not safe to walk the unmounted fiber tree
which I'm not sure is correct or not but we need to walk the instance
tree to be able to clean up virtual instances anyway. We already walk
the instance tree to clean up "remaining instances".

This is also simpler because we don't need to special case Suspense
boundaries. We simply clean up whatever branch we had before.

The ultimate goal is to also walk the instance tree for updates so we
don't need a fiber to instance map.
  • Loading branch information
@sebmarkbage
sebmarkbage authored Aug 13, 2024
1 parent d48603a commit e865fe0
Showing 1 changed file with 35 additions and 84 deletions.
119 changes: 35 additions & 84 deletions packages/react-devtools-shared/src/backend/fiber/renderer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1148,8 +1148,9 @@ export function attach(

// Recursively unmount all roots.
hook.getFiberRoots(rendererID).forEach(root => {
currentRootID = getFiberInstanceThrows(root.current).id;
unmountFiberRecursively(root.current);
const rootInstance = getFiberInstanceThrows(root.current);
currentRootID = rootInstance.id;
unmountInstanceRecursively(rootInstance);
flushPendingEvents(root);
currentRootID = -1;
});
Expand Down Expand Up @@ -2183,7 +2184,8 @@ export function attach(
return fiberInstance;
}

function recordUnmount(fiber: Fiber): null | FiberInstance {
function recordUnmount(fiberInstance: FiberInstance): void {
const fiber = fiberInstance.data;
if (__DEBUG__) {
debug('recordUnmount()', fiber, null);
}
Expand All @@ -2200,25 +2202,13 @@ export function attach(
}
}

const fiberInstance = getFiberInstanceUnsafe(fiber);
if (fiberInstance === null) {
// If we've never seen this Fiber, it might be inside of a legacy render Suspense fragment (so the store is not even aware of it).
// In that case we can just ignore it or it will cause errors later on.
// One example of this is a Lazy component that never resolves before being unmounted.
//
// This also might indicate a Fast Refresh force-remount scenario.
//
// TODO: This is fragile and can obscure actual bugs.
return null;
}

const id = fiberInstance.id;
const isRoot = fiber.tag === HostRoot;
if (isRoot) {
// Roots must be removed only after all children have been removed.
// So we track it separately.
pendingUnmountedRootID = id;
} else if (!shouldFilterFiber(fiber)) {
} else {
// To maintain child-first ordering,
// we'll push it into one of these queues,
// and later arrange them in the correct order.
Expand All @@ -2232,7 +2222,6 @@ export function attach(
idToRootMap.delete(id);
idToTreeBaseDurationMap.delete(id);
}
return fiberInstance;
}

// Running state of the remaining children from the previous version of this parent that
Expand Down Expand Up @@ -2308,10 +2297,7 @@ export function attach(
function unmountRemainingChildren() {
let child = remainingReconcilingChildren;
while (child !== null) {
if (child.kind === FIBER_INSTANCE) {
unmountFiberRecursively(child.data);
}
removeChild(child);
unmountInstanceRecursively(child);
child = remainingReconcilingChildren;
}
}
Expand Down Expand Up @@ -2434,69 +2420,33 @@ export function attach(

// We use this to simulate unmounting for Suspense trees
// when we switch from primary to fallback, or deleting a subtree.
function unmountFiberRecursively(fiber: Fiber) {
function unmountInstanceRecursively(instance: DevToolsInstance) {
if (__DEBUG__) {
debug('unmountFiberRecursively()', fiber, null);
if (instance.kind === FIBER_INSTANCE) {
debug('unmountInstanceRecursively()', instance.data, null);
}
}

let fiberInstance = null;

const shouldIncludeInTree = !shouldFilterFiber(fiber);
const stashedParent = reconcilingParent;
const stashedPrevious = previouslyReconciledSibling;
const stashedRemaining = remainingReconcilingChildren;
if (shouldIncludeInTree) {
fiberInstance = getFiberInstanceThrows(fiber);
// Push a new DevTools instance parent while reconciling this subtree.
reconcilingParent = fiberInstance;
previouslyReconciledSibling = null;
// Move all the children of this instance to the remaining set.
// We'll move them back one by one, and anything that remains is deleted.
remainingReconcilingChildren = fiberInstance.firstChild;
fiberInstance.firstChild = null;
}
// Push a new DevTools instance parent while reconciling this subtree.
reconcilingParent = instance;
previouslyReconciledSibling = null;
// Move all the children of this instance to the remaining set.
remainingReconcilingChildren = instance.firstChild;
try {
// We might meet a nested Suspense on our way.
const isTimedOutSuspense =
fiber.tag === SuspenseComponent && fiber.memoizedState !== null;

if (fiber.tag === HostHoistable) {
releaseHostResource(fiber, fiber.memoizedState);
}

let child = fiber.child;
if (isTimedOutSuspense) {
// If it's showing fallback tree, let's traverse it instead.
const primaryChildFragment = fiber.child;
const fallbackChildFragment = primaryChildFragment
? primaryChildFragment.sibling
: null;
// Skip over to the real Fiber child.
child = fallbackChildFragment ? fallbackChildFragment.child : null;
}

unmountChildrenRecursively(child);
// Unmount the remaining set.
unmountRemainingChildren();
} finally {
if (shouldIncludeInTree) {
reconcilingParent = stashedParent;
previouslyReconciledSibling = stashedPrevious;
remainingReconcilingChildren = stashedRemaining;
}
}
if (fiberInstance !== null) {
recordUnmount(fiber);
removeChild(fiberInstance);
reconcilingParent = stashedParent;
previouslyReconciledSibling = stashedPrevious;
remainingReconcilingChildren = stashedRemaining;
}
}

function unmountChildrenRecursively(firstChild: null | Fiber) {
let child: null | Fiber = firstChild;
while (child !== null) {
// Record simulated unmounts children-first.
// We skip nodes without return because those are real unmounts.
unmountFiberRecursively(child);
child = child.sibling;
if (instance.kind === FIBER_INSTANCE) {
recordUnmount(instance);
}
removeChild(instance);
}

function recordProfilingDurations(fiber: Fiber) {
Expand Down Expand Up @@ -2843,7 +2793,8 @@ export function attach(
} else if (!prevDidTimeout && nextDidTimeOut) {
// Primary -> Fallback:
// 1. Hide primary set
unmountChildrenRecursively(prevFiber.child);
// We simply don't re-add the fallback children and let
// unmountRemainingChildren() handle it.
// 2. Mount fallback set
const nextFiberChild = nextFiber.child;
const nextFallbackChildSet = nextFiberChild
Expand Down Expand Up @@ -3053,19 +3004,19 @@ export function attach(
const current = root.current;
const alternate = current.alternate;

const existingRoot =
let rootInstance =
fiberToFiberInstanceMap.get(current) ||
(alternate && fiberToFiberInstanceMap.get(alternate));
if (!existingRoot) {
const newRoot = createFiberInstance(current);
idToDevToolsInstanceMap.set(newRoot.id, newRoot);
fiberToFiberInstanceMap.set(current, newRoot);
if (!rootInstance) {
rootInstance = createFiberInstance(current);
idToDevToolsInstanceMap.set(rootInstance.id, rootInstance);
fiberToFiberInstanceMap.set(current, rootInstance);
if (alternate) {
fiberToFiberInstanceMap.set(alternate, newRoot);
fiberToFiberInstanceMap.set(alternate, rootInstance);
}
currentRootID = newRoot.id;
currentRootID = rootInstance.id;
} else {
currentRootID = existingRoot.id;
currentRootID = rootInstance.id;
}

// Before the traversals, remember to start tracking
Expand Down Expand Up @@ -3123,7 +3074,7 @@ export function attach(
} else if (wasMounted && !isMounted) {
// Unmount an existing root.
removeRootPseudoKey(currentRootID);
unmountFiberRecursively(alternate);
unmountInstanceRecursively(rootInstance);
}
} else {
// Mount a new root.
Expand Down

0 comments on commit e865fe0

Please sign in to comment.