Skip to content

Commit dbfab82

Browse files
author
Brian Vaughn
committed
DevTools should iterate over siblings during mount
Previously, DevTools recursed over 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 deepers 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 deepers, but is resiliant to wide trees (e.g. lists containing many items).
1 parent a5267fa commit dbfab82

File tree

1 file changed

+73
-75
lines changed

1 file changed

+73
-75
lines changed

packages/react-devtools-shared/src/backend/renderer.js

Lines changed: 73 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -1647,102 +1647,100 @@ export function attach(
16471647
}
16481648

16491649
function mountFiberRecursively(
1650-
fiber: Fiber,
1650+
firstChild: Fiber,
16511651
parentFiber: Fiber | null,
16521652
traverseSiblings: boolean,
16531653
traceNearestHostComponentUpdate: boolean,
16541654
) {
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+
}
16581662

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+
);
16641668

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+
}
16691673

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+
}
16771682
}
1678-
}
16791683

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+
}
16831687

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+
}
17051727
}
17061728
} 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) {
17161730
mountFiberRecursively(
1717-
primaryChild,
1731+
fiber.child,
17181732
shouldIncludeInTree ? fiber : parentFiber,
17191733
true,
17201734
traceNearestHostComponentUpdate,
17211735
);
17221736
}
17231737
}
1724-
} else {
1725-
if (fiber.child !== null) {
1726-
mountFiberRecursively(
1727-
fiber.child,
1728-
shouldIncludeInTree ? fiber : parentFiber,
1729-
true,
1730-
traceNearestHostComponentUpdate,
1731-
);
1732-
}
1733-
}
17341738

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);
17381742

1739-
if (traverseSiblings && fiber.sibling !== null) {
1740-
mountFiberRecursively(
1741-
fiber.sibling,
1742-
parentFiber,
1743-
true,
1744-
traceNearestHostComponentUpdate,
1745-
);
1743+
fiber = traverseSiblings ? fiber.sibling : null;
17461744
}
17471745
}
17481746

0 commit comments

Comments
 (0)