Skip to content

Commit 9a31c91

Browse files
committed
Deterministic updates
High priority updates typically require less work to render than low priority ones. It's beneficial to flush those first, in their own batch, before working on more expensive low priority ones. We do this even if a high priority is scheduled after a low priority one. However, we don't want this reordering of updates to affect the terminal state. State should be deterministic: once all work has been flushed, the final state should be the same regardless of how they were scheduled. To get both properties, we store updates on the queue in insertion order instead of priority order (always append). Then, when processing the queue, we skip over updates with insufficient priority. Instead of removing updates from the queue right after processing them, we only remove them if there are no unprocessed updates before it in the list. This means that updates may be processed more than once. As a bonus, the new implementation is simpler and requires less code.
1 parent 5b56a0f commit 9a31c91

15 files changed

+346
-325
lines changed

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
"glob-stream": "^6.1.0",
6161
"gzip-js": "~0.3.2",
6262
"gzip-size": "^3.0.0",
63+
"jasmine-check": "^1.0.0-rc.0",
6364
"jest": "20.1.0-delta.1",
6465
"jest-config": "20.1.0-delta.1",
6566
"jest-jasmine2": "20.1.0-delta.1",

scripts/jest/test-framework-setup.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,4 +68,6 @@ if (process.env.REACT_CLASS_EQUIVALENCE_TEST) {
6868
return expectation;
6969
};
7070
global.expectDev = expectDev;
71+
72+
require('jasmine-check').install();
7173
}

src/renderers/dom/fiber/__tests__/ReactDOMFiberAsync-test.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -284,8 +284,8 @@ describe('ReactDOMFiberAsync', () => {
284284

285285
// Flush the async updates
286286
jest.runAllTimers();
287-
expect(container.textContent).toEqual('BCAD');
288-
expect(ops).toEqual(['BC', 'BCAD']);
287+
expect(container.textContent).toEqual('ABCD');
288+
expect(ops).toEqual(['BC', 'ABCD']);
289289
});
290290
});
291291
});

src/renderers/noop/ReactNoopEntry.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,9 +296,18 @@ var ReactNoop = {
296296
const root = NoopRenderer.createContainer(container);
297297
roots.set(rootID, root);
298298
return {
299+
render(children: any) {
300+
const work = NoopRenderer.updateRoot(children, root, null);
301+
work.then(() => work.commit());
302+
},
299303
prerender(children: any) {
300304
return NoopRenderer.updateRoot(children, root, null);
301305
},
306+
unmount() {
307+
roots.delete(rootID);
308+
const work = NoopRenderer.updateRoot(null, root, null);
309+
work.then(() => work.commit());
310+
},
302311
getChildren() {
303312
return ReactNoop.getChildren(rootID);
304313
},

src/renderers/shared/fiber/ReactFiberBeginWork.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -350,7 +350,6 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
350350
workInProgress,
351351
updateQueue,
352352
null,
353-
prevState,
354353
null,
355354
renderExpirationTime,
356355
);
@@ -360,7 +359,7 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
360359
resetHydrationState();
361360
return bailoutOnAlreadyFinishedWork(current, workInProgress);
362361
}
363-
const element = state.element;
362+
const element = state !== null ? state.element : null;
364363
if (
365364
root.hydrate &&
366365
(current === null || current.child === null) &&

src/renderers/shared/fiber/ReactFiberClassComponent.js

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ module.exports = function(
111111
callback,
112112
isReplace: false,
113113
isForced: false,
114-
isTopLevelUnmount: false,
114+
nextCallback: null,
115115
next: null,
116116
};
117117
insertUpdateIntoFiber(fiber, update, currentTime);
@@ -136,7 +136,7 @@ module.exports = function(
136136
callback,
137137
isReplace: true,
138138
isForced: false,
139-
isTopLevelUnmount: false,
139+
nextCallback: null,
140140
next: null,
141141
};
142142
insertUpdateIntoFiber(fiber, update, currentTime);
@@ -161,7 +161,7 @@ module.exports = function(
161161
callback,
162162
isReplace: false,
163163
isForced: true,
164-
isTopLevelUnmount: false,
164+
nextCallback: null,
165165
next: null,
166166
};
167167
insertUpdateIntoFiber(fiber, update, currentTime);
@@ -456,7 +456,7 @@ module.exports = function(
456456
const unmaskedContext = getUnmaskedContext(workInProgress);
457457

458458
instance.props = props;
459-
instance.state = state;
459+
instance.state = workInProgress.memoizedState = state;
460460
instance.refs = emptyObject;
461461
instance.context = getMaskedContext(workInProgress, unmaskedContext);
462462

@@ -480,7 +480,6 @@ module.exports = function(
480480
workInProgress,
481481
updateQueue,
482482
instance,
483-
state,
484483
props,
485484
renderExpirationTime,
486485
);
@@ -647,7 +646,6 @@ module.exports = function(
647646
workInProgress,
648647
workInProgress.updateQueue,
649648
instance,
650-
oldState,
651649
newProps,
652650
renderExpirationTime,
653651
);

src/renderers/shared/fiber/ReactFiberCommitWork.js

Lines changed: 19 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,7 @@ var {
2929
clearCaughtError,
3030
} = require('ReactErrorUtils');
3131

32-
var {
33-
Placement,
34-
Update,
35-
Callback,
36-
ContentReset,
37-
} = require('ReactTypeOfSideEffect');
32+
var {Placement, Update, ContentReset} = require('ReactTypeOfSideEffect');
3833

3934
var invariant = require('fbjs/lib/invariant');
4035

@@ -487,16 +482,26 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
487482
}
488483
}
489484

