From bf7b7aeb1070f5184733ece11e67902d1ace85bd Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 16 Nov 2020 17:09:53 -0600 Subject: [PATCH] findDOMNode: Remove return pointer mutation (#20272) The last step of the `findDOMNode` algorithm is a search of the current tree. When descending into a child node, it mutates `child.return` so that it points to the current fiber pair, instead of a work-in-progress. This can cause bugs if `findDOMNode` is called at the wrong time, like in an interleaved event. For this reason (among others), you're not suppposed to use `findDOMNode` in Concurrent Mode. However, we still have some internal uses that we haven't migrated. To reduce the potential for bugs, I've removed the `.return` pointer assignment in favor of recursion. --- .../src/ReactFiberTreeReflection.js | 88 ++++++++----------- 1 file changed, 37 insertions(+), 51 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberTreeReflection.js b/packages/react-reconciler/src/ReactFiberTreeReflection.js index caf503d19ee62..0b3448fec2b09 100644 --- a/packages/react-reconciler/src/ReactFiberTreeReflection.js +++ b/packages/react-reconciler/src/ReactFiberTreeReflection.js @@ -265,71 +265,57 @@ export function findCurrentFiberUsingSlowPath(fiber: Fiber): Fiber | null { export function findCurrentHostFiber(parent: Fiber): Fiber | null { const currentParent = findCurrentFiberUsingSlowPath(parent); - if (!currentParent) { - return null; - } + return currentParent !== null + ? findCurrentHostFiberImpl(currentParent) + : null; +} +function findCurrentHostFiberImpl(node: Fiber) { // Next we'll drill down this component to find the first HostComponent/Text. - let node: Fiber = currentParent; - while (true) { - if (node.tag === HostComponent || node.tag === HostText) { - return node; - } else if (node.child) { - node.child.return = node; - node = node.child; - continue; - } - if (node === currentParent) { - return null; - } - while (!node.sibling) { - if (!node.return || node.return === currentParent) { - return null; - } - node = node.return; + if (node.tag === HostComponent || node.tag === HostText) { + return node; + } + + let child = node.child; + while (child !== null) { + const match = findCurrentHostFiberImpl(child); + if (match !== null) { + return match; } - node.sibling.return = node.return; - node = node.sibling; + child = child.sibling; } - // Flow needs the return null here, but ESLint complains about it. - // eslint-disable-next-line no-unreachable + return null; } export function findCurrentHostFiberWithNoPortals(parent: Fiber): Fiber | null { const currentParent = findCurrentFiberUsingSlowPath(parent); - if (!currentParent) { - return null; - } + return currentParent !== null + ? findCurrentHostFiberWithNoPortalsImpl(currentParent) + : null; +} +function findCurrentHostFiberWithNoPortalsImpl(node: Fiber) { // Next we'll drill down this component to find the first HostComponent/Text. - let node: Fiber = currentParent; - while (true) { - if ( - node.tag === HostComponent || - node.tag === HostText || - (enableFundamentalAPI && node.tag === FundamentalComponent) - ) { - return node; - } else if (node.child && node.tag !== HostPortal) { - node.child.return = node; - node = node.child; - continue; - } - if (node === currentParent) { - return null; - } - while (!node.sibling) { - if (!node.return || node.return === currentParent) { - return null; + if ( + node.tag === HostComponent || + node.tag === HostText || + (enableFundamentalAPI && node.tag === FundamentalComponent) + ) { + return node; + } + + let child = node.child; + while (child !== null) { + if (child.tag !== HostPortal) { + const match = findCurrentHostFiberWithNoPortalsImpl(child); + if (match !== null) { + return match; } - node = node.return; } - node.sibling.return = node.return; - node = node.sibling; + child = child.sibling; } - // Flow needs the return null here, but ESLint complains about it. - // eslint-disable-next-line no-unreachable + return null; }