Skip to content

Commit 2d46bad

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 f31b7ab commit 2d46bad

15 files changed

+350
-327
lines changed

package.json

+1
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

+4-2
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ if (process.env.REACT_CLASS_EQUIVALENCE_TEST) {
2828
env.afterEach(() => {
2929
if (console[methodName] !== newMethod && !isSpy(console[methodName])) {
3030
throw new Error(
31-
'Test did not tear down console.' + methodName + ' mock properly.'
31+
'Test did not tear down console.' + methodName + ' mock properly.',
3232
);
3333
}
3434
if (console[methodName].__callCount !== 0) {
@@ -40,7 +40,7 @@ if (process.env.REACT_CLASS_EQUIVALENCE_TEST) {
4040
"spyOn(console, '" +
4141
methodName +
4242
"') and test that the " +
43-
'warning occurs.'
43+
'warning occurs.',
4444
);
4545
}
4646
});
@@ -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

+2-2
Original file line numberDiff line numberDiff line change
@@ -286,8 +286,8 @@ describe('ReactDOMFiberAsync', () => {
286286

287287
// Flush the async updates
288288
jest.runAllTimers();
289-
expect(container.textContent).toEqual('BCAD');
290-
expect(ops).toEqual(['BC', 'BCAD']);
289+
expect(container.textContent).toEqual('ABCD');
290+
expect(ops).toEqual(['BC', 'ABCD']);
291291
});
292292
});
293293
}

src/renderers/noop/ReactNoopEntry.js

+9
Original file line numberDiff line numberDiff line change
@@ -298,9 +298,18 @@ var ReactNoop = {
298298
const root = NoopRenderer.createContainer(container);
299299
roots.set(rootID, root);
300300
return {
301+
render(children: any) {
302+
const work = NoopRenderer.updateRoot(children, root, null);
303+
work.then(() => work.commit());
304+
},
301305
prerender(children: any) {
302306
return NoopRenderer.updateRoot(children, root, null);
303307
},
308+
unmount() {
309+
roots.delete(rootID);
310+
const work = NoopRenderer.updateRoot(null, root, null);
311+
work.then(() => work.commit());
312+
},
304313
getChildren() {
305314
return ReactNoop.getChildren(rootID);
306315
},

src/renderers/shared/fiber/ReactFiberBeginWork.js

+1-2
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,6 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
352352
workInProgress,
353353
updateQueue,
354354
null,
355-
prevState,
356355
null,
357356
renderExpirationTime,
358357
);
@@ -362,7 +361,7 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
362361
resetHydrationState();
363362
return bailoutOnAlreadyFinishedWork(current, workInProgress);
364363
}
365-
const element = state.element;
364+
const element = state !== null ? state.element : null;
366365
if (
367366
root.hydrate &&
368367
(current === null || current.child === null) &&

src/renderers/shared/fiber/ReactFiberClassComponent.js

+4-6
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ module.exports = function(
113113
callback,
114114
isReplace: false,
115115
isForced: false,
116-
isTopLevelUnmount: false,
116+
nextCallback: null,
117117
next: null,
118118
};
119119
insertUpdateIntoFiber(fiber, update, currentTime);
@@ -138,7 +138,7 @@ module.exports = function(
138138
callback,
139139
isReplace: true,
140140
isForced: false,
141-
isTopLevelUnmount: false,
141+
nextCallback: null,
142142
next: null,
143143
};
144144
insertUpdateIntoFiber(fiber, update, currentTime);
@@ -163,7 +163,7 @@ module.exports = function(
163163
callback,
164164
isReplace: false,
165165
isForced: true,
166-
isTopLevelUnmount: false,
166+
nextCallback: null,
167167
next: null,
168168
};
169169
insertUpdateIntoFiber(fiber, update, currentTime);
@@ -458,7 +458,7 @@ module.exports = function(
458458
const unmaskedContext = getUnmaskedContext(workInProgress);
459459

460460
instance.props = props;
461-
instance.state = state;
461+
instance.state = workInProgress.memoizedState = state;
462462
instance.refs = emptyObject;
463463
instance.context = getMaskedContext(workInProgress, unmaskedContext);
464464

@@ -482,7 +482,6 @@ module.exports = function(
482482
workInProgress,
483483
updateQueue,
484484
instance,
485-
state,
486485
props,
487486
renderExpirationTime,
488487
);
@@ -649,7 +648,6 @@ module.exports = function(
649648
workInProgress,
650649
workInProgress.updateQueue,
651650
instance,
652-
oldState,
653651
newProps,
654652
renderExpirationTime,
655653
);

src/renderers/shared/fiber/ReactFiberCommitWork.js

+19-26
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,7 @@ var {
3131
clearCaughtError,
3232
} = require('ReactErrorUtils');
3333

34-
var {
35-
Placement,
36-
Update,
37-
Callback,
38-
ContentReset,
39-
} = require('ReactTypeOfSideEffect');
34+
var {Placement, Update, ContentReset} = require('ReactTypeOfSideEffect');
4035

4136
var invariant = require('fbjs/lib/invariant');
4237

@@ -489,16 +484,26 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
489484
}
490485
}
491486

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

