From ad01a2c753bbda53ed1fe286c3cfbc917f0f62c3 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 2 Aug 2024 14:56:35 -0400 Subject: [PATCH 1/4] Rename mountFiberRecursively to mountChildrenRecursively This is because this takes a child set not a specific Fiber. This makes it unlike updateFiberRecursively that takes the parent and then recurses. --- .../src/backend/fiber/renderer.js | 29 +++++++++---------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index 75cf2c7c03755..dc9384f6b9345 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -1094,7 +1094,7 @@ export function attach( hook.getFiberRoots(rendererID).forEach(root => { currentRootID = getOrGenerateFiberInstance(root.current).id; setRootPseudoKey(currentRootID, root.current); - mountFiberRecursively(root.current, null, false, false); + mountChildrenRecursively(root.current, null, false, false); flushPendingEvents(root); currentRootID = -1; }); @@ -2228,7 +2228,7 @@ export function attach( } } - function mountFiberRecursively( + function mountChildrenRecursively( firstChild: Fiber, parentInstance: DevToolsInstance | null, traverseSiblings: boolean, @@ -2243,7 +2243,7 @@ export function attach( getOrGenerateFiberInstance(fiber); if (__DEBUG__) { - debug('mountFiberRecursively()', fiber, parentInstance); + debug('mountChildrenRecursively()', fiber, parentInstance); } // If we have the tree selection from previous reload, try to match this Fiber. @@ -2270,8 +2270,7 @@ export function attach( // because we don't want to highlight every host node inside of a newly mounted subtree. } - const isSuspense = fiber.tag === ReactTypeOfWork.SuspenseComponent; - if (isSuspense) { + if (fiber.tag === SuspenseComponent) { const isTimedOut = fiber.memoizedState !== null; if (isTimedOut) { // Special case: if Suspense mounts in a timed-out state, @@ -2285,7 +2284,7 @@ export function attach( ? fallbackChildFragment.child : null; if (fallbackChild !== null) { - mountFiberRecursively( + mountChildrenRecursively( fallbackChild, newParentInstance, true, @@ -2302,7 +2301,7 @@ export function attach( primaryChild = fiber.child.child; } if (primaryChild !== null) { - mountFiberRecursively( + mountChildrenRecursively( primaryChild, newParentInstance, true, @@ -2312,7 +2311,7 @@ export function attach( } } else { if (fiber.child !== null) { - mountFiberRecursively( + mountChildrenRecursively( fiber.child, newParentInstance, true, @@ -2578,7 +2577,7 @@ export function attach( : null; if (prevFallbackChildSet == null && nextFallbackChildSet != null) { - mountFiberRecursively( + mountChildrenRecursively( nextFallbackChildSet, newParentInstance, true, @@ -2607,7 +2606,7 @@ export function attach( // 2. Mount primary set const nextPrimaryChildSet = nextFiber.child; if (nextPrimaryChildSet !== null) { - mountFiberRecursively( + mountChildrenRecursively( nextPrimaryChildSet, newParentInstance, true, @@ -2627,7 +2626,7 @@ export function attach( ? nextFiberChild.sibling : null; if (nextFallbackChildSet != null) { - mountFiberRecursively( + mountChildrenRecursively( nextFallbackChildSet, newParentInstance, true, @@ -2670,7 +2669,7 @@ export function attach( shouldResetChildren = true; } } else { - mountFiberRecursively( + mountChildrenRecursively( nextChild, newParentInstance, false, @@ -2799,7 +2798,7 @@ export function attach( }; } - mountFiberRecursively(root.current, null, false, false); + mountChildrenRecursively(root.current, null, false, false); flushPendingEvents(root); currentRootID = -1; }); @@ -2898,7 +2897,7 @@ export function attach( if (!wasMounted && isMounted) { // Mount a new root. setRootPseudoKey(currentRootID, current); - mountFiberRecursively(current, null, false, false); + mountChildrenRecursively(current, null, false, false); } else if (wasMounted && isMounted) { // Update an existing root. updateFiberRecursively(current, alternate, null, false); @@ -2910,7 +2909,7 @@ export function attach( } else { // Mount a new root. setRootPseudoKey(currentRootID, current); - mountFiberRecursively(current, null, false, false); + mountChildrenRecursively(current, null, false, false); } if (isProfiling && isProfilingSupported) { From 6c70be0190bb189a1a465e95ace4d601557bb978 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 2 Aug 2024 15:11:01 -0400 Subject: [PATCH 2/4] Split out updateChildrenRecursively from updateFiberRecursively This is actually the one that's a pair with mountChildrenRecursively --- .../src/backend/fiber/renderer.js | 119 ++++++++++-------- 1 file changed, 70 insertions(+), 49 deletions(-) diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index dc9384f6b9345..813bca16a1698 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -2494,6 +2494,68 @@ export function attach( } } + // Returns whether closest unfiltered fiber parent needs to reset its child list. + function updateChildrenRecursively( + nextFirstChild: null | Fiber, + prevFirstChild: null | Fiber, + parentInstance: DevToolsInstance | null, + traceNearestHostComponentUpdate: boolean, + ): boolean { + let shouldResetChildren = false; + // If the first child is different, we need to traverse them. + // Each next child will be either a new child (mount) or an alternate (update). + let nextChild = nextFirstChild; + let prevChildAtSameIndex = prevFirstChild; + while (nextChild) { + // We already know children will be referentially different because + // they are either new mounts or alternates of previous children. + // Schedule updates and mounts depending on whether alternates exist. + // We don't track deletions here because they are reported separately. + if (nextChild.alternate) { + const prevChild = nextChild.alternate; + if ( + updateFiberRecursively( + nextChild, + prevChild, + parentInstance, + traceNearestHostComponentUpdate, + ) + ) { + // If a nested tree child order changed but it can't handle its own + // child order invalidation (e.g. because it's filtered out like host nodes), + // propagate the need to reset child order upwards to this Fiber. + shouldResetChildren = true; + } + // However we also keep track if the order of the children matches + // the previous order. They are always different referentially, but + // if the instances line up conceptually we'll want to know that. + if (prevChild !== prevChildAtSameIndex) { + shouldResetChildren = true; + } + } else { + mountChildrenRecursively( + nextChild, + parentInstance, + false, + traceNearestHostComponentUpdate, + ); + shouldResetChildren = true; + } + // Try the next child. + nextChild = nextChild.sibling; + // Advance the pointer in the previous list so that we can + // keep comparing if they line up. + if (!shouldResetChildren && prevChildAtSameIndex !== null) { + prevChildAtSameIndex = prevChildAtSameIndex.sibling; + } + } + // If we have no more children, but used to, they don't line up. + if (prevChildAtSameIndex !== null) { + shouldResetChildren = true; + } + return shouldResetChildren; + } + // Returns whether closest unfiltered fiber parent needs to reset its child list. function updateFiberRecursively( nextFiber: Fiber, @@ -2638,55 +2700,14 @@ export function attach( // Common case: Primary -> Primary. // This is the same code path as for non-Suspense fibers. if (nextFiber.child !== prevFiber.child) { - // If the first child is different, we need to traverse them. - // Each next child will be either a new child (mount) or an alternate (update). - let nextChild = nextFiber.child; - let prevChildAtSameIndex = prevFiber.child; - while (nextChild) { - // We already know children will be referentially different because - // they are either new mounts or alternates of previous children. - // Schedule updates and mounts depending on whether alternates exist. - // We don't track deletions here because they are reported separately. - if (nextChild.alternate) { - const prevChild = nextChild.alternate; - if ( - updateFiberRecursively( - nextChild, - prevChild, - newParentInstance, - traceNearestHostComponentUpdate, - ) - ) { - // If a nested tree child order changed but it can't handle its own - // child order invalidation (e.g. because it's filtered out like host nodes), - // propagate the need to reset child order upwards to this Fiber. - shouldResetChildren = true; - } - // However we also keep track if the order of the children matches - // the previous order. They are always different referentially, but - // if the instances line up conceptually we'll want to know that. - if (prevChild !== prevChildAtSameIndex) { - shouldResetChildren = true; - } - } else { - mountChildrenRecursively( - nextChild, - newParentInstance, - false, - traceNearestHostComponentUpdate, - ); - shouldResetChildren = true; - } - // Try the next child. - nextChild = nextChild.sibling; - // Advance the pointer in the previous list so that we can - // keep comparing if they line up. - if (!shouldResetChildren && prevChildAtSameIndex !== null) { - prevChildAtSameIndex = prevChildAtSameIndex.sibling; - } - } - // If we have no more children, but used to, they don't line up. - if (prevChildAtSameIndex !== null) { + if ( + updateChildrenRecursively( + nextFiber.child, + prevFiber.child, + newParentInstance, + traceNearestHostComponentUpdate, + ) + ) { shouldResetChildren = true; } } else { From 435b88ce53f78f6167623a40c895eaab14cfdf31 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 2 Aug 2024 15:20:05 -0400 Subject: [PATCH 3/4] Split mountChildrenRecursively into mountFiberRecursively Instead of using a boolean flag we can just have this be a separate function and follow control flow. --- .../src/backend/fiber/renderer.js | 173 +++++++++--------- 1 file changed, 88 insertions(+), 85 deletions(-) diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index 813bca16a1698..a14b9cf9a9346 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -1094,7 +1094,7 @@ export function attach( hook.getFiberRoots(rendererID).forEach(root => { currentRootID = getOrGenerateFiberInstance(root.current).id; setRootPseudoKey(currentRootID, root.current); - mountChildrenRecursively(root.current, null, false, false); + mountFiberRecursively(root.current, null, false); flushPendingEvents(root); currentRootID = -1; }); @@ -2231,101 +2231,108 @@ export function attach( function mountChildrenRecursively( firstChild: Fiber, parentInstance: DevToolsInstance | null, - traverseSiblings: boolean, traceNearestHostComponentUpdate: boolean, - ) { + ): void { // Iterate over siblings rather than recursing. // This reduces the chance of stack overflow for wide trees (e.g. lists with many items). let fiber: Fiber | null = firstChild; while (fiber !== null) { - // Generate an ID even for filtered Fibers, in case it's needed later (e.g. for Profiling). - // TODO: Do we really need to do this eagerly? - getOrGenerateFiberInstance(fiber); + mountFiberRecursively( + fiber, + parentInstance, + traceNearestHostComponentUpdate, + ); + fiber = fiber.sibling; + } + } - if (__DEBUG__) { - debug('mountChildrenRecursively()', fiber, parentInstance); - } + function mountFiberRecursively( + fiber: Fiber, + parentInstance: DevToolsInstance | null, + traceNearestHostComponentUpdate: boolean, + ): void { + // Generate an ID even for filtered Fibers, in case it's needed later (e.g. for Profiling). + // TODO: Do we really need to do this eagerly? + getOrGenerateFiberInstance(fiber); - // If we have the tree selection from previous reload, try to match this Fiber. - // Also remember whether to do the same for siblings. - const mightSiblingsBeOnTrackedPath = - updateTrackedPathStateBeforeMount(fiber); - - const shouldIncludeInTree = !shouldFilterFiber(fiber); - const newParentInstance = shouldIncludeInTree - ? recordMount(fiber, parentInstance) - : parentInstance; - - if (traceUpdatesEnabled) { - if (traceNearestHostComponentUpdate) { - const elementType = getElementTypeForFiber(fiber); - // If an ancestor updated, we should mark the nearest host nodes for highlighting. - if (elementType === ElementTypeHostComponent) { - traceUpdatesForNodes.add(fiber.stateNode); - traceNearestHostComponentUpdate = false; - } - } + if (__DEBUG__) { + debug('mountFiberRecursively()', fiber, parentInstance); + } + + // If we have the tree selection from previous reload, try to match this Fiber. + // Also remember whether to do the same for siblings. + const mightSiblingsBeOnTrackedPath = + updateTrackedPathStateBeforeMount(fiber); - // We intentionally do not re-enable the traceNearestHostComponentUpdate flag in this branch, - // because we don't want to highlight every host node inside of a newly mounted subtree. + const shouldIncludeInTree = !shouldFilterFiber(fiber); + const newParentInstance = shouldIncludeInTree + ? recordMount(fiber, parentInstance) + : parentInstance; + + if (traceUpdatesEnabled) { + if (traceNearestHostComponentUpdate) { + const elementType = getElementTypeForFiber(fiber); + // If an ancestor updated, we should mark the nearest host nodes for highlighting. + if (elementType === ElementTypeHostComponent) { + traceUpdatesForNodes.add(fiber.stateNode); + traceNearestHostComponentUpdate = false; + } } - if (fiber.tag === SuspenseComponent) { - const isTimedOut = fiber.memoizedState !== null; - if (isTimedOut) { - // Special case: if Suspense mounts in a timed-out state, - // get the fallback child from the inner fragment and mount - // it as if it was our own child. Updates handle this too. - const primaryChildFragment = fiber.child; - const fallbackChildFragment = primaryChildFragment - ? primaryChildFragment.sibling - : null; - const fallbackChild = fallbackChildFragment - ? fallbackChildFragment.child - : null; - if (fallbackChild !== null) { - mountChildrenRecursively( - fallbackChild, - newParentInstance, - true, - traceNearestHostComponentUpdate, - ); - } - } else { - let primaryChild: Fiber | null = null; - const areSuspenseChildrenConditionallyWrapped = - OffscreenComponent === -1; - if (areSuspenseChildrenConditionallyWrapped) { - primaryChild = fiber.child; - } else if (fiber.child !== null) { - primaryChild = fiber.child.child; - } - if (primaryChild !== null) { - mountChildrenRecursively( - primaryChild, - newParentInstance, - true, - traceNearestHostComponentUpdate, - ); - } + // We intentionally do not re-enable the traceNearestHostComponentUpdate flag in this branch, + // because we don't want to highlight every host node inside of a newly mounted subtree. + } + + if (fiber.tag === SuspenseComponent) { + const isTimedOut = fiber.memoizedState !== null; + if (isTimedOut) { + // Special case: if Suspense mounts in a timed-out state, + // get the fallback child from the inner fragment and mount + // it as if it was our own child. Updates handle this too. + const primaryChildFragment = fiber.child; + const fallbackChildFragment = primaryChildFragment + ? primaryChildFragment.sibling + : null; + const fallbackChild = fallbackChildFragment + ? fallbackChildFragment.child + : null; + if (fallbackChild !== null) { + mountChildrenRecursively( + fallbackChild, + newParentInstance, + traceNearestHostComponentUpdate, + ); } } else { - if (fiber.child !== null) { + let primaryChild: Fiber | null = null; + const areSuspenseChildrenConditionallyWrapped = + OffscreenComponent === -1; + if (areSuspenseChildrenConditionallyWrapped) { + primaryChild = fiber.child; + } else if (fiber.child !== null) { + primaryChild = fiber.child.child; + } + if (primaryChild !== null) { mountChildrenRecursively( - fiber.child, + primaryChild, newParentInstance, - true, traceNearestHostComponentUpdate, ); } } - - // We're exiting this Fiber now, and entering its siblings. - // If we have selection to restore, we might need to re-activate tracking. - updateTrackedPathStateAfterMount(mightSiblingsBeOnTrackedPath); - - fiber = traverseSiblings ? fiber.sibling : null; + } else { + if (fiber.child !== null) { + mountChildrenRecursively( + fiber.child, + newParentInstance, + traceNearestHostComponentUpdate, + ); + } } + + // We're exiting this Fiber now, and entering its siblings. + // If we have selection to restore, we might need to re-activate tracking. + updateTrackedPathStateAfterMount(mightSiblingsBeOnTrackedPath); } // We use this to simulate unmounting for Suspense trees @@ -2533,10 +2540,9 @@ export function attach( shouldResetChildren = true; } } else { - mountChildrenRecursively( + mountFiberRecursively( nextChild, parentInstance, - false, traceNearestHostComponentUpdate, ); shouldResetChildren = true; @@ -2642,7 +2648,6 @@ export function attach( mountChildrenRecursively( nextFallbackChildSet, newParentInstance, - true, traceNearestHostComponentUpdate, ); @@ -2671,7 +2676,6 @@ export function attach( mountChildrenRecursively( nextPrimaryChildSet, newParentInstance, - true, traceNearestHostComponentUpdate, ); } @@ -2691,7 +2695,6 @@ export function attach( mountChildrenRecursively( nextFallbackChildSet, newParentInstance, - true, traceNearestHostComponentUpdate, ); shouldResetChildren = true; @@ -2819,7 +2822,7 @@ export function attach( }; } - mountChildrenRecursively(root.current, null, false, false); + mountFiberRecursively(root.current, null, false); flushPendingEvents(root); currentRootID = -1; }); @@ -2918,7 +2921,7 @@ export function attach( if (!wasMounted && isMounted) { // Mount a new root. setRootPseudoKey(currentRootID, current); - mountChildrenRecursively(current, null, false, false); + mountFiberRecursively(current, null, false); } else if (wasMounted && isMounted) { // Update an existing root. updateFiberRecursively(current, alternate, null, false); @@ -2930,7 +2933,7 @@ export function attach( } else { // Mount a new root. setRootPseudoKey(currentRootID, current); - mountChildrenRecursively(current, null, false, false); + mountFiberRecursively(current, null, false); } if (isProfiling && isProfilingSupported) { From 2fcf50b0d4530ab4d6a0205f0217f7e2fa413f3c Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 2 Aug 2024 15:43:29 -0400 Subject: [PATCH 4/4] Same thing for unmountFiberChildrenRecursively --- .../src/backend/fiber/renderer.js | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index a14b9cf9a9346..db45060a8e47f 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -2337,9 +2337,9 @@ export function attach( // We use this to simulate unmounting for Suspense trees // when we switch from primary to fallback. - function unmountFiberChildrenRecursively(fiber: Fiber) { + function unmountFiberRecursively(fiber: Fiber) { if (__DEBUG__) { - debug('unmountFiberChildrenRecursively()', fiber, null); + debug('unmountFiberRecursively()', fiber, null); } // We might meet a nested Suspense on our way. @@ -2358,11 +2358,16 @@ export function attach( child = fallbackChildFragment ? fallbackChildFragment.child : null; } + unmountChildrenRecursively(child); + } + + 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. if (child.return !== null) { - unmountFiberChildrenRecursively(child); + unmountFiberRecursively(child); recordUnmount(child, true); } child = child.sibling; @@ -2685,7 +2690,7 @@ export function attach( // 1. Hide primary set // This is not a real unmount, so it won't get reported by React. // We need to manually walk the previous tree and record unmounts. - unmountFiberChildrenRecursively(prevFiber); + unmountFiberRecursively(prevFiber); // 2. Mount fallback set const nextFiberChild = nextFiber.child; const nextFallbackChildSet = nextFiberChild