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.
Sebastian Markbage committed
Dec 1, 2016 at 21:21 UTC
a434f9e7afc2200787ab8cfa96e7641d98d3ce69
5 files changed
+126
-85
scripts/fiber/tests-failing.txt
-9
@@ -27,9 +27,6 @@ src/renderers/art/__tests__/ReactART-test.js
27
src/renderers/dom/__tests__/ReactDOMProduction-test.js
28
* should throw with an error code in production
29
30
-src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
31
-* findDOMNode should find dom element after expanding a fragment
32
-
30
src/renderers/dom/shared/__tests__/ReactDOM-test.js
31
* throws in render() if the mount callback is not a function
32
* throws in render() if the update callback is not a function
@@ -66,9 +63,6 @@ src/renderers/dom/shared/__tests__/ReactRenderDocument-test.js
63
* should throw on full document render w/ no markup
64
* supports findDOMNode on full-page components
65
69
-src/renderers/dom/shared/__tests__/findDOMNode-test.js
70
-* findDOMNode should find dom element after an update from null
71
-
66
src/renderers/shared/__tests__/ReactPerf-test.js
67
* should count no-op update as waste
68
* should count no-op update in child as waste
@@ -125,6 +119,3 @@ src/renderers/shared/shared/__tests__/ReactUpdates-test.js
119
120
src/renderers/shared/shared/__tests__/refs-test.js
121
* Should increase refs with an increase in divs
128
-
129
-src/test/__tests__/ReactTestUtils-test.js
130
-* traverses children in the correct order
scripts/fiber/tests-passing.txt
+3
@@ -507,6 +507,7 @@ src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
507
* should pass portal context when rendering subtree elsewhere
508
* should update portal context if it changes due to setState
509
* should update portal context if it changes due to re-render
510
+* findDOMNode should find dom element after expanding a fragment
511
512
src/renderers/dom/shared/__tests__/CSSProperty-test.js
513
* should generate browser prefixes for its `isUnitlessNumber`
@@ -713,6 +714,7 @@ src/renderers/dom/shared/__tests__/escapeTextContentForBrowser-test.js
714
src/renderers/dom/shared/__tests__/findDOMNode-test.js
715
* findDOMNode should return null if passed null
716
* findDOMNode should find dom element
717
+* findDOMNode should find dom element after an update from null
718
* findDOMNode should reject random objects
719
* findDOMNode should reject unmounted objects with render func
720
* findDOMNode should not throw an error when called within a component that is not mounted
@@ -1589,6 +1591,7 @@ src/test/__tests__/ReactTestUtils-test.js
1591
* can scryRenderedDOMComponentsWithClass with TextComponent
1592
* can scryRenderedDOMComponentsWithClass with className contains \n
1593
* can scryRenderedDOMComponentsWithClass with multiple classes
1594
+* traverses children in the correct order
1595
* should support injected wrapper components as DOM components
1596
* should change the value of an input field
1597
* should change the value of an input field in a component
src/renderers/shared/fiber/ReactFiberScheduler.js
+3
@@ -183,6 +183,9 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
183
commitPlacement(effectfulFiber);
184
// Clear the "placement" from effect tag so that we know that this is inserted, before
185
// any life-cycles like componentDidMount gets called.
186
+ // TODO: findDOMNode doesn't rely on this any more but isMounted
187
+ // does and isMounted is deprecated anyway so we should be able
188
+ // to kill this.
189
effectfulFiber.effectTag &= ~Placement;
190
break;
191
}
src/renderers/shared/fiber/ReactFiberTreeReflection.js
+87
-31
@@ -73,60 +73,116 @@ exports.isMounted = function(component : ReactComponent<any, any, any>) : boolea
73
return isFiberMountedImpl(fiber) === MOUNTED;
74
};
75
76
-exports.findCurrentHostFiber = function(parent : Fiber) : Fiber | null {
77
- // First check if this node itself is mounted.
78
- const state = isFiberMountedImpl(parent, true);
79
- if (state === UNMOUNTED) {
76
+function assertIsMounted(fiber) {
77
+ invariant(
78
+ isFiberMountedImpl(fiber) === MOUNTED,
79
+ 'Unable to find node on an unmounted component.'
80
+ );
81
+}
82
+
83
+function findCurrentFiberUsingSlowPath(fiber : Fiber) : Fiber | null {
84
+ let alternate = fiber.alternate;
85
+ if (!alternate) {
86
+ // If there is no alternate, then we only need to check if it is mounted.
87
+ const state = isFiberMountedImpl(fiber);
88
invariant(
81
- false,
89
+ state !== UNMOUNTED,
90
'Unable to find node on an unmounted component.'
91
);
84
- } else if (state === MOUNTING) {
85
- return null;
92
+ if (state === MOUNTING) {
93
+ return null;
94
+ }
95
+ return fiber;
96
}
97
+ // If we have two possible branches, we'll walk backwards up to the root
98
+ // to see what path the root points to. On the way we may hit one of the
99
+ // special cases and we'll deal with them.
100
+ let a = fiber;
101
+ let b = alternate;
102
+ while (true) {
103
+ let parentA = a.return;
104
+ let parentB = b.return;
105
+ if (!parentA || !parentB) {
106
+ // We're at the root.
107
+ break;
108
+ }
109
+ if (parentA.child === parentB.child) {
110
+ // If both parents are the same, then that is the current parent. If
111
+ // they're different but point to the same child, then it doesn't matter.
112
+ // Regardless, whatever child they point to is the current child.
113
+ // So we can now determine which child is current by scanning the child
114
+ // list for either A or B.
115
+ let child = parentA.child;
116
+ while (child) {
117
+ if (child === a) {
118
+ // We've determined that A is the current branch.
119
+ assertIsMounted(parentA);
120
+ return fiber;
121
+ }
122
+ if (child === b) {
123
+ // We've determined that B is the current branch.
124
+ assertIsMounted(parentA);
125
+ return alternate;
126
+ }
127
+ child = child.sibling;
128
+ }
129
+ // We should never have an alternate for any mounting node. So the only
130
+ // way this could possibly happen is if this was unmounted, if at all.
131
+ invariant(
132
+ false,
133
+ 'Unable to find node on an unmounted component.'
134
+ );
135
+ }
136
+ a = parentA;
137
+ b = parentB;
138
+ invariant(
139
+ a.alternate === b,
140
+ 'Return fibers should always be each others\' alternates.'
141
+ );
142
+ }
143
+ // If the root is not a host container, we're in a disconnected tree. I.e.
144
+ // unmounted.
145
+ invariant(
146
+ a.tag === HostRoot,
147
+ 'Unable to find node on an unmounted component.'
148
+ );
149
+ if (a.stateNode.current === a) {
150
+ // We've determined that A is the current branch.
151
+ return fiber;
152
+ }
153
+ // Otherwise B has to be current branch.
154
+ return alternate;
155
+}
156
+exports.findCurrentFiberUsingSlowPath = findCurrentFiberUsingSlowPath;
157
88
- let didTryOtherTree = false;
89
-
90
- // If the component doesn't have a child we first check the alternate to see
91
- // if it has any and if so, if those were just recently inserted.
92
- if (!parent.child && parent.alternate) {
93
- parent = parent.alternate;
158
+exports.findCurrentHostFiber = function(parent : Fiber) : Fiber | null {
159
+ const currentParent = findCurrentFiberUsingSlowPath(parent);
160
+ if (!currentParent) {
161
+ return null;
162
}
163
164
// Next we'll drill down this component to find the first HostComponent/Text.
97
- let node : Fiber = parent;
165
+ let node : Fiber = currentParent;
166
while (true) {
99
- if ((node.effectTag & Placement) !== NoEffect || !node.return) {
100
- // If any node along the way was deleted, or is an insertion, that means
101
- // that we're actually in a work in progress to update this component with
102
- // a different component. We need to look in the "current" fiber instead.
103
- if (!parent.alternate) {
104
- return null;
105
- }
106
- if (didTryOtherTree) {
107
- // Safety, to avoid an infinite loop if something goes wrong.
108
- throw new Error('This should never hit this infinite loop.');
109
- }
110
- didTryOtherTree = true;
111
- node = parent = parent.alternate;
112
- continue;
113
- }
167
if (node.tag === HostComponent || node.tag === HostText) {
168
return node;
169
} else if (node.child) {
170
+ // TODO: If we hit a Portal, we're supposed to skip it.
171
// TODO: Coroutines need to visit the stateNode.
172
+ node.child.return = node;
173
node = node.child;
174
continue;
175
}
121
- if (node === parent) {
176
+ if (node === currentParent) {
177
return null;
178
}
179
while (!node.sibling) {
125
- if (!node.return || node.return === parent) {
180
+ if (!node.return || node.return === currentParent) {
181
return null;
182
}
183
node = node.return;
184
}
185
+ node.sibling.return = node.return;
186
node = node.sibling;
187
}
188
// Flow needs the return null here, but ESLint complains about it.
src/test/ReactTestUtils.js
+33
-45
@@ -20,9 +20,9 @@ var ReactControlledComponent = require('ReactControlledComponent');
20
var ReactDOM = require('ReactDOM');
21
var ReactDOMComponentTree = require('ReactDOMComponentTree');
22
var ReactBrowserEventEmitter = require('ReactBrowserEventEmitter');
23
+var ReactFiberTreeReflection = require('ReactFiberTreeReflection');
24
var ReactInstanceMap = require('ReactInstanceMap');
25
var ReactTypeOfWork = require('ReactTypeOfWork');
25
-var ReactTypeOfSideEffect = require('ReactTypeOfSideEffect');
26
var ReactGenericBatching = require('ReactGenericBatching');
27
var SyntheticEvent = require('SyntheticEvent');
28
var ReactShallowRenderer = require('ReactShallowRenderer');
@@ -37,10 +37,6 @@ var {
37
HostComponent,
38
HostText,
39
} = ReactTypeOfWork;
40
-var {
41
- NoEffect,
42
- Placement,
43
-} = ReactTypeOfSideEffect;
40
41
function Event(suffix) {}
42
@@ -84,34 +80,40 @@ function findAllInRenderedFiberTreeInternal(fiber, test) {
80
if (!fiber) {
81
return [];
82
}
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) {
87
return [];
88
}
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;
116
}
113
- // TODO: visit stateNode for coroutines
114
- return ret;
117
}
118
119
/**
@@ -222,21 +224,7 @@ var ReactTestUtils = {
224
);
225
var internalInstance = ReactInstanceMap.get(inst);
226
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);
228
} else {
229
return findAllInRenderedStackTreeInternal(internalInstance, test);
230
}