Call componentWillUnmount during deletion phase
While we're deleting nodes, we need to call the unmount life-cycle. However, there is a special case that we don't want to keep deleting every host node along the way since deleting the top node is enough.
Sebastian Markbage committed
Oct 7, 2016 at 13:36 UTC
90993c5324ade21afaf6175758eab21ba8d18f99
2 files changed
+150
-4
src/renderers/shared/fiber/ReactFiberCommitWork.js
+49
-4
@@ -132,8 +132,36 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
132
}
133
}
134
135
+ function commitNestedUnmounts(root : Fiber) {
136
+ // While we're inside a removed host node we don't want to call
137
+ // removeChild on the inner nodes because they're removed by the top
138
+ // call anyway. We also want to call componentWillUnmount on all
139
+ // composites before this host node is removed from the tree. Therefore
140
+ // we do an inner loop while we're still inside the host node.
141
+ let node : Fiber = root;
142
+ while (true) {
143
+ commitUnmount(node);
144
+ if (node.child) {
145
+ // TODO: Coroutines need to visit the stateNode.
146
+ node = node.child;
147
+ continue;
148
+ }
149
+ if (node === root) {
150
+ return;
151
+ }
152
+ while (!node.sibling) {
153
+ if (!node.return || node.return === root) {
154
+ return;
155
+ }
156
+ node = node.return;
157
+ }
158
+ node = node.sibling;
159
+ }
160
+ }
161
+
162
function commitDeletion(current : Fiber) : void {
163
// Recursively delete all host nodes from the parent.
164
+ // TODO: Error handling.
165
const parent = getHostParent(current);
166
if (!parent) {
167
return;
@@ -146,11 +174,17 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
174
let node : Fiber = current;
175
while (true) {
176
if (node.tag === HostComponent || node.tag === HostText) {
177
+ commitNestedUnmounts(node);
178
+ // After all the children have unmounted, it is now safe to remove the
179
+ // node from the tree.
180
removeChild(parent, node.stateNode);
150
- } else if (node.child) {
151
- // TODO: Coroutines need to visit the stateNode.
152
- node = node.child;
153
- continue;
181
+ } else {
182
+ commitUnmount(node);
183
+ if (node.child) {
184
+ // TODO: Coroutines need to visit the stateNode.
185
+ node = node.child;
186
+ continue;
187
+ }
188
}
189
if (node === current) {
190
return;
@@ -165,6 +199,17 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
199
}
200
}
201
202
+ function commitUnmount(current : Fiber) : void {
203
+ switch (current.tag) {
204
+ case ClassComponent: {
205
+ const instance = current.stateNode;
206
+ if (typeof instance.componentWillUnmount === 'function') {
207
+ instance.componentWillUnmount();
208
+ }
209
+ }
210
+ }
211
+ }
212
+
213
function commitWork(current : ?Fiber, finishedWork : Fiber) : void {
214
switch (finishedWork.tag) {
215
case ClassComponent: {
src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js
+101
@@ -132,6 +132,38 @@ describe('ReactIncrementalSideEffects', () => {
132
133
});
134
135
+ it('can deletes children either components, host or text', function() {
136
+
137
+ function Bar(props) {
138
+ return <span prop={props.children} />;
139
+ }
140
+
141
+ function Foo(props) {
142
+ return (
143
+ <div>
144
+ {props.show ? [
145
+ <div key="a" />,
146
+ <Bar key="b">Hello</Bar>,
147
+ 'World',
148
+ ] : []}
149
+ </div>
150
+ );
151
+ }
152
+
153
+ ReactNoop.render(<Foo show={true} />);
154
+ ReactNoop.flush();
155
+ expect(ReactNoop.root.children).toEqual([
156
+ div(div(), span('Hello'), 'World'),
157
+ ]);
158
+
159
+ ReactNoop.render(<Foo show={false} />);
160
+ ReactNoop.flush();
161
+ expect(ReactNoop.root.children).toEqual([
162
+ div(),
163
+ ]);
164
+
165
+ });
166
+
167
it('does not update child nodes if a flush is aborted', () => {
168
169
function Bar(props) {
@@ -459,4 +491,73 @@ describe('ReactIncrementalSideEffects', () => {
491
});
492
493
// TODO: Test that callbacks are not lost if an update is preempted.
494
+
495
+ it('calls componentWillUnmount after a deletion, even if nested', () => {
496
+
497
+ var ops = [];
498
+
499
+ class Bar extends React.Component {
500
+ componentWillUnmount() {
501
+ ops.push(this.props.name);
502
+ }
503
+ render() {
504
+ return <span />;
505
+ }
506
+ }
507
+
508
+ class Wrapper extends React.Component {
509
+ componentWillUnmount() {
510
+ ops.push('Wrapper');
511
+ }
512
+ render() {
513
+ return <Bar name={this.props.name} />;
514
+ }
515
+ }
516
+
517
+ function Foo(props) {
518
+ return (
519
+ <div>
520
+ {props.show ? [
521
+ <Bar key="a" name="A" />,
522
+ <Wrapper key="b" name="B" />,
523
+ <div key="cd">
524
+ <Bar name="C" />
525
+ <Wrapper name="D" />,
526
+ </div>,
527
+ [
528
+ <Bar key="e" name="E" />,
529
+ <Bar key="f" name="F" />,
530
+ ],
531
+ ] : []}
532
+ <div>
533
+ {props.show ? <Bar key="g" name="G" /> : null}
534
+ </div>
535
+ <Bar name="this should not unmount" />
536
+ </div>
537
+ );
538
+ }
539
+
540
+ ReactNoop.render(<Foo show={true} />);
541
+ ReactNoop.flush();
542
+ expect(ops).toEqual([]);
543
+
544
+ ReactNoop.render(<Foo show={false} />);
545
+ ReactNoop.flush();
546
+ expect(ops).toEqual([
547
+ 'A',
548
+ 'Wrapper',
549
+ 'B',
550
+ 'C',
551
+ 'Wrapper',
552
+ 'D',
553
+ 'E',
554
+ 'F',
555
+ 'G',
556
+ ]);
557
+
558
+ });
559
+
560
+ // TODO: Test that unmounts and deletions happen in the expected way for
561
+ // aborted and resumed render life-cycles.
562
+
563
});