Resume work deep in a bailed out child immediately in same pri
I found a way to test this case without any need for setState.
Sebastian Markbage committed
Jun 19, 2016 at 03:23 UTC
ea08c0020c242d4a811f3197f421e0fd035d2e0f
4 files changed
+194
-88
src/renderers/shared/fiber/ReactFiberBeginWork.js
+20
-8
@@ -31,6 +31,7 @@ var {
31
NoWork,
32
OffscreenPriority,
33
} = require('ReactPriorityLevel');
34
+var { findNextUnitOfWorkAtPriority } = require('ReactFiberPendingWork');
35
36
function reconcileChildren(current, workInProgress, nextChildren) {
37
const priority = workInProgress.pendingWorkPriority;
@@ -145,23 +146,34 @@ function beginWork(current : ?Fiber, workInProgress : Fiber) : ?Fiber {
146
// and children from that node.
147
workInProgress.memoizedProps = workInProgress.pendingProps;
148
workInProgress.output = current.output;
149
+ const priorityLevel = workInProgress.pendingWorkPriority;
150
+ workInProgress.pendingProps = null;
151
workInProgress.pendingWorkPriority = NoWork;
152
+ workInProgress.stateNode = current.stateNode;
153
if (current.child) {
154
+ // If we bail out but still has work with the current priority in this
155
+ // subtree, we need to go find it right now. If we don't, we won't flush
156
+ // it until the next tick.
157
workInProgress.child = current.child;
158
reuseChildren(workInProgress, workInProgress.child);
159
+ if (workInProgress.pendingWorkPriority <= priorityLevel) {
160
+ // TODO: This passes the current node and reads the priority level and
161
+ // pending props from that. We want it to read our priority level and
162
+ // pending props from the work in progress. Needs restructuring.
163
+ return findNextUnitOfWorkAtPriority(workInProgress.alternate, priorityLevel);
164
+ } else {
165
+ return null;
166
+ }
167
} else {
168
workInProgress.child = null;
169
+ return null;
170
}
155
- workInProgress.stateNode = current.stateNode;
156
- return null;
171
}
158
- if (workInProgress.pendingProps === workInProgress.memoizedProps) {
172
+ // if (workInProgress.pendingProps === workInProgress.memoizedProps) {
173
// In a ping-pong scenario, this version could actually contain the
160
- // old props. In that case, we can just bail out.
161
- // TODO: I don't yet quite understand what should happen on the
162
- // reused node here.
163
- return null;
164
- }
174
+ // old props. In that case, we should be able to just bail out but I
175
+ // don't yet quite understand what should happen on the reused node here.
176
+ // }
177
switch (workInProgress.tag) {
178
case IndeterminateComponent:
179
mountIndeterminateComponent(current, workInProgress);
src/renderers/shared/fiber/ReactFiberPendingWork.js
new
+98
@@ -0,0 +1,98 @@
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
+ * @providesModule ReactFiberPendingWork
10
+ * @flow
11
+ */
12
+
13
+'use strict';
14
+
15
+import type { Fiber } from 'ReactFiber';
16
+import type { PriorityLevel } from 'ReactPriorityLevel';
17
+
18
+var ReactFiber = require('ReactFiber');
19
+
20
+var {
21
+ NoWork,
22
+} = require('ReactPriorityLevel');
23
+
24
+function cloneSiblings(current : Fiber, workInProgress : Fiber, parent : Fiber) {
25
+ while (current.sibling) {
26
+ current = current.sibling;
27
+ workInProgress = workInProgress.sibling = ReactFiber.cloneFiber(
28
+ current,
29
+ current.pendingWorkPriority
30
+ );
31
+ workInProgress.parent = parent;
32
+ }
33
+ workInProgress.sibling = null;
34
+}
35
+
36
+exports.findNextUnitOfWorkAtPriority = function(currentRoot : Fiber, priorityLevel : PriorityLevel) : ?Fiber {
37
+ let current = currentRoot;
38
+ while (current) {
39
+ if (current.pendingWorkPriority !== NoWork &&
40
+ current.pendingWorkPriority <= priorityLevel) {
41
+ // This node has work to do that fits our priority level criteria.
42
+ if (current.pendingProps !== null) {
43
+ // We found some work to do. We need to return the "work in progress"
44
+ // of this node which will be the alternate.
45
+ const workInProgress = current.alternate;
46
+ if (!workInProgress) {
47
+ throw new Error('Should have wip now');
48
+ }
49
+ workInProgress.pendingProps = current.pendingProps;
50
+ return workInProgress;
51
+ }
52
+ // If we have a child let's see if any of our children has work to do.
53
+ // Only bother doing this at all if the current priority level matches
54
+ // because it is the highest priority for the whole subtree.
55
+ // TODO: Coroutines can have work in their stateNode which is another
56
+ // type of child that needs to be searched for work.
57
+ if (current.child) {
58
+ // Ensure we have a work in progress copy to backtrack through.
59
+ let currentChild = current.child;
60
+ let workInProgress = current.alternate;
61
+ if (!workInProgress) {
62
+ throw new Error('Should have wip now');
63
+ }
64
+ workInProgress.pendingWorkPriority = current.pendingWorkPriority;
65
+ workInProgress.child = ReactFiber.cloneFiber(currentChild, NoWork);
66
+ workInProgress.child.parent = workInProgress;
67
+ cloneSiblings(currentChild, workInProgress.child, workInProgress);
68
+ current = current.child;
69
+ continue;
70
+ }
71
+ // If we match the priority but has no child and no work to do,
72
+ // then we can safely reset the flag.
73
+ current.pendingWorkPriority = NoWork;
74
+ }
75
+ if (current === currentRoot) {
76
+ if (current.pendingWorkPriority <= priorityLevel) {
77
+ current.pendingWorkPriority = NoWork;
78
+ }
79
+ return null;
80
+ }
81
+ while (!current.sibling) {
82
+ // TODO: Stop using parent here. See below.
83
+ // $FlowFixMe: This downcast is not safe. It is intentionally an error.
84
+ current = current.parent;
85
+ if (!current) {
86
+ return null;
87
+ }
88
+ if (current.pendingWorkPriority <= priorityLevel) {
89
+ // If this subtree had work left to do, we would have returned it by
90
+ // now. This could happen if a child with pending work gets cleaned up
91
+ // but we don't clear the flag then. It is safe to reset it now.
92
+ current.pendingWorkPriority = NoWork;
93
+ }
94
+ }
95
+ current = current.sibling;
96
+ }
97
+ return null;
98
+}
src/renderers/shared/fiber/ReactFiberScheduler.js
+6
-80
@@ -13,13 +13,13 @@
13
'use strict';
14
15
import type { Fiber } from 'ReactFiber';
16
-import type { PriorityLevel } from 'ReactPriorityLevel';
16
import type { FiberRoot } from 'ReactFiberRoot';
17
import type { HostConfig } from 'ReactFiberReconciler';
18
20
-var ReactFiber = require('ReactFiber');
19
+var { cloneFiber } = require('ReactFiber');
20
var { beginWork } = require('ReactFiberBeginWork');
21
var { completeWork } = require('ReactFiberCompleteWork');
22
+var { findNextUnitOfWorkAtPriority } = require('ReactFiberPendingWork');
23
24
var {
25
NoWork,
@@ -42,77 +42,6 @@ module.exports = function<T, P, I>(config : HostConfig<T, P, I>) {
42
let nextScheduledRoot : ?FiberRoot = null;
43
let lastScheduledRoot : ?FiberRoot = null;
44
45
- function cloneSiblings(current : Fiber, workInProgress : Fiber, parent : Fiber) {
46
- while (current.sibling) {
47
- current = current.sibling;
48
- workInProgress = workInProgress.sibling = ReactFiber.cloneFiber(
49
- current,
50
- current.pendingWorkPriority
51
- );
52
- workInProgress.parent = parent;
53
- }
54
- workInProgress.sibling = null;
55
- }
56
-
57
- function findNextUnitOfWorkAtPriority(root : FiberRoot, priorityLevel : PriorityLevel) : ?Fiber {
58
- let current = root.current;
59
- ReactFiber.cloneFiber(current, current.pendingWorkPriority);
60
- while (current) {
61
- if (current.pendingWorkPriority !== 0 &&
62
- current.pendingWorkPriority <= priorityLevel) {
63
- // This node has work to do that fits our priority level criteria.
64
- if (current.pendingProps !== null) {
65
- // We found some work to do. We need to return the "work in progress"
66
- // of this node which will be the alternate.
67
- const workInProgress = current.alternate;
68
- if (!workInProgress) {
69
- throw new Error('Should have wip now');
70
- }
71
- workInProgress.pendingProps = current.pendingProps;
72
- return workInProgress;
73
- }
74
- // If we have a child let's see if any of our children has work to do.
75
- // Only bother doing this at all if the current priority level matches
76
- // because it is the highest priority for the whole subtree.
77
- // TODO: Coroutines can have work in their stateNode which is another
78
- // type of child that needs to be searched for work.
79
- if (current.child) {
80
- // Ensure we have a work in progress copy to backtrack through.
81
- let currentChild = current.child;
82
- let workInProgress = current.alternate;
83
- if (!workInProgress) {
84
- throw new Error('Should have wip now');
85
- }
86
- workInProgress.pendingWorkPriority = current.pendingWorkPriority;
87
- workInProgress.child = ReactFiber.cloneFiber(currentChild, NoWork);
88
- workInProgress.child.parent = workInProgress;
89
- cloneSiblings(currentChild, workInProgress.child, workInProgress);
90
- current = current.child;
91
- continue;
92
- }
93
- // If we match the priority but has no child and no work to do,
94
- // then we can safely reset the flag.
95
- current.pendingWorkPriority = NoWork;
96
- }
97
- while (!current.sibling) {
98
- // TODO: Stop using parent here. See below.
99
- // $FlowFixMe: This downcast is not safe. It is intentionally an error.
100
- current = current.parent;
101
- if (!current) {
102
- return null;
103
- }
104
- if (current.pendingWorkPriority <= priorityLevel) {
105
- // If this subtree had work left to do, we would have returned it by
106
- // now. This could happen if a child with pending work gets cleaned up
107
- // but we don't clear the flag then. It is safe to reset it now.
108
- current.pendingWorkPriority = NoWork;
109
- }
110
- }
111
- current = current.sibling;
112
- }
113
- return null;
114
- }
115
-
45
function findNextUnitOfWork() {
46
// Clear out roots with no more work on them.
47
while (nextScheduledRoot && nextScheduledRoot.current.pendingWorkPriority === NoWork) {
@@ -126,19 +55,20 @@ module.exports = function<T, P, I>(config : HostConfig<T, P, I>) {
55
}
56
let root = nextScheduledRoot;
57
while (root) {
58
+ cloneFiber(root.current, root.current.pendingWorkPriority);
59
// Find the highest possible priority work to do.
60
// This loop is unrolled just to satisfy Flow's enum constraint.
61
// We could make arbitrary many idle priority levels but having
62
// too many just means flushing changes too often.
133
- let work = findNextUnitOfWorkAtPriority(root, HighPriority);
63
+ let work = findNextUnitOfWorkAtPriority(root.current, HighPriority);
64
if (work) {
65
return work;
66
}
137
- work = findNextUnitOfWorkAtPriority(root, LowPriority);
67
+ work = findNextUnitOfWorkAtPriority(root.current, LowPriority);
68
if (work) {
69
return work;
70
}
141
- work = findNextUnitOfWorkAtPriority(root, OffscreenPriority);
71
+ work = findNextUnitOfWorkAtPriority(root.current, OffscreenPriority);
72
if (work) {
73
return work;
74
}
@@ -157,10 +87,6 @@ module.exports = function<T, P, I>(config : HostConfig<T, P, I>) {
87
const current = workInProgress.alternate;
88
const next = completeWork(current, workInProgress);
89
160
- // TODO: If workInProgress returns null but still has work with the
161
- // current priority in its subtree, we need to go find it right now.
162
- // If we don't, we won't flush it until the next tick.
163
-
90
// The work is now done. We don't need this anymore. This flags
91
// to the system not to redo any work here.
92
workInProgress.pendingProps = null;
src/renderers/shared/fiber/__tests__/ReactIncremental-test.js
+70
@@ -281,6 +281,76 @@ describe('ReactIncremental', function() {
281
// Flush the rest to make sure that the bailout didn't block this work.
282
ReactNoop.flush();
283
expect(ops).toEqual(['Middle']);
284
+ });
285
+
286
+ it('can resume work in a bailed subtree within one pass', function() {
287
+
288
+ var ops = [];
289
+
290
+ function Bar(props) {
291
+ ops.push('Bar');
292
+ return <div>{props.children}</div>;
293
+ }
294
+
295
+ function Tester() {
296
+ // This component is just here to ensure that the bail out is
297
+ // in fact in effect in the expected place for this test.
298
+ ops.push('Tester');
299
+ return <div />;
300
+ }
301
+
302
+ function Middle(props) {
303
+ ops.push('Middle');
304
+ return <span>{props.children}</span>;
305
+ }
306
+
307
+ var middleContent = (
308
+ <aaa>
309
+ <Tester />
310
+ <bbb hidden={true}>
311
+ <ccc>
312
+ <Middle>Hi</Middle>
313
+ </ccc>
314
+ </bbb>
315
+ </aaa>
316
+ );
317
+
318
+ function Foo(props) {
319
+ ops.push('Foo');
320
+ return (
321
+ <div hidden={props.text === 'bar'}>
322
+ <Bar>{props.text}</Bar>
323
+ {middleContent}
324
+ <Bar>{props.text}</Bar>
325
+ </div>
326
+ );
327
+ }
328
+
329
+ // Init
330
+ ReactNoop.render(<Foo text="foo" />);
331
+ ReactNoop.flushLowPri(52);
332
+
333
+ expect(ops).toEqual(['Foo', 'Bar', 'Tester', 'Bar']);
334
+
335
+ ops = [];
336
+
337
+ // Make a quick update which will create a low pri tree on top of the
338
+ // already low pri tree.
339
+ ReactNoop.render(<Foo text="bar" />);
340
+ ReactNoop.flushLowPri(15);
341
+
342
+ expect(ops).toEqual(['Foo']);
343
+
344
+ ops = [];
345
+
346
+ // At this point, middle will bail out but it has not yet fully rendered.
347
+ // Since that is the same priority as its parent tree. This should render
348
+ // as a single batch. Therefore, it is correct that Middle should be in the
349
+ // middle. If it occurs after the two "Bar" components then it was flushed
350
+ // after them which is not correct.
351
+ ReactNoop.flush();
352
+ expect(ops).toEqual(['Bar', 'Middle', 'Bar']);
353
354
});
355
+
356
});