@samitouri / QOS-React-2 / commits / a0d6b155dc

DevTools should iterate over siblings during mount (#21377)

Previously, DevTools recursed over both children and siblings during mount. This caused potential stack overflows when there were a lot of children (e.g. a list containing many items). Given the following example component tree: A B C D E F G A method that recurses for every child and sibling leads to a max depth of 6: A A -> B A -> B -> E A -> B -> C A -> B -> C -> D A -> B -> C -> D -> F A -> B -> C -> D -> F -> G The stack gets deeper as the tree gets either deeper or wider. A method that recurses for every child and iterates over siblings leads to a max depth of 4: A A -> B A -> B -> E A -> C A -> D A -> D -> F A -> D -> F -> G The stack gets deeper as the tree gets deeper but is resilient to wide trees (e.g. lists containing many items).

Brian Vaughn committed Apr 28, 2021 at 10:29 UTC a0d6b155dc3a182162b9f91baac33df39d7919df
1 file changed +73 -75
packages/react-devtools-shared/src/backend/renderer.js
+73 -75
@@ -1647,102 +1647,100 @@ export function attach(
1647 }
1648
1649 function mountFiberRecursively(
1650 - fiber: Fiber,
1650 + firstChild: Fiber,
1651 parentFiber: Fiber | null,
1652 traverseSiblings: boolean,
1653 traceNearestHostComponentUpdate: boolean,
1654 ) {
1655 - if (__DEBUG__) {
1656 - debug('mountFiberRecursively()', fiber, parentFiber);
1657 - }
1655 + // Iterate over siblings rather than recursing.
1656 + // This reduces the chance of stack overflow for wide trees (e.g. lists with many items).
1657 + let fiber: Fiber | null = firstChild;
1658 + while (fiber !== null) {
1659 + if (__DEBUG__) {
1660 + debug('mountFiberRecursively()', fiber, parentFiber);
1661 + }
1662
1659 - // If we have the tree selection from previous reload, try to match this Fiber.
1660 - // Also remember whether to do the same for siblings.
1661 - const mightSiblingsBeOnTrackedPath = updateTrackedPathStateBeforeMount(
1662 - fiber,
1663 - );
1663 + // If we have the tree selection from previous reload, try to match this Fiber.
1664 + // Also remember whether to do the same for siblings.
1665 + const mightSiblingsBeOnTrackedPath = updateTrackedPathStateBeforeMount(
1666 + fiber,
1667 + );
1668
1665 - const shouldIncludeInTree = !shouldFilterFiber(fiber);
1666 - if (shouldIncludeInTree) {
1667 - recordMount(fiber, parentFiber);
1668 - }
1669 + const shouldIncludeInTree = !shouldFilterFiber(fiber);
1670 + if (shouldIncludeInTree) {
1671 + recordMount(fiber, parentFiber);
1672 + }
1673
1670 - if (traceUpdatesEnabled) {
1671 - if (traceNearestHostComponentUpdate) {
1672 - const elementType = getElementTypeForFiber(fiber);
1673 - // If an ancestor updated, we should mark the nearest host nodes for highlighting.
1674 - if (elementType === ElementTypeHostComponent) {
1675 - traceUpdatesForNodes.add(fiber.stateNode);
1676 - traceNearestHostComponentUpdate = false;
1674 + if (traceUpdatesEnabled) {
1675 + if (traceNearestHostComponentUpdate) {
1676 + const elementType = getElementTypeForFiber(fiber);
1677 + // If an ancestor updated, we should mark the nearest host nodes for highlighting.
1678 + if (elementType === ElementTypeHostComponent) {
1679 + traceUpdatesForNodes.add(fiber.stateNode);
1680 + traceNearestHostComponentUpdate = false;
1681 + }
1682 }
1678 - }
1683
1680 - // We intentionally do not re-enable the traceNearestHostComponentUpdate flag in this branch,
1681 - // because we don't want to highlight every host node inside of a newly mounted subtree.
1682 - }
1684 + // We intentionally do not re-enable the traceNearestHostComponentUpdate flag in this branch,
1685 + // because we don't want to highlight every host node inside of a newly mounted subtree.
1686 + }
1687
1684 - const isSuspense = fiber.tag === ReactTypeOfWork.SuspenseComponent;
1685 - if (isSuspense) {
1686 - const isTimedOut = fiber.memoizedState !== null;
1687 - if (isTimedOut) {
1688 - // Special case: if Suspense mounts in a timed-out state,
1689 - // get the fallback child from the inner fragment and mount
1690 - // it as if it was our own child. Updates handle this too.
1691 - const primaryChildFragment = fiber.child;
1692 - const fallbackChildFragment = primaryChildFragment
1693 - ? primaryChildFragment.sibling
1694 - : null;
1695 - const fallbackChild = fallbackChildFragment
1696 - ? fallbackChildFragment.child
1697 - : null;
1698 - if (fallbackChild !== null) {
1699 - mountFiberRecursively(
1700 - fallbackChild,
1701 - shouldIncludeInTree ? fiber : parentFiber,
1702 - true,
1703 - traceNearestHostComponentUpdate,
1704 - );
1688 + const isSuspense = fiber.tag === ReactTypeOfWork.SuspenseComponent;
1689 + if (isSuspense) {
1690 + const isTimedOut = fiber.memoizedState !== null;
1691 + if (isTimedOut) {
1692 + // Special case: if Suspense mounts in a timed-out state,
1693 + // get the fallback child from the inner fragment and mount
1694 + // it as if it was our own child. Updates handle this too.
1695 + const primaryChildFragment = fiber.child;
1696 + const fallbackChildFragment = primaryChildFragment
1697 + ? primaryChildFragment.sibling
1698 + : null;
1699 + const fallbackChild = fallbackChildFragment
1700 + ? fallbackChildFragment.child
1701 + : null;
1702 + if (fallbackChild !== null) {
1703 + mountFiberRecursively(
1704 + fallbackChild,
1705 + shouldIncludeInTree ? fiber : parentFiber,
1706 + true,
1707 + traceNearestHostComponentUpdate,
1708 + );
1709 + }
1710 + } else {
1711 + let primaryChild: Fiber | null = null;
1712 + const areSuspenseChildrenConditionallyWrapped =
1713 + OffscreenComponent === -1;
1714 + if (areSuspenseChildrenConditionallyWrapped) {
1715 + primaryChild = fiber.child;
1716 + } else if (fiber.child !== null) {
1717 + primaryChild = fiber.child.child;
1718 + }
1719 + if (primaryChild !== null) {
1720 + mountFiberRecursively(
1721 + primaryChild,
1722 + shouldIncludeInTree ? fiber : parentFiber,
1723 + true,
1724 + traceNearestHostComponentUpdate,
1725 + );
1726 + }
1727 }
1728 } else {
1707 - let primaryChild: Fiber | null = null;
1708 - const areSuspenseChildrenConditionallyWrapped =
1709 - OffscreenComponent === -1;
1710 - if (areSuspenseChildrenConditionallyWrapped) {
1711 - primaryChild = fiber.child;
1712 - } else if (fiber.child !== null) {
1713 - primaryChild = fiber.child.child;
1714 - }
1715 - if (primaryChild !== null) {
1729 + if (fiber.child !== null) {
1730 mountFiberRecursively(
1717 - primaryChild,
1731 + fiber.child,
1732 shouldIncludeInTree ? fiber : parentFiber,
1733 true,
1734 traceNearestHostComponentUpdate,
1735 );
1736 }
1737 }
1724 - } else {
1725 - if (fiber.child !== null) {
1726 - mountFiberRecursively(
1727 - fiber.child,
1728 - shouldIncludeInTree ? fiber : parentFiber,
1729 - true,
1730 - traceNearestHostComponentUpdate,
1731 - );
1732 - }
1733 - }
1738
1735 - // We're exiting this Fiber now, and entering its siblings.
1736 - // If we have selection to restore, we might need to re-activate tracking.
1737 - updateTrackedPathStateAfterMount(mightSiblingsBeOnTrackedPath);
1739 + // We're exiting this Fiber now, and entering its siblings.
1740 + // If we have selection to restore, we might need to re-activate tracking.
1741 + updateTrackedPathStateAfterMount(mightSiblingsBeOnTrackedPath);
1742
1739 - if (traverseSiblings && fiber.sibling !== null) {
1740 - mountFiberRecursively(
1741 - fiber.sibling,
1742 - parentFiber,
1743 - true,
1744 - traceNearestHostComponentUpdate,
1745 - );
1743 + fiber = traverseSiblings ? fiber.sibling : null;
1744 }
1745 }
1746