[DevTools] Unmount instance by walking the instance tree instead of the 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.
Sebastian Markbåge committed
Aug 13, 2024 at 15:18 UTC
e865fe0722f889126b818d462a73026d7d0b6867
1 file changed
+35
-84
packages/react-devtools-shared/src/backend/fiber/renderer.js
+35
-84
@@ -1148,8 +1148,9 @@ export function attach(
1148
1149
// Recursively unmount all roots.
1150
hook.getFiberRoots(rendererID).forEach(root => {
1151
- currentRootID = getFiberInstanceThrows(root.current).id;
1152
- unmountFiberRecursively(root.current);
1151
+ const rootInstance = getFiberInstanceThrows(root.current);
1152
+ currentRootID = rootInstance.id;
1153
+ unmountInstanceRecursively(rootInstance);
1154
flushPendingEvents(root);
1155
currentRootID = -1;
1156
});
@@ -2183,7 +2184,8 @@ export function attach(
2184
return fiberInstance;
2185
}
2186
2186
- function recordUnmount(fiber: Fiber): null | FiberInstance {
2187
+ function recordUnmount(fiberInstance: FiberInstance): void {
2188
+ const fiber = fiberInstance.data;
2189
if (__DEBUG__) {
2190
debug('recordUnmount()', fiber, null);
2191
}
@@ -2200,25 +2202,13 @@ export function attach(
2202
}
2203
}
2204
2203
- const fiberInstance = getFiberInstanceUnsafe(fiber);
2204
- if (fiberInstance === null) {
2205
- // 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).
2206
- // In that case we can just ignore it or it will cause errors later on.
2207
- // One example of this is a Lazy component that never resolves before being unmounted.
2208
- //
2209
- // This also might indicate a Fast Refresh force-remount scenario.
2210
- //
2211
- // TODO: This is fragile and can obscure actual bugs.
2212
- return null;
2213
- }
2214
-
2205
const id = fiberInstance.id;
2206
const isRoot = fiber.tag === HostRoot;
2207
if (isRoot) {
2208
// Roots must be removed only after all children have been removed.
2209
// So we track it separately.
2210
pendingUnmountedRootID = id;
2221
- } else if (!shouldFilterFiber(fiber)) {
2211
+ } else {
2212
// To maintain child-first ordering,
2213
// we'll push it into one of these queues,
2214
// and later arrange them in the correct order.
@@ -2232,7 +2222,6 @@ export function attach(
2222
idToRootMap.delete(id);
2223
idToTreeBaseDurationMap.delete(id);
2224
}
2235
- return fiberInstance;
2225
}
2226
2227
// Running state of the remaining children from the previous version of this parent that
@@ -2308,10 +2297,7 @@ export function attach(
2297
function unmountRemainingChildren() {
2298
let child = remainingReconcilingChildren;
2299
while (child !== null) {
2311
- if (child.kind === FIBER_INSTANCE) {
2312
- unmountFiberRecursively(child.data);
2313
- }
2314
- removeChild(child);
2300
+ unmountInstanceRecursively(child);
2301
child = remainingReconcilingChildren;
2302
}
2303
}
@@ -2434,69 +2420,33 @@ export function attach(
2420
2421
// We use this to simulate unmounting for Suspense trees
2422
// when we switch from primary to fallback, or deleting a subtree.
2437
- function unmountFiberRecursively(fiber: Fiber) {
2423
+ function unmountInstanceRecursively(instance: DevToolsInstance) {
2424
if (__DEBUG__) {
2439
- debug('unmountFiberRecursively()', fiber, null);
2425
+ if (instance.kind === FIBER_INSTANCE) {
2426
+ debug('unmountInstanceRecursively()', instance.data, null);
2427
+ }
2428
}
2429
2442
- let fiberInstance = null;
2443
-
2444
- const shouldIncludeInTree = !shouldFilterFiber(fiber);
2430
const stashedParent = reconcilingParent;
2431
const stashedPrevious = previouslyReconciledSibling;
2432
const stashedRemaining = remainingReconcilingChildren;
2448
- if (shouldIncludeInTree) {
2449
- fiberInstance = getFiberInstanceThrows(fiber);
2450
- // Push a new DevTools instance parent while reconciling this subtree.
2451
- reconcilingParent = fiberInstance;
2452
- previouslyReconciledSibling = null;
2453
- // Move all the children of this instance to the remaining set.
2454
- // We'll move them back one by one, and anything that remains is deleted.
2455
- remainingReconcilingChildren = fiberInstance.firstChild;
2456
- fiberInstance.firstChild = null;
2457
- }
2433
+ // Push a new DevTools instance parent while reconciling this subtree.
2434
+ reconcilingParent = instance;
2435
+ previouslyReconciledSibling = null;
2436
+ // Move all the children of this instance to the remaining set.
2437
+ remainingReconcilingChildren = instance.firstChild;
2438
try {
2459
- // We might meet a nested Suspense on our way.
2460
- const isTimedOutSuspense =
2461
- fiber.tag === SuspenseComponent && fiber.memoizedState !== null;
2462
-
2463
- if (fiber.tag === HostHoistable) {
2464
- releaseHostResource(fiber, fiber.memoizedState);
2465
- }
2466
-
2467
- let child = fiber.child;
2468
- if (isTimedOutSuspense) {
2469
- // If it's showing fallback tree, let's traverse it instead.
2470
- const primaryChildFragment = fiber.child;
2471
- const fallbackChildFragment = primaryChildFragment
2472
- ? primaryChildFragment.sibling
2473
- : null;
2474
- // Skip over to the real Fiber child.
2475
- child = fallbackChildFragment ? fallbackChildFragment.child : null;
2476
- }
2477
-
2478
- unmountChildrenRecursively(child);
2439
+ // Unmount the remaining set.
2440
+ unmountRemainingChildren();
2441
} finally {
2480
- if (shouldIncludeInTree) {
2481
- reconcilingParent = stashedParent;
2482
- previouslyReconciledSibling = stashedPrevious;
2483
- remainingReconcilingChildren = stashedRemaining;
2484
- }
2485
- }
2486
- if (fiberInstance !== null) {
2487
- recordUnmount(fiber);
2488
- removeChild(fiberInstance);
2442
+ reconcilingParent = stashedParent;
2443
+ previouslyReconciledSibling = stashedPrevious;
2444
+ remainingReconcilingChildren = stashedRemaining;
2445
}
2490
- }
2491
-
2492
- function unmountChildrenRecursively(firstChild: null | Fiber) {
2493
- let child: null | Fiber = firstChild;
2494
- while (child !== null) {
2495
- // Record simulated unmounts children-first.
2496
- // We skip nodes without return because those are real unmounts.
2497
- unmountFiberRecursively(child);
2498
- child = child.sibling;
2446
+ if (instance.kind === FIBER_INSTANCE) {
2447
+ recordUnmount(instance);
2448
}
2449
+ removeChild(instance);
2450
}
2451
2452
function recordProfilingDurations(fiber: Fiber) {
@@ -2843,7 +2793,8 @@ export function attach(
2793
} else if (!prevDidTimeout && nextDidTimeOut) {
2794
// Primary -> Fallback:
2795
// 1. Hide primary set
2846
- unmountChildrenRecursively(prevFiber.child);
2796
+ // We simply don't re-add the fallback children and let
2797
+ // unmountRemainingChildren() handle it.
2798
// 2. Mount fallback set
2799
const nextFiberChild = nextFiber.child;
2800
const nextFallbackChildSet = nextFiberChild
@@ -3053,19 +3004,19 @@ export function attach(
3004
const current = root.current;
3005
const alternate = current.alternate;
3006
3056
- const existingRoot =
3007
+ let rootInstance =
3008
fiberToFiberInstanceMap.get(current) ||
3009
(alternate && fiberToFiberInstanceMap.get(alternate));
3059
- if (!existingRoot) {
3060
- const newRoot = createFiberInstance(current);
3061
- idToDevToolsInstanceMap.set(newRoot.id, newRoot);
3062
- fiberToFiberInstanceMap.set(current, newRoot);
3010
+ if (!rootInstance) {
3011
+ rootInstance = createFiberInstance(current);
3012
+ idToDevToolsInstanceMap.set(rootInstance.id, rootInstance);
3013
+ fiberToFiberInstanceMap.set(current, rootInstance);
3014
if (alternate) {
3064
- fiberToFiberInstanceMap.set(alternate, newRoot);
3015
+ fiberToFiberInstanceMap.set(alternate, rootInstance);
3016
}
3066
- currentRootID = newRoot.id;
3017
+ currentRootID = rootInstance.id;
3018
} else {
3068
- currentRootID = existingRoot.id;
3019
+ currentRootID = rootInstance.id;
3020
}
3021
3022
// Before the traversals, remember to start tracking
@@ -3123,7 +3074,7 @@ export function attach(
3074
} else if (wasMounted && !isMounted) {
3075
// Unmount an existing root.
3076
removeRootPseudoKey(currentRootID);
3126
- unmountFiberRecursively(alternate);
3077
+ unmountInstanceRecursively(rootInstance);
3078
}
3079
} else {
3080
// Mount a new root.