Skip to content

Commit

Permalink
findDOMNode: Remove return pointer mutation (facebook#20272)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
acdlite authored and koto committed Jun 15, 2021
1 parent 62fea2c commit 440f8ad
Showing 1 changed file with 37 additions and 51 deletions.
88 changes: 37 additions & 51 deletions packages/react-reconciler/src/ReactFiberTreeReflection.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down

0 comments on commit 440f8ad

Please sign in to comment.