Skip to content

Commit ffba29a

Browse files
sebmarkbagesebmarkbage
committed
Fix findDOMNode and findAllInRenderedFiberTreeInternal
This strategy finds the current fiber. It traverses back up to the root if the two trees are completely separate and determines which one is current. If the two trees converge anywhere along the way, we assume that is the current tree. We find the current child by searching the converged child set. This could fail if there's any way for both return pointers to point backwards to the work in progress. I don't think there is but I could be wrong. This may also fail on coroutines where we have reparenting situations.
1 parent a1feccd commit ffba29a

File tree

4 files changed

+121
-79
lines changed

4 files changed

+121
-79
lines changed

‎scripts/fiber/tests-failing.txt

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,3 @@ src/renderers/shared/shared/__tests__/ReactUpdates-test.js
117117

118118
src/renderers/shared/shared/__tests__/refs-test.js
119119
* Should increase refs with an increase in divs
120-
121-
src/test/__tests__/ReactTestUtils-test.js
122-
* traverses children in the correct order

‎scripts/fiber/tests-passing.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1568,6 +1568,7 @@ src/test/__tests__/ReactTestUtils-test.js
15681568
* can scryRenderedDOMComponentsWithClass with TextComponent
15691569
* can scryRenderedDOMComponentsWithClass with className contains \n
15701570
* can scryRenderedDOMComponentsWithClass with multiple classes
1571+
* traverses children in the correct order
15711572
* should support injected wrapper components as DOM components
15721573
* should change the value of an input field
15731574
* should change the value of an input field in a component

‎src/renderers/shared/fiber/ReactFiberTreeReflection.js

Lines changed: 87 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -71,60 +71,116 @@ exports.isMounted = function(component : ReactComponent<any, any, any>) : boolea
7171
return isFiberMounted(fiber) === MOUNTED;
7272
};
7373

