@samitouri / QOS-React / commits / 1914f94041

Handle unmounted and not-yet-inserted subtrees in isMounted

There are two cases where we have a Fiber that is not actually mounted. Either it is part of a tree that has not yet been inserted or it is part of a tree that was unmounted. For the insertion case, we can check the parents to see if there is any insertion effect pending along the parent path. For deletions, we can now check if any of the return pointers is null without actually being the root.

Sebastian Markbage committed Oct 24, 2016 at 13:46 UTC 1914f94041b24b23f4a9cde536b804ee062735cd
4 files changed +176 -16
src/renderers/shared/fiber/ReactFiberCommitWork.js
+13 -11
@@ -182,18 +182,11 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
182 }
183 }
184
185 - function commitDeletion(current : Fiber) : void {
186 - // Recursively delete all host nodes from the parent.
187 - // TODO: Error handling.
188 - const parent = getHostParent(current);
189 -
185 + function unmountHostComponents(parent, current) {
186 // We only have the top Fiber that was inserted but we need recurse down its
187 // children to find all the terminal nodes.
192 - // TODO: Call componentWillUnmount on all classes as needed. Recurse down
193 - // removed HostComponents but don't call removeChild on already removed
194 - // children.
188 let node : Fiber = current;
196 - outer: while (true) {
189 + while (true) {
190 if (node.tag === HostComponent || node.tag === HostText) {
191 commitNestedUnmounts(node);
192 // After all the children have unmounted, it is now safe to remove the
@@ -210,16 +203,25 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
203 }
204 }
205 if (node === current) {
213 - break outer;
206 + return;
207 }
208 while (!node.sibling) {
209 if (!node.return || node.return === current) {
217 - break outer;
210 + return;
211 }
212 node = node.return;
213 }
214 node = node.sibling;
215 }
216 + }
217 +
218 + function commitDeletion(current : Fiber) : void {
219 + // Recursively delete all host nodes from the parent.
220 + // TODO: Error handling.
221 + const parent = getHostParent(current);
222 +
223 + unmountHostComponents(parent, current);
224 +
225 // Cut off the return pointers to disconnect it from the tree. Ideally, we
226 // should clear the child pointer of the parent alternate to let this
227 // get GC:ed but we don't know which for sure which parent is the current
src/renderers/shared/fiber/ReactFiberScheduler.js
+6 -1
@@ -119,12 +119,18 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
119 switch (effectfulFiber.effectTag) {
120 case Placement: {
121 commitInsertion(effectfulFiber);
122 + // Clear the effect tag so that we know that this is inserted, before
123 + // any life-cycles like componentDidMount gets called.
124 + effectfulFiber.effectTag = NoWork;
125 break;
126 }
127 case PlacementAndUpdate: {
128 commitInsertion(effectfulFiber);
129 const current = effectfulFiber.alternate;
130 commitWork(current, effectfulFiber);
131 + // Clear the effect tag so that we know that this is inserted, before
132 + // any life-cycles like componentDidMount gets called.
133 + effectfulFiber.effectTag = Update;
134 break;
135 }
136 case Update: {
@@ -158,7 +164,6 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
164 effectfulFiber.nextEffect = null;
165 // Ensure that we reset the effectTag here so that we can rely on effect
166 // tags to reason about the current life-cycle.
161 - effectfulFiber.effectTag = NoWork;
167 effectfulFiber = next;
168 }
169
src/renderers/shared/fiber/ReactFiberTreeReflection.js
+30 -4
@@ -23,15 +23,41 @@ var {
23 HostText,
24 } = require('ReactTypeOfWork');
25
26 +var {
27 + NoEffect,
28 + Placement,
29 +} = require('ReactTypeOfSideEffect');
30 +
31 exports.isMounted = function(component : ReactComponent<any, any, any>) : boolean {
32 var parent : ?Fiber = ReactInstanceMap.get(component);
33 if (!parent) {
34 return false;
35 }
31 - // TODO: This doesn't deal with the case where it has completed but not yet
32 - // committed. It also doesn't deal with unmounts since they currently don't
33 - // clean up the item in the ReactInstanceMap.
34 - return true;
36 + let node = parent;
37 + if (!parent.alternate) {
38 + // If there is no alternate, this might be a new tree that isn't inserted
39 + // yet. If it is, then it will have a pending insertion effect on it.
40 + if ((node.effectTag & Placement) !== NoEffect) {
41 + return false;
42 + }
43 + while (node.return) {
44 + node = node.return;
45 + if ((node.effectTag & Placement) !== NoEffect) {
46 + return false;
47 + }
48 + }
49 + } else {
50 + while (node.return) {
51 + node = node.return;
52 + }
53 + }
54 + if (node.tag === HostContainer) {
55 + // TODO: Check if this was a nested HostContainer when used with
56 + // renderContainerIntoSubtree.
57 + return true;
58 + }
59 + // If we didn't hit the root, that means that we're in an disconnected tree.
60 + return false;
61 };
62
63 exports.findCurrentHostFiber = function(component : ReactComponent<any, any, any>) : Fiber | null {
src/renderers/shared/fiber/__tests__/ReactIncrementalReflection-test.js new
+127
@@ -0,0 +1,127 @@
1 +/**
2 + * Copyright 2013-present, Facebook, Inc.
3 + * All rights reserved.
4 + *
5 + * This source code is licensed under the BSD-style license found in the
6 + * LICENSE file in the root directory of this source tree. An additional grant
7 + * of patent rights can be found in the PATENTS file in the same directory.
8 + *
9 + * @emails react-core
10 + */
11 +
12 +'use strict';
13 +
14 +var React;
15 +var ReactNoop;
16 +
17 +describe('ReactIncrementalReflection', () => {
18 + beforeEach(() => {
19 + React = require('React');
20 + ReactNoop = require('ReactNoop');
21 + });
22 +
23 + it('handles isMounted even when the initial render is deferred', () => {
24 +
25 + let ops = [];
26 +
27 + const instances = [];
28 +
29 + const Component = React.createClass({
30 + componentWillMount() {
31 + instances.push(this);
32 + ops.push('componentWillMount', this.isMounted());
33 + },
34 + componentDidMount() {
35 + ops.push('componentDidMount', this.isMounted());
36 + },
37 + render() {
38 + return <span />;
39 + },
40 + });
41 +
42 + function Foo() {
43 + return <Component />;
44 + }
45 +
46 + ReactNoop.render(<Foo />);
47 +
48 + // Render part way through but don't yet commit the updates.
49 + ReactNoop.flushDeferredPri(20);
50 +
51 + expect(ops).toEqual([
52 + 'componentWillMount', false,
53 + ]);
54 +
55 + expect(instances[0].isMounted()).toBe(false);
56 +
57 + ops = [];
58 +
59 + // Render the rest and commit the updates.
60 + ReactNoop.flush();
61 +
62 + expect(ops).toEqual([
63 + 'componentDidMount', true,
64 + ]);
65 +
66 + expect(instances[0].isMounted()).toBe(true);
67 +
68 + });
69 +
70 + it('handles isMounted when an unmount is deferred', () => {
71 +
72 + let ops = [];
73 +
74 + const instances = [];
75 +
76 + const Component = React.createClass({
77 + componentWillMount() {
78 + instances.push(this);
79 + },
80 + componentWillUnmount() {
81 + ops.push('componentWillUnmount', this.isMounted());
82 + },
83 + render() {
84 + ops.push('Component');
85 + return <span />;
86 + },
87 + });
88 +
89 + function Other() {
90 + ops.push('Other');
91 + return <span />;
92 + }
93 +
94 + function Foo(props) {
95 + return props.mount ? <Component /> : <Other />;
96 + }
97 +
98 + ReactNoop.render(<Foo mount={true} />);
99 + ReactNoop.flush();
100 +
101 + expect(ops).toEqual(['Component']);
102 + ops = [];
103 +
104 + expect(instances[0].isMounted()).toBe(true);
105 +
106 + ReactNoop.render(<Foo mount={false} />);
107 + // Render part way through but don't yet commit the updates so it is not
108 + // fully unmounted yet.
109 + ReactNoop.flushDeferredPri(20);
110 +
111 + expect(ops).toEqual(['Other']);
112 + ops = [];
113 +
114 + expect(instances[0].isMounted()).toBe(true);
115 +
116 + // Finish flushing the unmount.
117 + ReactNoop.flush();
118 +
119 + expect(ops).toEqual([
120 + 'componentWillUnmount', true,
121 + ]);
122 +
123 + expect(instances[0].isMounted()).toBe(false);
124 +
125 + });
126 +
127 +});