490-
function commitCallbacks(callbackList, context) {
491-
for (let i = 0; i < callbackList.length; i++) {
492-
const callback = callbackList[i];
485+
function commitCallbacks(updateQueue, context) {
486+
let callbackNode = updateQueue.firstCallback;
487+
// Reset the callback list before calling them in case something throws.
488+
updateQueue.firstCallback = updateQueue.lastCallback = null;
489+
490+
while (callbackNode !== null) {
491+
const callback = callbackNode.callback;
492+
// Remove this callback from the update object in case it's still part
493+
// of the queue, so that we don't call it again.
494+
callbackNode.callback = null;
493495
invariant(
494496
typeof callback === 'function',
495497
'Invalid argument passed as callback. Expected a function. Instead ' +
496498
'received: %s',
497499
callback,
498500
);
499501
callback.call(context);
502+
const nextCallback = callbackNode.nextCallback;
503+
callbackNode.nextCallback = null;
504+
callbackNode = nextCallback;
500505
}
501506
}
502507

@@ -529,31 +534,19 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
529534
}
530535
}
531536
}
532-
if (
533-
finishedWork.effectTag & Callback &&
534-
finishedWork.updateQueue !== null
535-
) {
536-
const updateQueue = finishedWork.updateQueue;
537-
if (updateQueue.callbackList !== null) {
538-
// Set the list to null to make sure they don't get called more than once.
539-
const callbackList = updateQueue.callbackList;
540-
updateQueue.callbackList = null;
541-
commitCallbacks(callbackList, instance);
542-
}
537+
const updateQueue = finishedWork.updateQueue;
538+
if (updateQueue !== null) {
539+
commitCallbacks(updateQueue, instance);
543540
}
544541
return;
545542
}
546543
case HostRoot: {
547544
const updateQueue = finishedWork.updateQueue;
548-
if (updateQueue !== null && updateQueue.callbackList !== null) {
549-
// Set the list to null to make sure they don't get called more
550-
// than once.
551-
const callbackList = updateQueue.callbackList;
552-
updateQueue.callbackList = null;
545+
if (updateQueue !== null) {
553546
const instance = finishedWork.child !== null
554547
? finishedWork.child.stateNode
555548
: null;
556-
commitCallbacks(callbackList, instance);
549+
commitCallbacks(updateQueue, instance);
557550
}
558551
return;
559552
}

src/renderers/shared/fiber/ReactFiberReconciler.js

Lines changed: 6 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -283,43 +283,22 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
283283
callback,
284284
);
285285
}
286-
const isTopLevelUnmount = nextState.element === null;
287286
const update = {
288287
priorityLevel,
289288
expirationTime,
290289
partialState: nextState,
291290
callback,
292291
isReplace: false,
293292
isForced: false,
294-
isTopLevelUnmount,
293+
nextCallback: null,
295294
next: null,
296295
};
297-
const update2 = insertUpdateIntoFiber(current, update, currentTime);
298-
299-
if (isTopLevelUnmount) {
300-
// TODO: Redesign the top-level mount/update/unmount API to avoid this
301-
// special case.
302-
const queue1 = current.updateQueue;
303-
const queue2 = current.alternate !== null
304-
? current.alternate.updateQueue
305-
: null;
306-
307-
// Drop all updates that are lower-priority, so that the tree is not
308-
// remounted. We need to do this for both queues.
309-
if (queue1 !== null && update.next !== null) {
310-
update.next = null;
311-
queue1.last = update;
312-
}
313-
if (queue2 !== null && update2 !== null && update2.next !== null) {
314-
update2.next = null;
315-
queue2.last = update;
316-
}
317-
}
296+
insertUpdateIntoFiber(current, update, currentTime);
318297

