Append deletions to the effect list
First clear any progressed deletions for any case where we start over with the "current" set of children. Once we've performed a new reconciliation we need to add the deletions to the side-effect list (which we know is empty because we just emptied it). For other effects, instead of just adding a fiber to an effect list we need to mark it with an update. Then after completion we add it to the the effect list if it had any effects at all. This means that we lose the opportunity to control if a fiber gets added before or after its children but that was already flawed since we want certain side-effects to happen before others on a global level. Instead, we'll do multiple passes through the effect list.
Sebastian Markbage committed
Oct 4, 2016 at 15:34 UTC
442ab71fc75218cb86bbd8240afaafc74fffe03a
4 files changed
+69
-39
src/renderers/shared/fiber/ReactChildFiber.js
-5
@@ -58,7 +58,6 @@ const {
58
} = ReactPriorityLevel;
59
60
const {
61
- NoEffect,
61
Placement,
62
Deletion,
63
} = ReactTypeOfSideEffect;
@@ -582,10 +581,6 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
581
existingChildren.forEach(child => deleteChild(returnFiber, child));
582
}
583
585
- // TODO: Add deletions and insert/moves to the side-effect list.
586
- // TODO: Clear the deletion list when we don't reconcile in place. When
587
- // progressedChild isn't reused.
588
-
584
return resultingFirstChild;
585
}
586
src/renderers/shared/fiber/ReactFiberBeginWork.js
+30
-4
@@ -66,6 +66,18 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>, g
66
}
67
}
68
69
+ function clearDeletions(workInProgress) {
70
+ workInProgress.progressedFirstDeletion =
71
+ workInProgress.progressedLastDeletion =
72
+ null;
73
+ }
74
+
75
+ function transferDeletions(workInProgress) {
76
+ // Any deletions get added first into the effect list.
77
+ workInProgress.firstEffect = workInProgress.progressedFirstDeletion;
78
+ workInProgress.lastEffect = workInProgress.progressedLastDeletion;
79
+ }
80
+
81
function reconcileChildren(current, workInProgress, nextChildren) {
82
const priorityLevel = workInProgress.pendingWorkPriority;
83
reconcileChildrenAtPriority(current, workInProgress, nextChildren, priorityLevel);
@@ -90,23 +102,31 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>, g
102
// If the current child is the same as the work in progress, it means that
103
// we haven't yet started any work on these children. Therefore, we use
104
// the clone algorithm to create a copy of all the current children.
105
+
106
+ // If we had any progressed work already, that is invalid at this point so
107
+ // let's throw it out.
108
+ clearDeletions(workInProgress);
109
+
110
workInProgress.child = reconcileChildFibers(
111
workInProgress,
112
workInProgress.child,
113
nextChildren,
114
priorityLevel
115
);
116
+
117
+ transferDeletions(workInProgress);
118
} else {
100
- // If, on the other hand, we don't have a current fiber or if it is
101
- // already using a clone, that means we've already begun some work on this
102
- // tree and we can continue where we left off by reconciling against the
103
- // existing children.
119
+ // If, on the other hand, it is already using a clone, that means we've
120
+ // already begun some work on this tree and we can continue where we left
121
+ // off by reconciling against the existing children.
122
workInProgress.child = reconcileChildFibersInPlace(
123
workInProgress,
124
workInProgress.child,
125
nextChildren,
126
priorityLevel
127
);
128
+
129
+ transferDeletions(workInProgress);
130
}
131
markChildAsProgressed(current, workInProgress, priorityLevel);
132
}
@@ -353,6 +373,12 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>, g
373
// return null;
374
// }
375
376
+ if (current && workInProgress.child === current.child) {
377
+ // If we had any progressed work already, that is invalid at this point so
378
+ // let's throw it out.
379
+ clearDeletions(workInProgress);
380
+ }
381
+
382
cloneChildFibers(current, workInProgress);
383
markChildAsProgressed(current, workInProgress, priorityLevel);
384
return workInProgress.child;
src/renderers/shared/fiber/ReactFiberCompleteWork.js
+12
-26
@@ -19,6 +19,7 @@ import type { ReifiedYield } from 'ReactReifiedYield';
19
20
var { reconcileChildFibers } = require('ReactChildFiber');
21
var ReactTypeOfWork = require('ReactTypeOfWork');
22
+var ReactTypeOfSideEffect = require('ReactTypeOfSideEffect');
23
var {
24
IndeterminateComponent,
25
FunctionalComponent,
@@ -31,6 +32,9 @@ var {
32
YieldComponent,
33
Fragment,
34
} = ReactTypeOfWork;
35
+var {
36
+ Update,
37
+} = ReactTypeOfSideEffect;
38
39
module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
40
@@ -38,28 +42,10 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
42
const createTextInstance = config.createTextInstance;
43
const prepareUpdate = config.prepareUpdate;
44
41
- function markForPreEffect(workInProgress : Fiber) {
42
- // Schedule a side-effect on this fiber, BEFORE the children's side-effects.
43
- if (workInProgress.firstEffect) {
44
- workInProgress.nextEffect = workInProgress.firstEffect;
45
- workInProgress.firstEffect = workInProgress;
46
- } else {
47
- workInProgress.firstEffect = workInProgress;
48
- workInProgress.lastEffect = workInProgress;
49
- }
50
- }
51
-
52
- // TODO: It's possible this will create layout thrash issues because mutations
53
- // of the DOM and life-cycles are interleaved. E.g. if a componentDidMount
54
- // of a sibling reads, then the next sibling updates and reads etc.
55
- function markForPostEffect(workInProgress : Fiber) {
56
- // Schedule a side-effect on this fiber, AFTER the children's side-effects.
57
- if (workInProgress.lastEffect) {
58
- workInProgress.lastEffect.nextEffect = workInProgress;
59
- } else {
60
- workInProgress.firstEffect = workInProgress;
61
- }
62
- workInProgress.lastEffect = workInProgress;
45
+ function markUpdate(workInProgress : Fiber) {
46
+ // Tag the fiber with an update effect. This turns a Placement into
47
+ // an UpdateAndPlacement.
48
+ workInProgress.effectTag |= Update;
49
}
50
51
function transferOutput(child : ?Fiber, returnFiber : Fiber) {
@@ -143,7 +129,7 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
129
// Transfer update queue to callbackList field so callbacks can be
130
// called during commit phase.
131
workInProgress.callbackList = workInProgress.updateQueue;
146
- markForPostEffect(workInProgress);
132
+ markUpdate(workInProgress);
133
return null;
134
case HostContainer:
135
transferOutput(workInProgress.child, workInProgress);
@@ -152,7 +138,7 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
138
// all the other side-effects in the subtree. We need to schedule it
139
// before so that the entire tree is up-to-date before the life-cycles
140
// are invoked.
155
- markForPreEffect(workInProgress);
141
+ markUpdate(workInProgress);
142
return null;
143
case HostComponent:
144
let newProps = workInProgress.pendingProps;
@@ -172,7 +158,7 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
158
const instance : I = workInProgress.stateNode;
159
if (prepareUpdate(instance, oldProps, newProps, children)) {
160
// This returns true if there was something to update.
175
- markForPreEffect(workInProgress);
161
+ markUpdate(workInProgress);
162
}
163
// TODO: Is this actually ever going to change? Why set it every time?
164
workInProgress.output = instance;
@@ -197,7 +183,7 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
183
if (current && workInProgress.stateNode != null) {
184
// If we have an alternate, that means this is an update and we need to
185
// schedule a side-effect to do the updates.
200
- markForPreEffect(workInProgress);
186
+ markUpdate(workInProgress);
187
} else {
188
if (typeof newText !== 'string') {
189
if (workInProgress.stateNode === null) {
src/renderers/shared/fiber/ReactFiberScheduler.js
+27
-4
@@ -30,6 +30,12 @@ var {
30
SynchronousPriority,
31
} = require('ReactPriorityLevel');
32
33
+var {
34
+ NoEffect,
35
+ Update,
36
+ PlacementAndUpdate,
37
+} = require('ReactTypeOfSideEffect');
38
+
39
var timeHeuristicForUnitOfWork = 1;
40
41
export type Scheduler = {
@@ -106,7 +112,10 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
112
let effectfulFiber = finishedWork.firstEffect;
113
while (effectfulFiber) {
114
const current = effectfulFiber.alternate;
109
- commitWork(current, effectfulFiber);
115
+ if (effectfulFiber.effectTag === Update ||
116
+ effectfulFiber.effectTag === PlacementAndUpdate) {
117
+ commitWork(current, effectfulFiber);
118
+ }
119
const next = effectfulFiber.nextEffect;
120
// Ensure that we clean these up so that we don't accidentally keep them.
121
// I'm not actually sure this matters because we can't reset firstEffect
@@ -151,12 +160,26 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
160
workInProgress.pendingProps = null;
161
workInProgress.updateQueue = null;
162
163
+ // If this fiber had side-effects, we append it to the end of its own
164
+ // effect list.
165
+ if (workInProgress.effectTag !== NoEffect) {
166
+ // Schedule a side-effect on this fiber, AFTER the children's
167
+ // side-effects. We can perform certain side-effects earlier if
168
+ // needed, by doing multiple passes over the effect list.
169
+ if (workInProgress.lastEffect) {
170
+ workInProgress.lastEffect.nextEffect = workInProgress;
171
+ } else {
172
+ workInProgress.firstEffect = workInProgress;
173
+ }
174
+ workInProgress.lastEffect = workInProgress;
175
+ }
176
+
177
const returnFiber = workInProgress.return;
178
179
if (returnFiber) {
157
- // Ensure that the first and last effect of the parent corresponds
158
- // to the children's first and last effect. This probably relies on
159
- // children completing in order.
180
+ // Append all the effects of the subtree and this fiber onto the effect
181
+ // list of the parent. The completion order of the children affects the
182
+ // side-effect order.
183
if (!returnFiber.firstEffect) {
184
returnFiber.firstEffect = workInProgress.firstEffect;
185
}