diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 5c20e5f398b29..d0109ddfebd4f 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -1187,6 +1187,14 @@ src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js * invokes ref callbacks after insertion/update/unmount * supports string refs +src/renderers/shared/fiber/__tests__/ReactIncrementalUpdates-test.js +* applies updates in order of priority +* applies updates with equal priority in insertion order +* only drops updates with equal or lesser priority when replaceState is called +* can abort an update, schedule additional updates, and resume +* can abort an update, schedule a replaceState, and resume +* does not call callbacks that are scheduled by another callback until a later commit + src/renderers/shared/fiber/__tests__/ReactTopLevelFragment-test.js * should render a simple fragment at the top of a component * should preserve state when switching from a single child diff --git a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js index e823abe8ca7d8..e83b0cca93e53 100644 --- a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js +++ b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js @@ -20,7 +20,11 @@ const { Callback: CallbackEffect, } = require('ReactTypeOfSideEffect'); -const { NoWork } = require('ReactPriorityLevel'); +const { + NoWork, + SynchronousPriority, + TaskPriority, +} = require('ReactPriorityLevel'); type PartialState = $Subtype | @@ -37,22 +41,57 @@ type Update = { next: Update | null, }; +// Linked-list of updates +// +// - A "pending" update is one that has been scheduled but not yet used +// for reconciliation. +// - A "progressed" update is an update that was used for reconciliation, but +// has not yet been flushed. +// +// The queue maintains a pointer to the last progressed update in the list. +// Updates that come after that pointer are pending. The pointer is set to the +// end of the list during reconciliation. +// +// Pending updates are sorted by priority then insertion. Progressed updates +// are sorted by the order in which they were applied during reconciliation, +// which may not be by priority: if a component bails out before the updates are +// committed, in the next render, the progressed updates are applied in the same +// order that they were previously, even if a higher priority update comes in. +// +// Once a progressed update is flushed/committed, it's removed from the queue. export type UpdateQueue = { - // Points to the first update. first: Update | null, + // A pointer to the last progressed update in the queue. This may be null + // even in a non-empty queue, if all the updates are pending. lastProgressedUpdate: Update | null, - // Points to the last update. last: Update | null, }; -function getFirstPendingUpdate(queue : UpdateQueue) { +function comparePriority(a : PriorityLevel, b : PriorityLevel) : number { + // When comparing update priorities, treat sync and Task work as equal. + // TODO: Could we avoid the need for this by always coercing sync priority + // to Task when scheduling an update? + if ((a === TaskPriority || a === SynchronousPriority) && + (b === TaskPriority || b === SynchronousPriority)) { + return 0; + } + if (a === NoWork && b !== NoWork) { + return -Infinity; + } + if (a !== NoWork && b === NoWork) { + return Infinity; + } + return a - b; +} + +function getFirstPendingUpdate(queue : UpdateQueue) : Update | null { if (queue.lastProgressedUpdate) { return queue.lastProgressedUpdate.next; } return queue.first; } -function getFirstProgressedUpdate(queue : UpdateQueue) { +function getFirstProgressedUpdate(queue : UpdateQueue) : Update | null { if (queue.lastProgressedUpdate) { return queue.first; } @@ -60,8 +99,12 @@ function getFirstProgressedUpdate(queue : UpdateQueue) { } function hasPendingUpdate(queue : UpdateQueue, priorityLevel : PriorityLevel) : boolean { - // TODO: Check priority level - return Boolean(getFirstPendingUpdate(queue)); + const firstPendingUpdate = getFirstPendingUpdate(queue); + if (!firstPendingUpdate) { + return false; + } + // Return true if the first pending update has greater or equal priority. + return comparePriority(firstPendingUpdate.priorityLevel, priorityLevel) <= 0; } exports.hasPendingUpdate = hasPendingUpdate; @@ -72,7 +115,7 @@ function ensureUpdateQueue(fiber : Fiber) : UpdateQueue { // We already have an update queue. return fiber.updateQueue; } - const queue = { + const queue : UpdateQueue = { first: null, lastProgressedUpdate: null, last: null, @@ -87,20 +130,61 @@ function ensureUpdateQueue(fiber : Fiber) : UpdateQueue { } exports.ensureUpdateQueue = ensureUpdateQueue; -function insertUpdateIntoQueue(queue : UpdateQueue, update : Update, priorityLevel : PriorityLevel) : void { +function insertUpdateIntoQueue(queue : UpdateQueue, update : Update) : void { // Add a pending update to the end of the queue. - // TODO: Insert updates in order of priority. if (!queue.last) { // The queue is empty. queue.first = queue.last = update; - } else { - // The queue is not empty. Append the update to the end. + return; + } + // The queue is not empty. Insert the new update into the queue, sorted by + // priority then insertion order. + const firstPendingUpdate = getFirstPendingUpdate(queue); + if (!firstPendingUpdate && queue.last) { + // This is the first pending update. Add it to the end of the queue. queue.last.next = update; queue.last = update; - if (queue.lastProgressedUpdate && !queue.lastProgressedUpdate.next) { - queue.lastProgressedUpdate.next = update; + return; + } + + const priorityLevel = update.priorityLevel; + const lastPendingUpdate = queue.last; + + let insertAfter; + let insertBefore; + if (queue.last && comparePriority(queue.last.priorityLevel, priorityLevel) <= 0) { + // Fast path where the incoming update has equal or lower priority than the + // last pending update. We can just append it to the end of the queue. + insertAfter = lastPendingUpdate; + insertBefore = null; + } else { + // Loop through the pending updates to find the first one with lower + // priority than the incoming update. Insert the incoming update before + // that one. + insertAfter = queue.lastProgressedUpdate; + insertBefore = firstPendingUpdate; + while ( + insertBefore && + comparePriority(insertBefore.priorityLevel, priorityLevel) <= 0 + ) { + insertAfter = insertBefore; + insertBefore = insertBefore.next; } } + + if (insertAfter) { + insertAfter.next = update; + } else { + // This is the first item in the queue. + queue.first = update; + } + + if (insertBefore) { + update.next = insertBefore; + } else { + // This is the last item in the queue. + queue.last = update; + } } function addUpdate( @@ -108,7 +192,7 @@ function addUpdate( partialState : PartialState | null, priorityLevel : PriorityLevel ) : void { - const update = { + const update : Update = { priorityLevel, partialState, callback: null, @@ -116,7 +200,7 @@ function addUpdate( isForced: false, next: null, }; - insertUpdateIntoQueue(queue, update, priorityLevel); + insertUpdateIntoQueue(queue, update); } exports.addUpdate = addUpdate; @@ -125,7 +209,7 @@ function addReplaceUpdate( state : any | null, priorityLevel : PriorityLevel ) : void { - const replaceUpdate = { + const update : Update = { priorityLevel, partialState: state, callback: null, @@ -133,29 +217,57 @@ function addReplaceUpdate( isForced: false, next: null, }; - + // Add a pending update to the end of the queue. if (!queue.last) { // The queue is empty. - queue.first = queue.last = replaceUpdate; - } else { - // The queue is not empty. + queue.first = queue.last = update; + return; + } + // The queue is not empty. Insert the new update into the queue, sorted by + // priority then insertion order. Since this is a replace, drop all pending + // updates with equal priority. We can't drop updates with higher priority, + // because they might be flushed in an earlier commit. We'll drop them during + // the commit phase if necessary. + const firstPendingUpdate = getFirstPendingUpdate(queue); + if (!firstPendingUpdate && queue.last) { + // This is the first pending update. Add it to the end of the queue. + queue.last.next = update; + queue.last = update; + return; + } - // Drop all existing pending updates. - // TODO: Only drop updates with matching priority. - if (queue.lastProgressedUpdate) { - queue.lastProgressedUpdate.next = replaceUpdate; - queue.last = replaceUpdate; - } else { - // Drop everything - queue.first = queue.last = replaceUpdate; + // Find the last pending update with equal priority. + let replaceAfter = queue.lastProgressedUpdate; + let replaceBefore = firstPendingUpdate; + if (replaceBefore) { + let comparison = Infinity; + while (replaceBefore && + (comparison = comparePriority(replaceBefore.priorityLevel, priorityLevel)) <= 0) { + if (comparison < 0) { + replaceAfter = replaceBefore; + } + replaceBefore = replaceBefore.next; } } + if (replaceAfter) { + replaceAfter.next = update; + } else { + // This is the first item in the queue. + queue.first = update; + } + + if (replaceBefore) { + update.next = replaceBefore; + } else { + // This is the last item in the queue. + queue.last = update; + } } exports.addReplaceUpdate = addReplaceUpdate; function addForceUpdate(queue : UpdateQueue, priorityLevel : PriorityLevel) : void { - const update = { + const update : Update = { priorityLevel, partialState: null, callback: null, @@ -163,21 +275,24 @@ function addForceUpdate(queue : UpdateQueue, priorityLevel : PriorityLevel) : vo isForced: true, next: null, }; - insertUpdateIntoQueue(queue, update, priorityLevel); + insertUpdateIntoQueue(queue, update); } exports.addForceUpdate = addForceUpdate; function addCallback(queue : UpdateQueue, callback: Callback, priorityLevel : PriorityLevel) : void { - if (getFirstPendingUpdate(queue) && queue.last && !queue.last.callback) { + if (getFirstPendingUpdate(queue) && + queue.last && + (queue.last.priorityLevel === priorityLevel) && + !queue.last.callback) { // If pending updates already exist, and the last pending update does not - // have a callback, we can add the new callback to that update. - // TODO: Add an additional check to ensure the priority matches. + // have a callback, and the priority levels are equal, we can add the + // incoming callback to that update to avoid an extra allocation. queue.last.callback = callback; return; } - const update = { + const update : Update = { priorityLevel, partialState: null, callback, @@ -185,25 +300,13 @@ function addCallback(queue : UpdateQueue, callback: Callback, priorityLevel : Pr isForced: false, next: null, }; - insertUpdateIntoQueue(queue, update, priorityLevel); + insertUpdateIntoQueue(queue, update); } exports.addCallback = addCallback; function getPendingPriority(queue : UpdateQueue) : PriorityLevel { - // Loop through the pending updates to recompute the pending priority. - // TODO: Once updates are sorted, just read from the first pending update. - let priorityLevel = NoWork; - // Start with first pending update - let update = getFirstPendingUpdate(queue); - while (update) { - if (priorityLevel === NoWork || - priorityLevel >= update.priorityLevel) { - // Update pending priority - priorityLevel = update.priorityLevel; - } - update = update.next; - } - return priorityLevel; + const firstPendingUpdate = getFirstPendingUpdate(queue); + return firstPendingUpdate ? firstPendingUpdate.priorityLevel : NoWork; } exports.getPendingPriority = getPendingPriority; @@ -225,23 +328,25 @@ function beginUpdateQueue( props : any, priorityLevel : PriorityLevel ) : any { - // This merges the entire update queue into a single object, not just the - // pending updates, because the previous state and props may have changed. - // TODO: Would memoization be worth it? + // Applies updates with matching priority to the previous state to create + // a new state object. If an update was used previously but never flushed + // due to a bail out, it's used again regardless of its priority. // Reset these flags. We'll update them while looping through the queue. workInProgress.effectTag &= ~ForceUpdate; workInProgress.effectTag &= ~CallbackEffect; + const prevLastProgressedUpdate = queue.lastProgressedUpdate; let state = prevState; let dontMutatePrevState = true; let isEmpty = true; - - // TODO: Stop merging once we reach an update whose priority doesn't match. - // Should this also apply to updates that were previous merged but bailed out? - let update : Update | null = queue.first; + let alreadyProgressedUpdate = Boolean(prevLastProgressedUpdate); let lastProgressedUpdate = null; - while (update) { + let update : Update | null = queue.first; + while (update && ( + alreadyProgressedUpdate || + comparePriority(update.priorityLevel, priorityLevel) <= 0 + )) { let partialState; if (update.isReplace) { // A replace should drop all previous updates in the queue, so @@ -267,10 +372,15 @@ function beginUpdateQueue( if (update.callback) { workInProgress.effectTag |= CallbackEffect; } + if (update === prevLastProgressedUpdate) { + alreadyProgressedUpdate = false; + } lastProgressedUpdate = update; update = update.next; } + + // Mark the point in the queue where we stopped applying updates queue.lastProgressedUpdate = lastProgressedUpdate; if (isEmpty) { @@ -296,8 +406,9 @@ function commitUpdateQueue(finishedWork : Fiber, queue : UpdateQueue, context : } } - // Drop all completed updates, leaving only the pending updates. + // Drop all comitted updates, leaving only the pending updates. queue.first = getFirstPendingUpdate(queue); + queue.lastProgressedUpdate = null; if (!queue.first) { queue.last = queue.lastProgressedUpdate = null; diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalUpdates-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalUpdates-test.js new file mode 100644 index 0000000000000..7dbf05ad52bfc --- /dev/null +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalUpdates-test.js @@ -0,0 +1,248 @@ +/** + * Copyright 2013-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + * + * @emails react-core + */ + +'use strict'; + +var React; +var ReactNoop; + +describe('ReactIncrementalUpdates', () => { + beforeEach(() => { + jest.resetModuleRegistry(); + React = require('React'); + ReactNoop = require('ReactNoop'); + }); + + function div(...children) { + children = children.map(c => typeof c === 'string' ? { text: c } : c); + return { type: 'div', children, prop: undefined }; + } + + function span(prop) { + return { type: 'span', children: [], prop }; + } + + it('applies updates in order of priority', () => { + let state; + class Foo extends React.Component { + state = {}; + componentDidMount() { + ReactNoop.performAnimationWork(() => { + // Has Animation priority + this.setState({ b: 'b' }); + this.setState({ c: 'c' }); + }); + // Has Task priority + this.setState({ a: 'a' }); + } + render() { + state = this.state; + return
; + } + } + + ReactNoop.render(); + ReactNoop.flush(); + expect(Object.keys(state)).toEqual(['a', 'b', 'c']); + }); + + it('applies updates with equal priority in insertion order', () => { + let state; + class Foo extends React.Component { + state = {}; + componentDidMount() { + // All have Task priority + this.setState({ a: 'a' }); + this.setState({ b: 'b' }); + this.setState({ c: 'c' }); + } + render() { + state = this.state; + return
; + } + } + + ReactNoop.render(); + ReactNoop.flush(); + expect(Object.keys(state)).toEqual(['a', 'b', 'c']); + }); + + it('only drops updates with equal or lesser priority when replaceState is called', () => { + let instance; + const Foo = React.createClass({ + getInitialState() { + return {}; + }, + render() { + instance = this; + return ( + + ); + }, + }); + + ReactNoop.render(); + ReactNoop.flush(); + + instance.setState({ x: 'x' }); + instance.setState({ y: 'y' }); + ReactNoop.performAnimationWork(() => { + instance.setState({ a: 'a' }); + instance.setState({ b: 'b' }); + }); + instance.replaceState({ c: 'c' }); + instance.setState({ d: 'd' }); + + ReactNoop.flushAnimationPri(); + // Even though a replaceState has been already scheduled, it hasn't been + // flushed yet because it has low priority. + expect(ReactNoop.getChildren()).toEqual([span('ab')]); + + ReactNoop.flush(); + // Now the rest of the updates are flushed. + expect(ReactNoop.getChildren()).toEqual([span('cd')]); + }); + + it('can abort an update, schedule additional updates, and resume', () => { + let instance; + class Foo extends React.Component { + state = {}; + render() { + instance = this; + return ( + + ); + } + } + + ReactNoop.render(); + ReactNoop.flush(); + + let progressedUpdates = []; + function createUpdate(letter) { + return () => { + progressedUpdates.push(letter); + return { + [letter]: letter, + }; + }; + } + + instance.setState(createUpdate('a')); + instance.setState(createUpdate('b')); + instance.setState(createUpdate('c')); + + // Do just enough work to begin the update but not enough to flush it + ReactNoop.flushDeferredPri(20); + expect(ReactNoop.getChildren()).toEqual([span('')]); + expect(progressedUpdates).toEqual(['a', 'b', 'c']); + + instance.setState(createUpdate('f')); + ReactNoop.performAnimationWork(() => { + instance.setState(createUpdate('d')); + instance.setState(createUpdate('e')); + }); + instance.setState(createUpdate('g')); + + // Updates a, b, and c were aborted, so they should be applied first even + // though they have low priority. Update f was scheduled after the render + // was aborted, so it should come after d and e, which have higher priority. + ReactNoop.flush(); + expect(ReactNoop.getChildren()).toEqual([span('abcdefg')]); + expect(progressedUpdates).toEqual(['a', 'b', 'c', 'd', 'e', 'f', 'g']); + }); + + it('can abort an update, schedule a replaceState, and resume', () => { + let instance; + const Foo = React.createClass({ + getInitialState() { + return {}; + }, + render() { + instance = this; + return ( + + ); + }, + }); + + ReactNoop.render(); + ReactNoop.flush(); + + let progressedUpdates = []; + function createUpdate(letter) { + return () => { + progressedUpdates.push(letter); + return { + [letter]: letter, + }; + }; + } + + instance.setState(createUpdate('a')); + instance.setState(createUpdate('b')); + instance.setState(createUpdate('c')); + + // Do just enough work to begin the update but not enough to flush it + ReactNoop.flushDeferredPri(20); + expect(ReactNoop.getChildren()).toEqual([span('')]); + expect(progressedUpdates).toEqual(['a', 'b', 'c']); + + progressedUpdates = []; + + instance.setState(createUpdate('f')); + ReactNoop.performAnimationWork(() => { + instance.setState(createUpdate('d')); + instance.replaceState(createUpdate('e')); + }); + instance.setState(createUpdate('g')); + + // Updates a, b, and c were aborted, so they should be applied first even + // though they have low priority. Update f was scheduled after the render + // was aborted, so it should come after d and e, which have higher priority. + // Because e is a replaceState, d gets dropped. + ReactNoop.flush(); + expect(ReactNoop.getChildren()).toEqual([span('efg')]); + // Ensure that update d is not progressed before being replaced. + expect(progressedUpdates).toEqual(['e', 'f', 'g']); + }); + + it('does not call callbacks that are scheduled by another callback until a later commit', () => { + let ops = []; + class Foo extends React.Component { + state = {}; + componentDidMount() { + ops.push('did mount'); + this.setState({ a: 'a' }, () => { + ops.push('callback a'); + this.setState({ b: 'b' }, () => { + ops.push('callback b'); + }); + }); + } + render() { + ops.push('render'); + return
; + } + } + + ReactNoop.render(); + ReactNoop.flush(); + expect(ops).toEqual([ + 'render', + 'did mount', + 'render', + 'callback a', + 'render', + 'callback b', + ]); + }); +});