319298
if (isPrerender) {
320299
// Block the root from committing at this expiration time.
321300
if (root.blockers === null) {
322-
root.blockers = createUpdateQueue();
301+
root.blockers = createUpdateQueue(null);
323302
}
324303
const block = {
325304
priorityLevel: null,
@@ -328,7 +307,7 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
328307
callback: null,
329308
isReplace: false,
330309
isForced: false,
331-
isTopLevelUnmount: false,
310+
nextCallback: null,
332311
next: null,
333312
};
334313
insertUpdateIntoQueue(root.blockers, block, currentTime);
@@ -349,7 +328,7 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
349328
if (blockers === null) {
350329
return;
351330
}
352-
processUpdateQueue(blockers, null, null, null, expirationTime);
331+
processUpdateQueue(blockers, null, null, expirationTime);
353332
expireWork(root, expirationTime);
354333
};
355334
WorkNode.prototype.then = function(callback) {
@@ -400,7 +379,7 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
400379

401380
let completionCallbacks = container.completionCallbacks;
402381
if (completionCallbacks === null) {
403-
completionCallbacks = createUpdateQueue();
382+
completionCallbacks = createUpdateQueue(null);
404383
}
405384

406385
return new WorkNode(container, expirationTime);

src/renderers/shared/fiber/ReactFiberScheduler.js

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -414,21 +414,23 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
414414
// the end of the current batch.
415415
const completionCallbacks = root.completionCallbacks;
416416
if (completionCallbacks !== null) {
417-
processUpdateQueue(completionCallbacks, null, null, null, completedAt);
418-
const callbackList = completionCallbacks.callbackList;
419-
if (callbackList !== null) {
420-
// Add new callbacks to list of completion callbacks
417+
processUpdateQueue(completionCallbacks, null, null, completedAt);
418+
// Add new callbacks to list of completion callbacks
419+
let callbackNode = completionCallbacks.firstCallback;
420+
completionCallbacks.firstCallback = completionCallbacks.lastCallback = null;
421+
while (callbackNode !== null) {
422+
const callback: () => mixed = (callbackNode.callback: any);
423+
// Remove this callback from the update object in case it's still part
424+
// of the queue, so that we don't call it again.
425+
callbackNode.callback = null;
421426
if (rootCompletionCallbackList === null) {
422-
rootCompletionCallbackList = callbackList;
427+
rootCompletionCallbackList = [callback];
423428
} else {
424-
for (let i = 0; i < callbackList.length; i++) {
425-
rootCompletionCallbackList.push(callbackList[i]);
426-
}
427-
}
428-
completionCallbacks.callbackList = null;
429-
if (completionCallbacks.first === null) {
430-
root.completionCallbacks = null;
429+
rootCompletionCallbackList.push(callback);
431430
}
431+
const nextCallback = callbackNode.nextCallback;
432+
callbackNode.nextCallback = null;
433+
callbackNode = nextCallback;
432434
}
433435
}
434436
}
@@ -1648,12 +1650,12 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
16481650
callback,
16491651
isReplace: false,
16501652
isForced: false,
1651-
isTopLevelUnmount: false,
1653+
nextCallback: null,
16521654
next: null,
16531655
};
16541656
const currentTime = recalculateCurrentTime();
16551657
if (root.completionCallbacks === null) {
1656-
root.completionCallbacks = createUpdateQueue();
1658+
root.completionCallbacks = createUpdateQueue(null);
16571659
}
16581660
insertUpdateIntoQueue(root.completionCallbacks, update, currentTime);
16591661
if (expirationTime === root.completedAt) {

0 commit comments

Comments
 (0)