@@ -531,31 +536,19 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
531536
}
532537
}
533538
}
534-
if (
535-
finishedWork.effectTag & Callback &&
536-
finishedWork.updateQueue !== null
537-
) {
538-
const updateQueue = finishedWork.updateQueue;
539-
if (updateQueue.callbackList !== null) {
540-
// Set the list to null to make sure they don't get called more than once.
541-
const callbackList = updateQueue.callbackList;
542-
updateQueue.callbackList = null;
543-
commitCallbacks(callbackList, instance);
544-
}
539+
const updateQueue = finishedWork.updateQueue;
540+
if (updateQueue !== null) {
541+
commitCallbacks(updateQueue, instance);
545542
}
546543
return;
547544
}
548545
case HostRoot: {
549546
const updateQueue = finishedWork.updateQueue;
550-
if (updateQueue !== null && updateQueue.callbackList !== null) {
551-
// Set the list to null to make sure they don't get called more
552-
// than once.
553-
const callbackList = updateQueue.callbackList;
554-
updateQueue.callbackList = null;
547+
if (updateQueue !== null) {
555548
const instance = finishedWork.child !== null
556549
? finishedWork.child.stateNode
557550
: null;
558-
commitCallbacks(callbackList, instance);
551+
commitCallbacks(updateQueue, instance);
559552
}
560553
return;
561554
}

src/renderers/shared/fiber/ReactFiberReconciler.js

+6-27
Original file line numberDiff line numberDiff line change
@@ -278,43 +278,22 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
278278
callback,
279279
);
280280
}
281-
const isTopLevelUnmount = nextState.element === null;
282281
const update = {
283282
priorityLevel,
284283
expirationTime,
285284
partialState: nextState,
286285
callback,
287286
isReplace: false,
288287
isForced: false,
289-
isTopLevelUnmount,
288+
nextCallback: null,
290289
next: null,
291290
};
292-
const update2 = insertUpdateIntoFiber(current, update, currentTime);
293-
294-
if (isTopLevelUnmount) {
295-
// TODO: Redesign the top-level mount/update/unmount API to avoid this
296-
// special case.
297-
const queue1 = current.updateQueue;
298-
const queue2 = current.alternate !== null
299-
? current.alternate.updateQueue
300-
: null;
301-
302-
// Drop all updates that are lower-priority, so that the tree is not
303-
// remounted. We need to do this for both queues.
304-
if (queue1 !== null && update.next !== null) {
305-
update.next = null;
306-
queue1.last = update;
307-
}
308-
if (queue2 !== null && update2 !== null && update2.next !== null) {
309-
update2.next = null;
310-
queue2.last = update;
311-
}
312-
}
291+
insertUpdateIntoFiber(current, update, currentTime);
313292

314293
if (isPrerender) {
315294
// Block the root from committing at this expiration time.
316295
if (root.blockers === null) {
317-
root.blockers = createUpdateQueue();
296+
root.blockers = createUpdateQueue(null);
318297
}
319298
const block = {
320299
priorityLevel: null,
@@ -323,7 +302,7 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
323302
callback: null,
324303
isReplace: false,
325304
isForced: false,
326-
isTopLevelUnmount: false,
305+
nextCallback: null,
327306
next: null,
328307
};
329308
insertUpdateIntoQueue(root.blockers, block, currentTime);
@@ -344,7 +323,7 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
344323
if (blockers === null) {
345324
return;
346325
}
347-
processUpdateQueue(blockers, null, null, null, expirationTime);
326+
processUpdateQueue(blockers, null, null, expirationTime);
348327
expireWork(root, expirationTime);
349328
};
350329
WorkNode.prototype.then = function(callback) {
@@ -395,7 +374,7 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
395374

396375
let completionCallbacks = container.completionCallbacks;
397376
if (completionCallbacks === null) {
398-
completionCallbacks = createUpdateQueue();
377+
completionCallbacks = createUpdateQueue(null);
399378
}
400379

401380
return new WorkNode(container, expirationTime);

src/renderers/shared/fiber/ReactFiberScheduler.js

+16-14
Original file line numberDiff line numberDiff line change
@@ -416,21 +416,23 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
416416
// the end of the current batch.
417417
const completionCallbacks = root.completionCallbacks;
418418
if (completionCallbacks !== null) {
419-
processUpdateQueue(completionCallbacks, null, null, null, completedAt);
420-
const callbackList = completionCallbacks.callbackList;
421-
if (callbackList !== null) {
422-
// Add new callbacks to list of completion callbacks
419+
processUpdateQueue(completionCallbacks, null, null, completedAt);
420+
// Add new callbacks to list of completion callbacks
421+
let callbackNode = completionCallbacks.firstCallback;
422+
completionCallbacks.firstCallback = completionCallbacks.lastCallback = null;
423+
while (callbackNode !== null) {
424+
const callback: () => mixed = (callbackNode.callback: any);
425+
// Remove this callback from the update object in case it's still part
426+
// of the queue, so that we don't call it again.
427+
callbackNode.callback = null;
423428
if (rootCompletionCallbackList === null) {
424-
rootCompletionCallbackList = callbackList;
429+
rootCompletionCallbackList = [callback];
425430
} else {
426-
for (let i = 0; i < callbackList.length; i++) {
427-
rootCompletionCallbackList.push(callbackList[i]);
428-
}
429-
}
430-
completionCallbacks.callbackList = null;
431-
if (completionCallbacks.first === null) {
432-
root.completionCallbacks = null;
431+
rootCompletionCallbackList.push(callback);
433432
}
433+
const nextCallback = callbackNode.nextCallback;
434+
callbackNode.nextCallback = null;
435+
callbackNode = nextCallback;
434436
}
435437
}
436438
}
@@ -1642,12 +1644,12 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
16421644
callback,
16431645
isReplace: false,
16441646
isForced: false,
1645-
isTopLevelUnmount: false,
1647+
nextCallback: null,
16461648
next: null,
16471649
};
16481650
const currentTime = recalculateCurrentTime();
16491651
if (root.completionCallbacks === null) {
1650-
root.completionCallbacks = createUpdateQueue();
1652+
root.completionCallbacks = createUpdateQueue(null);
16511653
}
16521654
insertUpdateIntoQueue(root.completionCallbacks, update, currentTime);
16531655
if (expirationTime === root.completedAt) {

0 commit comments

Comments
 (0)