74-
exports.findCurrentHostFiber = function(parent : Fiber) : Fiber | null {
75-
// First check if this node itself is mounted.
76-
const state = isFiberMounted(parent, true);
77-
if (state === UNMOUNTED) {
74+
function assertIsMounted(fiber) {
75+
invariant(
76+
isFiberMounted(fiber) === MOUNTED,
77+
'Unable to find node on an unmounted component.'
78+
);
79+
}
80+
81+
function findCurrentFiberUsingSlowPath(fiber : Fiber) : Fiber | null {
82+
let alternate = fiber.alternate;
83+
if (!alternate) {
84+
// If there is no alternate, then we only need to check if it is mounted.
85+
const state = isFiberMounted(fiber);
7886
invariant(
79-
false,
87+
state !== UNMOUNTED,
8088
'Unable to find node on an unmounted component.'
8189
);
82-
} else if (state === MOUNTING) {
83-
return null;
90+
if (state === MOUNTING) {
91+
return null;
92+
}
93+
return fiber;
8494
}
95+
// If we have two possible branches, we'll walk backwards up to the root
96+
// to see what path the root points to. On the way we may hit one of the
97+
// special cases and we'll deal with them.
98+
let a = fiber;
99+
let b = alternate;
100+
while (true) {
101+
let parentA = a.return;
102+
let parentB = b.return;
103+
if (!parentA || !parentB) {
104+
// We're at the root.
105+
break;
106+
}
107+
if (parentA.child === parentB.child) {
108+
// If both parents are the same, then that is the current parent. If
109+
// they're different but point to the same child, then it doesn't matter.
110+
// Regardless, whatever child they point to is the current child.
111+
// So we can now determine which child is current by scanning the child
112+
// list for either A or B.
113+
let child = parentA.child;
114+
while (child) {
115+
if (child === a) {
116+
// We've determined that A is the current branch.
117+
assertIsMounted(parentA);
118+
return fiber;
119+
}
120+
if (child === b) {
121+
// We've determined that B is the current branch.
122+
assertIsMounted(parentA);
123+
return alternate;
124+
}
125+
child = child.sibling;
126+
}
127+
// We should never have an alternate for any mounting node. So the only
128+
// way this could possibly happen is if this was unmounted, if at all.
129+
invariant(
130+
false,
131+
'Unable to find node on an unmounted component.'
132+
);
133+
}
134+
a = parentA;
135+
b = parentB;
136+
invariant(
137+
a.alternate === b,
138+
'Return fibers should always be each others\' alternates.'
139+
);
140+
}
141+
// If the root is not a host container, we're in a disconnected tree. I.e.
142+
// unmounted.
143+
invariant(
144+
a.tag === HostContainer,
145+
'Unable to find node on an unmounted component.'
146+
);
147+
if (a.stateNode.current === a) {
148+
// We've determined that A is the current branch.
149+
return fiber;
150+
}
151+
// Otherwise B has to be current branch.
152+
return alternate;
153+
}
154+
exports.findCurrentFiberUsingSlowPath = findCurrentFiberUsingSlowPath;
85155

86-
let didTryOtherTree = false;
87-
88-
// If the component doesn't have a child we first check the alternate to see
89-
// if it has any and if so, if those were just recently inserted.
90-
if (!parent.child && parent.alternate) {
91-
parent = parent.alternate;
156+
exports.findCurrentHostFiber = function(parent : Fiber) : Fiber | null {
157+
const currentParent = findCurrentFiberUsingSlowPath(parent);
158+
if (!currentParent) {
159+
return null;
92160
}
93161

94162
// Next we'll drill down this component to find the first HostComponent/Text.
95-
let node : Fiber = parent;
163+
let node : Fiber = currentParent;
96164
while (true) {
97-
if ((node.effectTag & Placement) !== NoEffect || !node.return) {
98-
// If any node along the way was deleted, or is an insertion, that means
99-
// that we're actually in a work in progress to update this component with
100-
// a different component. We need to look in the "current" fiber instead.
101-
if (!parent.alternate) {
102-
return null;
103-
}
104-
if (didTryOtherTree) {
105-
// Safety, to avoid an infinite loop if something goes wrong.
106-
throw new Error('This should never hit this infinite loop.');
107-
}
108-
didTryOtherTree = true;
109-
node = parent = parent.alternate;
110-
continue;
111-
}
112165
if (node.tag === HostComponent || node.tag === HostText) {
113166
return node;
114167
} else if (node.child) {
168+
// TODO: If we hit a Portal, we're supposed to skip it.
115169
// TODO: Coroutines need to visit the stateNode.
170+
node.child.return = node;
116171
node = node.child;
117172
continue;
118173
}
119-
if (node === parent) {
174+
if (node === currentParent) {
120175
return null;
121176
}
122177
while (!node.sibling) {
123-
if (!node.return || node.return === parent) {
178+
if (!node.return || node.return === currentParent) {
124179
return null;
125180
}
126181
node = node.return;
127182
}
183+
node.sibling.return = node.return;
128184
node = node.sibling;
129185
}
130186
// Flow needs the return null here, but ESLint complains about it.

‎src/test/ReactTestUtils.js

Lines changed: 33 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,9 @@ var ReactControlledComponent = require('ReactControlledComponent');
2020
var ReactDOM = require('ReactDOM');
2121
var ReactDOMComponentTree = require('ReactDOMComponentTree');
2222
var ReactBrowserEventEmitter = require('ReactBrowserEventEmitter');
23+
var ReactFiberTreeReflection = require('ReactFiberTreeReflection');
2324
var ReactInstanceMap = require('ReactInstanceMap');
2425
var ReactTypeOfWork = require('ReactTypeOfWork');
25-
var ReactTypeOfSideEffect = require('ReactTypeOfSideEffect');
2626
var ReactGenericBatching = require('ReactGenericBatching');
2727
var SyntheticEvent = require('SyntheticEvent');
2828
var ReactShallowRenderer = require('ReactShallowRenderer');
@@ -37,10 +37,6 @@ var {
3737
HostComponent,
3838
HostText,
3939
} = ReactTypeOfWork;
40-
var {
41-
NoEffect,
42-
Placement,
43-
} = ReactTypeOfSideEffect;
4440

4541
function Event(suffix) {}
4642

@@ -84,34 +80,40 @@ function findAllInRenderedFiberTreeInternal(fiber, test) {
8480
if (!fiber) {
8581
return [];
8682
}
87-
if (
88-
fiber.tag !== ClassComponent &&
89-
fiber.tag !== FunctionalComponent &&
90-
fiber.tag !== HostComponent &&
91-
fiber.tag !== HostText
92-
) {
83+
var currentParent = ReactFiberTreeReflection.findCurrentFiberUsingSlowPath(
84+
fiber
85+
);
86+
if (!currentParent) {
9387
return [];
9488
}
95-
if ((fiber.effectTag & Placement) !== NoEffect || !fiber.return) {
96-
// If any node along the way was deleted, or is an insertion, that means
97-
// that we're actually in a work in progress to update this component with
98-
// a different component. We need to look in the "current" fiber instead.
99-
return null;
100-
}
101-
var publicInst = fiber.stateNode;
102-
var ret = publicInst && test(publicInst) ? [publicInst] : [];
103-
var child = fiber.child;
104-
while (child) {
105-
ret = ret.concat(
106-
findAllInRenderedFiberTreeInternal(
107-
child,
108-
test
109-
)
110-
);
111-
child = child.sibling;
89+
let node = currentParent;
90+
let ret = [];
91+
while (true) {
92+
if (node.tag === HostComponent || node.tag === HostText ||
93+
node.tag === ClassComponent || node.tag === FunctionalComponent) {
94+
var publicInst = node.stateNode;
95+
if (test(publicInst)) {
96+
ret.push(publicInst);
97+
}
98+
}
99+
if (node.child) {
100+
// TODO: Coroutines need to visit the stateNode.
101+
node.child.return = node;
102+
node = node.child;
103+
continue;
104+
}
105+
if (node === currentParent) {
106+
return ret;
107+
}
108+
while (!node.sibling) {
109+
if (!node.return || node.return === currentParent) {
110+
return ret;
111+
}
112+
node = node.return;
113+
}
114+
node.sibling.return = node.return;
115+
node = node.sibling;
112116
}
113-
// TODO: visit stateNode for coroutines
114-
return ret;
115117
}
116118

117119
/**
@@ -222,21 +224,7 @@ var ReactTestUtils = {
222224
);
223225
var internalInstance = ReactInstanceMap.get(inst);
224226
if (internalInstance && typeof internalInstance.tag === 'number') {
225-
var fiber = internalInstance;
226-
if (!fiber.child && fiber.alternate) {
227-
// When we don't have children, we try the alternate first to see if it
228-
// has any current children first.
229-
fiber = fiber.alternate;
230-
}
231-
var results = findAllInRenderedFiberTreeInternal(fiber, test);
232-
if (results === null) {
233-
// Null is a sentinel that indicates that this was the wrong fiber.
234-
results = findAllInRenderedFiberTreeInternal(fiber.alternate, test);
235-
if (results === null) {
236-
throw new Error('This should never happen.');
237-
}
238-
}
239-
return results;
227+
return findAllInRenderedFiberTreeInternal(internalInstance, test);
240228
} else {
241229
return findAllInRenderedStackTreeInternal(internalInstance, test);
242230
}

0 commit comments

Comments
 (0)