[DevTools] Unmount by walking previous nodes no longer in the new tree (#30644)
This no longer uses the handleCommitFiberUnmount hook to track unmounts. Instead, we can just unmount the DevToolsInstances that we didn't reuse. This doesn't account for cleaning up instances that were unnecessarily created when they weren't in the tree. I have a separate follow up for that. This also removes the queuing of untracking. This was added in #21523 but I don't believe it has been needed for a while because the mentioned flushPendingErrorsAndWarningsAfterDelay hasn't called untrackFiberID for a while so the race condition doesn't exist. It's hard to tell though because from the issues there weren't really any repros submitted.
Sebastian Markbåge committed
Aug 12, 2024 at 00:35 UTC
68dbd84b61cc2504c30e19f748f59a52d331f851
1 file changed
+51
-92
packages/react-devtools-shared/src/backend/fiber/renderer.js
+51
-92
@@ -1405,81 +1405,37 @@ export function attach(
1405
1406
// Removes a Fiber (and its alternate) from the Maps used to track their id.
1407
// This method should always be called when a Fiber is unmounting.
1408
- function untrackFiberID(fiber: Fiber) {
1408
+ function untrackFiber(fiberInstance: FiberInstance) {
1409
if (__DEBUG__) {
1410
- debug('untrackFiberID()', fiber, null, 'schedule after delay');
1410
+ debug('untrackFiber()', fiberInstance.data, null);
1411
}
1412
1413
- // Untrack Fibers after a slight delay in order to support a Fast Refresh edge case:
1414
- // 1. Component type is updated and Fast Refresh schedules an update+remount.
1415
- // 2. flushPendingErrorsAndWarningsAfterDelay() runs, sees the old Fiber is no longer mounted
1416
- // (it's been disconnected by Fast Refresh), and calls untrackFiberID() to clear it from the Map.
1417
- // 3. React flushes pending passive effects before it runs the next render,
1418
- // which logs an error or warning, which causes a new ID to be generated for this Fiber.
1419
- // 4. DevTools now tries to unmount the old Component with the new ID.
1420
- //
1421
- // The underlying problem here is the premature clearing of the Fiber ID,
1422
- // but DevTools has no way to detect that a given Fiber has been scheduled for Fast Refresh.
1423
- // (The "_debugNeedsRemount" flag won't necessarily be set.)
1424
- //
1425
- // The best we can do is to delay untracking by a small amount,
1426
- // and give React time to process the Fast Refresh delay.
1427
-
1428
- untrackFibersSet.add(fiber);
1413
+ idToDevToolsInstanceMap.delete(fiberInstance.id);
1414
1430
- // React may detach alternate pointers during unmount;
1431
- // Since our untracking code is async, we should explicily track the pending alternate here as well.
1432
- const alternate = fiber.alternate;
1433
- if (alternate !== null) {
1434
- untrackFibersSet.add(alternate);
1415
+ // Also clear any errors/warnings associated with this fiber.
1416
+ clearErrorsForElementID(fiberInstance.id);
1417
+ clearWarningsForElementID(fiberInstance.id);
1418
+ if (fiberInstance.flags & FORCE_ERROR) {
1419
+ fiberInstance.flags &= ~FORCE_ERROR;
1420
+ forceErrorCount--;
1421
+ if (forceErrorCount === 0 && setErrorHandler != null) {
1422
+ setErrorHandler(shouldErrorFiberAlwaysNull);
1423
+ }
1424
}
1436
-
1437
- if (untrackFibersTimeoutID === null) {
1438
- untrackFibersTimeoutID = setTimeout(untrackFibers, 1000);
1425
+ if (fiberInstance.flags & FORCE_SUSPENSE_FALLBACK) {
1426
+ fiberInstance.flags &= ~FORCE_SUSPENSE_FALLBACK;
1427
+ forceFallbackCount--;
1428
+ if (forceFallbackCount === 0 && setSuspenseHandler != null) {
1429
+ setSuspenseHandler(shouldSuspendFiberAlwaysFalse);
1430
+ }
1431
}
1440
- }
1441
-
1442
- const untrackFibersSet: Set<Fiber> = new Set();
1443
- let untrackFibersTimeoutID: TimeoutID | null = null;
1432
1445
- function untrackFibers() {
1446
- if (untrackFibersTimeoutID !== null) {
1447
- clearTimeout(untrackFibersTimeoutID);
1448
- untrackFibersTimeoutID = null;
1433
+ const fiber = fiberInstance.data;
1434
+ fiberToFiberInstanceMap.delete(fiber);
1435
+ const {alternate} = fiber;
1436
+ if (alternate !== null) {
1437
+ fiberToFiberInstanceMap.delete(alternate);
1438
}
1450
-
1451
- untrackFibersSet.forEach(fiber => {
1452
- const fiberInstance = fiberToFiberInstanceMap.get(fiber);
1453
- if (fiberInstance !== undefined) {
1454
- idToDevToolsInstanceMap.delete(fiberInstance.id);
1455
-
1456
- // Also clear any errors/warnings associated with this fiber.
1457
- clearErrorsForElementID(fiberInstance.id);
1458
- clearWarningsForElementID(fiberInstance.id);
1459
- if (fiberInstance.flags & FORCE_ERROR) {
1460
- fiberInstance.flags &= ~FORCE_ERROR;
1461
- forceErrorCount--;
1462
- if (forceErrorCount === 0 && setErrorHandler != null) {
1463
- setErrorHandler(shouldErrorFiberAlwaysNull);
1464
- }
1465
- }
1466
- if (fiberInstance.flags & FORCE_SUSPENSE_FALLBACK) {
1467
- fiberInstance.flags &= ~FORCE_SUSPENSE_FALLBACK;
1468
- forceFallbackCount--;
1469
- if (forceFallbackCount === 0 && setSuspenseHandler != null) {
1470
- setSuspenseHandler(shouldSuspendFiberAlwaysFalse);
1471
- }
1472
- }
1473
- }
1474
-
1475
- fiberToFiberInstanceMap.delete(fiber);
1476
-
1477
- const {alternate} = fiber;
1478
- if (alternate !== null) {
1479
- fiberToFiberInstanceMap.delete(alternate);
1480
- }
1481
- });
1482
- untrackFibersSet.clear();
1439
}
1440
1441
function getChangeDescription(
@@ -2054,7 +2010,7 @@ export function attach(
2010
// Fill in the real unmounts in the reverse order.
2011
// They were inserted parents-first by React, but we want children-first.
2012
// So we traverse our array backwards.
2057
- for (let j = pendingRealUnmountedIDs.length - 1; j >= 0; j--) {
2013
+ for (let j = 0; j < pendingRealUnmountedIDs.length; j++) {
2014
operations[i++] = pendingRealUnmountedIDs[j];
2015
}
2016
// Fill in the simulated unmounts (hidden Suspense subtrees) in their order.
@@ -2282,7 +2238,7 @@ export function attach(
2238
}
2239
2240
if (!fiber._debugNeedsRemount) {
2285
- untrackFiberID(fiber);
2241
+ untrackFiber(fiberInstance);
2242
2243
const isProfilingSupported = fiber.hasOwnProperty('treeBaseDuration');
2244
if (isProfilingSupported) {
@@ -2363,6 +2319,17 @@ export function attach(
2319
instance.parent = null;
2320
}
2321
2322
+ function unmountRemainingChildren() {
2323
+ let child = remainingReconcilingChildren;
2324
+ while (child !== null) {
2325
+ if (child.kind === FIBER_INSTANCE) {
2326
+ unmountFiberRecursively(child.data, false);
2327
+ }
2328
+ removeChild(child);
2329
+ child = remainingReconcilingChildren;
2330
+ }
2331
+ }
2332
+
2333
function mountChildrenRecursively(
2334
firstChild: Fiber,
2335
traceNearestHostComponentUpdate: boolean,
@@ -2484,8 +2451,8 @@ export function attach(
2451
}
2452
2453
// We use this to simulate unmounting for Suspense trees
2487
- // when we switch from primary to fallback.
2488
- function unmountFiberRecursively(fiber: Fiber) {
2454
+ // when we switch from primary to fallback, or deleting a subtree.
2455
+ function unmountFiberRecursively(fiber: Fiber, isSimulated: boolean) {
2456
if (__DEBUG__) {
2457
debug('unmountFiberRecursively()', fiber, null);
2458
}
@@ -2526,7 +2493,7 @@ export function attach(
2493
child = fallbackChildFragment ? fallbackChildFragment.child : null;
2494
}
2495
2529
- unmountChildrenRecursively(child);
2496
+ unmountChildrenRecursively(child, isSimulated);
2497
} finally {
2498
if (shouldIncludeInTree) {
2499
reconcilingParent = stashedParent;
@@ -2535,19 +2502,20 @@ export function attach(
2502
}
2503
}
2504
if (fiberInstance !== null) {
2538
- recordUnmount(fiber, true);
2505
+ recordUnmount(fiber, isSimulated);
2506
removeChild(fiberInstance);
2507
}
2508
}
2509
2543
- function unmountChildrenRecursively(firstChild: null | Fiber) {
2510
+ function unmountChildrenRecursively(
2511
+ firstChild: null | Fiber,
2512
+ isSimulated: boolean,
2513
+ ) {
2514
let child: null | Fiber = firstChild;
2515
while (child !== null) {
2516
// Record simulated unmounts children-first.
2517
// We skip nodes without return because those are real unmounts.
2548
- if (child.return !== null) {
2549
- unmountFiberRecursively(child);
2550
- }
2518
+ unmountFiberRecursively(child, isSimulated);
2519
child = child.sibling;
2520
}
2521
}
@@ -2890,7 +2858,7 @@ export function attach(
2858
// 1. Hide primary set
2859
// This is not a real unmount, so it won't get reported by React.
2860
// We need to manually walk the previous tree and record unmounts.
2893
- unmountChildrenRecursively(prevFiber.child);
2861
+ unmountChildrenRecursively(prevFiber.child, true);
2862
// 2. Mount fallback set
2863
const nextFiberChild = nextFiber.child;
2864
const nextFallbackChildSet = nextFiberChild
@@ -2922,6 +2890,7 @@ export function attach(
2890
// All the remaining children will be children of this same fiber so we can just reuse them.
2891
// I.e. we just restore them by undoing what we did above.
2892
fiberInstance.firstChild = remainingReconcilingChildren;
2893
+ remainingReconcilingChildren = null;
2894
} else {
2895
// If this fiber is filtered there might be changes to this set elsewhere so we have
2896
// to visit each child to place it back in the set. We let the child bail out instead.
@@ -2984,6 +2953,7 @@ export function attach(
2953
}
2954
} finally {
2955
if (shouldIncludeInTree) {
2956
+ unmountRemainingChildren();
2957
reconcilingParent = stashedParent;
2958
previouslyReconciledSibling = stashedPrevious;
2959
remainingReconcilingChildren = stashedRemaining;
@@ -3068,15 +3038,8 @@ export function attach(
3038
}
3039
3040
function handleCommitFiberUnmount(fiber: any) {
3071
- // If the untrackFiberSet already has the unmounted Fiber, this means we've already
3072
- // recordedUnmount, so we don't need to do it again. If we don't do this, we might
3073
- // end up double-deleting Fibers in some cases (like Legacy Suspense).
3074
- if (!untrackFibersSet.has(fiber)) {
3075
- // This is not recursive.
3076
- // We can't traverse fibers after unmounting so instead
3077
- // we rely on React telling us about each unmount.
3078
- recordUnmount(fiber, false);
3079
- }
3041
+ // This Hook is no longer used. After having shipped DevTools everywhere it is
3042
+ // safe to stop calling it from Fiber.
3043
}
3044
3045
function handlePostCommitFiberRoot(root: any) {
@@ -3097,10 +3060,6 @@ export function attach(
3060
const current = root.current;
3061
const alternate = current.alternate;
3062
3100
- // Flush any pending Fibers that we are untracking before processing the new commit.
3101
- // If we don't do this, we might end up double-deleting Fibers in some cases (like Legacy Suspense).
3102
- untrackFibers();
3103
-
3063
currentRootID = getOrGenerateFiberInstance(current).id;
3064
3065
// Before the traversals, remember to start tracking
@@ -3158,7 +3117,7 @@ export function attach(
3117
} else if (wasMounted && !isMounted) {
3118
// Unmount an existing root.
3119
removeRootPseudoKey(currentRootID);
3161
- recordUnmount(current, false);
3120
+ unmountFiberRecursively(alternate, false);
3121
}
3122
} else {
3123
// Mount a new root.