Skip to content

Commit d99be5c

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 ee89fe7 commit d99be5c

14 files changed

+184
-259
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: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -403,7 +403,7 @@ var ReactNoop = {
403403
logHostInstances(container.children, depth + 1);
404404
}
405405

406-
function logUpdateQueue(updateQueue: UpdateQueue, depth) {
406+
function logUpdateQueue(updateQueue: UpdateQueue<mixed>, depth) {
407407
log(' '.repeat(depth + 1) + 'QUEUED UPDATES');
408408
const firstUpdate = updateQueue.first;
409409
if (!firstUpdate) {

src/renderers/shared/fiber/ReactFiber.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ export type Fiber = {|
109109
memoizedProps: any, // The props used to create the output.
110110

111111
// A queue of state updates and callbacks.
112-
updateQueue: UpdateQueue | null,
112+
updateQueue: UpdateQueue<mixed> | null,
113113

114114
// The state used to create the output
115115
memoizedState: any,

src/renderers/shared/fiber/ReactFiberBeginWork.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,6 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
328328
workInProgress,
329329
updateQueue,
330330
null,
331-
prevState,
332331
null,
333332
renderExpirationTime,
334333
);
@@ -338,7 +337,7 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
338337
resetHydrationState();
339338
return bailoutOnAlreadyFinishedWork(current, workInProgress);
340339
}
341-
const element = state.element;
340+
const element = state !== null ? state.element : null;
342341
if (
343342
(current === null || current.child === null) &&
344343
enterHydrationState(workInProgress)

src/renderers/shared/fiber/ReactFiberClassComponent.js

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -398,7 +398,7 @@ module.exports = function(
398398
const unmaskedContext = getUnmaskedContext(workInProgress);
399399

400400
instance.props = props;
401-
instance.state = state;
401+
instance.state = workInProgress.memoizedState = state;
402402
instance.refs = emptyObject;
403403
instance.context = getMaskedContext(workInProgress, unmaskedContext);
404404

@@ -422,7 +422,6 @@ module.exports = function(
422422
workInProgress,
423423
updateQueue,
424424
instance,
425-
state,
426425
props,
427426
renderExpirationTime,
428427
);
@@ -589,7 +588,6 @@ module.exports = function(
589588
workInProgress,
590589
workInProgress.updateQueue,
591590
instance,
592-
oldState,
593591
newProps,
594592
renderExpirationTime,
595593
);

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: 2 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -286,38 +286,16 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
286286
callback,
287287
);
288288
}
289-
const isTopLevelUnmount = nextState.element === null;
290289
const update = {
291290
expirationTime,
292291
partialState: nextState,
293292
callback,
294293
isReplace: false,
295294
isForced: false,
296-
isTopLevelUnmount,
295+
nextCallback: null,
297296
next: null,
298297
};
299-
const update2 = insertUpdateIntoFiber(current, update);
300-
301-
if (isTopLevelUnmount) {
302-
// TODO: Redesign the top-level mount/update/unmount API to avoid this
303-
// special case.
304-
const queue1 = current.updateQueue;
305-
const queue2 = current.alternate !== null
306-
? current.alternate.updateQueue
307-
: null;
308-
309-
// Drop all updates that are lower-priority, so that the tree is not
310-
// remounted. We need to do this for both queues.
311-
if (queue1 !== null && update.next !== null) {
312-
update.next = null;
313-
queue1.last = update;
314-
}
315-
if (queue2 !== null && update2 !== null && update2.next !== null) {
316-
update2.next = null;
317-
queue2.last = update;
318-
}
319-
}
320-
298+
insertUpdateIntoFiber(current, update);
321299
scheduleWork(current, expirationTime);
322300
}
323301

0 commit comments

Comments
 (0)