Fix reordering of bailed out children (#8457)
Ben Alpert committed
Nov 29, 2016 at 14:57 UTC
6110c585840e11d61a87d43c4f882a6ce78884bd
4 files changed
+42
-5
scripts/fiber/tests-passing.txt
+1
@@ -1345,6 +1345,7 @@ src/renderers/shared/shared/__tests__/ReactMultiChild-test.js
1345
* should replace children with different constructors
1346
* should NOT replace children with different owners
1347
* should replace children with different keys
1348
+* should reorder bailed-out children
1349
1350
src/renderers/shared/shared/__tests__/ReactMultiChildReconcile-test.js
1351
* should reset internal state if removed then readded in an array
src/renderers/shared/fiber/ReactFiberCommitWork.js
+4
-2
@@ -132,7 +132,7 @@ module.exports = function<T, P, I, TI, C>(
132
}
133
}
134
135
- function commitInsertion(finishedWork : Fiber) : void {
135
+ function commitPlacement(finishedWork : Fiber) : void {
136
// Recursively insert all host nodes into the parent.
137
const parent = getHostParent(finishedWork);
138
const before = getHostSibling(finishedWork);
@@ -152,6 +152,7 @@ module.exports = function<T, P, I, TI, C>(
152
// the portal directly.
153
} else if (node.child) {
154
// TODO: Coroutines need to visit the stateNode.
155
+ node.child.return = node;
156
node = node.child;
157
continue;
158
}
@@ -164,6 +165,7 @@ module.exports = function<T, P, I, TI, C>(
165
}
166
node = node.return;
167
}
168
+ node.sibling.return = node.return;
169
node = node.sibling;
170
}
171
}
@@ -423,7 +425,7 @@ module.exports = function<T, P, I, TI, C>(
425
}
426
427
return {
426
- commitInsertion,
428
+ commitPlacement,
429
commitDeletion,
430
commitWork,
431
commitLifeCycles,
src/renderers/shared/fiber/ReactFiberScheduler.js
+3
-3
@@ -67,7 +67,7 @@ type TrappedError = {
67
module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
68
const { beginWork } = ReactFiberBeginWork(config, scheduleUpdate);
69
const { completeWork } = ReactFiberCompleteWork(config);
70
- const { commitInsertion, commitDeletion, commitWork, commitLifeCycles } =
70
+ const { commitPlacement, commitDeletion, commitWork, commitLifeCycles } =
71
ReactFiberCommitWork(config, trapError);
72
73
const hostScheduleAnimationCallback = config.scheduleAnimationCallback;
@@ -174,7 +174,7 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
174
switch (effectfulFiber.effectTag) {
175
case Placement:
176
case PlacementAndCallback: {
177
- commitInsertion(effectfulFiber);
177
+ commitPlacement(effectfulFiber);
178
// Clear the "placement" from effect tag so that we know that this is inserted, before
179
// any life-cycles like componentDidMount gets called.
180
effectfulFiber.effectTag ^= Placement;
@@ -183,7 +183,7 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
183
case PlacementAndUpdate:
184
case PlacementAndUpdateAndCallback: {
185
// Placement
186
- commitInsertion(effectfulFiber);
186
+ commitPlacement(effectfulFiber);
187
// Clear the "placement" from effect tag so that we know that this is inserted, before
188
// any life-cycles like componentDidMount gets called.
189
effectfulFiber.effectTag ^= Placement;
src/renderers/shared/shared/__tests__/ReactMultiChild-test.js
+34
@@ -220,4 +220,38 @@ describe('ReactMultiChild', () => {
220
);
221
});
222
});
223
+
224
+ it('should reorder bailed-out children', () => {
225
+ spyOn(console, 'error');
226
+
227
+ class LetterInner extends React.Component {
228
+ render() {
229
+ return <div>{this.props.char}</div>;
230
+ }
231
+ }
232
+
233
+ class Letter extends React.Component {
234
+ render() {
235
+ return <LetterInner char={this.props.char} />;
236
+ }
237
+ shouldComponentUpdate() {
238
+ return false;
239
+ }
240
+ }
241
+
242
+ class Letters extends React.Component {
243
+ render() {
244
+ const letters = this.props.letters.split('');
245
+ return <div>{letters.map((c) => <Letter key={c} char={c} />)}</div>;
246
+ }
247
+ }
248
+
249
+ var container = document.createElement('div');
250
+
251
+ // Two random strings -- some additions, some removals, some moves
252
+ ReactDOM.render(<Letters letters="XKwHomsNjIkBcQWFbiZU" />, container);
253
+ expect(container.textContent).toBe('XKwHomsNjIkBcQWFbiZU');
254
+ ReactDOM.render(<Letters letters="EHCjpdTUuiybDvhRJwZt" />, container);
255
+ expect(container.textContent).toBe('EHCjpdTUuiybDvhRJwZt');
256
+ });
257
});