Reset effect list when we recompute children
We only use the effect list when we reuse our progressed children. Otherwise it needs to reset to null. In all other branches, other than bailoutOnLowPriority, we revisit the children which recreates this list. We should also not add fibers to their own effect list since it becomes difficult to perform work on self without touching the children too. Nothing else does that neither. Since that means that the root isn't added to an effect list we need to special case the root.
Sebastian Markbage committed
Oct 6, 2016 at 19:17 UTC
43645a658625317d56ce526591bc143ab31b6009
3 files changed
+36
-16
src/renderers/shared/fiber/ReactFiber.js
+3
-2
@@ -221,8 +221,9 @@ exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fi
221
// extra memory if needed.
222
let alt = fiber.alternate;
223
if (alt) {
224
- // Whenever we clone, we do so to get a new work in progress.
225
- // This ensures that we've reset these in the new tree.
224
+ // If we clone, then we do so from the "current" state. The current state
225
+ // can't have any side-effects that are still valid so we reset just to be
226
+ // sure.
227
alt.effectTag = NoEffect;
228
alt.nextEffect = null;
229
alt.firstEffect = null;
src/renderers/shared/fiber/ReactFiberBeginWork.js
+11
@@ -403,9 +403,15 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>, g
403
404
function bailoutOnLowPriority(current, workInProgress) {
405
if (current) {
406
+ // TODO: If I have started work on this node, mark it as finished, then
407
+ // return do other work, come back and hit this node... we killed that
408
+ // work. It is now in an inconsistent state. We probably need to check
409
+ // progressedChild or something.
410
workInProgress.child = current.child;
411
workInProgress.memoizedProps = current.memoizedProps;
412
workInProgress.output = current.output;
413
+ workInProgress.firstEffect = null;
414
+ workInProgress.lastEffect = null;
415
}
416
return null;
417
}
@@ -416,6 +422,11 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>, g
422
return bailoutOnLowPriority(current, workInProgress);
423
}
424
425
+ // If we don't bail out, we're going be recomputing our children so we need
426
+ // to drop our effect list.
427
+ workInProgress.firstEffect = null;
428
+ workInProgress.lastEffect = null;
429
+
430
if (workInProgress.progressedPriority === priorityLevel) {
431
// If we have progressed work on this priority level already, we can
432
// proceed this that as the child.
src/renderers/shared/fiber/ReactFiberScheduler.js
+22
-14
@@ -143,6 +143,13 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
143
effectfulFiber.nextEffect = null;
144
effectfulFiber = next;
145
}
146
+
147
+ // Finally if the root itself had an effect, we perform that since it is not
148
+ // part of the effect list.
149
+ if (finishedWork.effectTag !== NoEffect) {
150
+ const current = finishedWork.alternate;
151
+ commitWork(current, finishedWork);
152
+ }
153
}
154
155
function resetWorkPriority(workInProgress : Fiber) {
@@ -179,20 +186,6 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
186
workInProgress.pendingProps = null;
187
workInProgress.updateQueue = null;
188
182
- // If this fiber had side-effects, we append it to the end of its own
183
- // effect list.
184
- if (workInProgress.effectTag !== NoEffect) {
185
- // Schedule a side-effect on this fiber, AFTER the children's
186
- // side-effects. We can perform certain side-effects earlier if
187
- // needed, by doing multiple passes over the effect list.
188
- if (workInProgress.lastEffect) {
189
- workInProgress.lastEffect.nextEffect = workInProgress;
190
- } else {
191
- workInProgress.firstEffect = workInProgress;
192
- }
193
- workInProgress.lastEffect = workInProgress;
194
- }
195
-
189
const returnFiber = workInProgress.return;
190
191
if (returnFiber) {
@@ -208,6 +201,21 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
201
}
202
returnFiber.lastEffect = workInProgress.lastEffect;
203
}
204
+
205
+ // If this fiber had side-effects, we append it AFTER the children's
206
+ // side-effects. We can perform certain side-effects earlier if
207
+ // needed, by doing multiple passes over the effect list. We don't want
208
+ // to schedule our own side-effect on our own list because if end up
209
+ // reusing children we'll schedule this effect onto itself since we're
210
+ // at the end.
211
+ if (workInProgress.effectTag !== NoEffect) {
212
+ if (returnFiber.lastEffect) {
213
+ returnFiber.lastEffect.nextEffect = workInProgress;
214
+ } else {
215
+ returnFiber.firstEffect = workInProgress;
216
+ }
217
+ returnFiber.lastEffect = workInProgress;
218
+ }
219
}
220
221
if (next) {