Don't swap the trees until after the first pass of the commit phase, where componentWillUnmount is called, but before the second pass, where componentDidMount/Update is called.
Don't swap the trees until after the first pass of the commit phase, where componentWillUnmount is called, but before the second pass, where componentDidMount/Update is called.
Andrew Clark committed
Jan 31, 2017 at 14:39 UTC
af7ae1be91563014bcd8bf7c9c77af04c8e8c1c9
2 files changed
+26
-3
src/renderers/shared/fiber/ReactFiberScheduler.js
+6
-1
@@ -346,7 +346,6 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(config : HostConfig<T, P,
346
'related to the return field.'
347
);
348
}
349
- root.current = finishedWork;
349
350
// Updates that occur during the commit phase should have Task priority
351
const previousPriorityContext = priorityContext;
@@ -392,6 +391,12 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(config : HostConfig<T, P,
391
392
resetAfterCommit(commitInfo);
393
394
+ // The work-in-progress tree is now the current tree. This must come after
395
+ // the first pass of the commit phase, so that the previous tree is still
396
+ // current during componentWillUnmount, but before the second pass, so that
397
+ // the finished work is current during componentDidMount/Update.
398
+ root.current = finishedWork;
399
+
400
// In the second pass we'll perform all life-cycles and ref callbacks.
401
// Life-cycles happen as a separate pass so that all placements, updates,
402
// and deletions in the entire tree have already been invoked.
src/renderers/shared/fiber/__tests__/ReactIncrementalReflection-test.js
+20
-2
@@ -148,12 +148,19 @@ describe('ReactIncrementalReflection', () => {
148
componentDidUpdate() {
149
ops.push('componentDidUpdate', ReactNoop.findInstance(this));
150
}
151
+ componentWillUnmount() {
152
+ ops.push('componentWillUnmount', ReactNoop.findInstance(this));
153
+ }
154
render() {
155
ops.push('render');
156
return this.props.step < 2 ?
157
<span ref={ref => this.span = ref} /> :
158
this.props.step === 2 ?
159
<div ref={ref => this.div = ref} /> :
160
+ this.props.step === 3 ?
161
+ null :
162
+ this.props.step === 4 ?
163
+ <div ref={ref => this.span = ref} /> :
164
null;
165
}
166
}
@@ -250,7 +257,7 @@ describe('ReactIncrementalReflection', () => {
257
// We should now find the new host node.
258
expect(ReactNoop.findInstance(classInstance)).toBe(hostDiv);
259
253
- // Finally we will render to null but not yet commit it.
260
+ // Render to null but don't commit it yet.
261
ReactNoop.render(<Foo step={3} />);
262
ReactNoop.flushDeferredPri(25);
263
@@ -273,7 +280,18 @@ describe('ReactIncrementalReflection', () => {
280
281
// This should still be the host div since the deletion is not committed.
282
expect(ReactNoop.findInstance(classInstance)).toBe(null);
276
- });
283
284
+ // Render a div again
285
+ ReactNoop.render(<Foo step={4} />);
286
+ ReactNoop.flush();
287
+
288
+ ops = [];
289
290
+ // Unmount the component.
291
+ ReactNoop.render([]);
292
+ ReactNoop.flush();
293
+ expect(ops).toEqual([
294
+ 'componentWillUnmount', hostDiv,
295
+ ]);
296
+ });
297
});