From ab920970aed771e3e39332aa7e5417845e80101d Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 28 Oct 2016 10:58:40 -0700 Subject: [PATCH 01/14] Ensure first iteration of performAnimationWork loop has right priority I don't think this actually changes any behavior because of the way findNextUnitOfWork works, but I think this is easier to follow. --- src/renderers/shared/fiber/ReactFiberScheduler.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index fd236dbfce55..44af01cb34ae 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -425,17 +425,16 @@ module.exports = function(config : HostConfig) { // Always start from the root nextUnitOfWork = findNextUnitOfWork(); while (nextUnitOfWork && - nextPriorityLevel !== NoWork) { + nextPriorityLevel !== NoWork && + nextPriorityLevel <= AnimationPriority) { nextUnitOfWork = performUnitOfWork(nextUnitOfWork, false); if (!nextUnitOfWork) { // Keep searching for animation work until there's no more left nextUnitOfWork = findNextUnitOfWork(); } - // Stop if the next unit of work is low priority - if (nextPriorityLevel > AnimationPriority) { - scheduleDeferredCallback(performDeferredWork); - return; - } + } + if (nextUnitOfWork && nextPriorityLevel > AnimationPriority) { + scheduleDeferredCallback(performDeferredWork); } } From a07f5eede0a3e0f083b37cd28610f67d2ff98f07 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 28 Oct 2016 11:12:10 -0700 Subject: [PATCH 02/14] Synchronous work --- .../shared/fiber/ReactFiberScheduler.js | 73 +++++++++++-------- 1 file changed, 44 insertions(+), 29 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 44af01cb34ae..433a36c7f950 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -373,20 +373,7 @@ module.exports = function(config : HostConfig) { } function performDeferredWork(deadline) { - try { - performDeferredWorkUnsafe(deadline); - } catch (error) { - const failedUnitOfWork = nextUnitOfWork; - // Reset because it points to the error boundary: - nextUnitOfWork = null; - if (!failedUnitOfWork) { - // We shouldn't end up here because nextUnitOfWork - // should always be set while work is being performed. - throw error; - } - const trappedError = trapError(failedUnitOfWork, error); - handleErrors([trappedError]); - } + performAndHandleErrors(performDeferredWorkUnsafe, deadline); } function scheduleDeferredWork(root : FiberRoot, priority : PriorityLevel) { @@ -439,20 +426,7 @@ module.exports = function(config : HostConfig) { } function performAnimationWork() { - try { - performAnimationWorkUnsafe(); - } catch (error) { - const failedUnitOfWork = nextUnitOfWork; - // Reset because it points to the error boundary: - nextUnitOfWork = null; - if (!failedUnitOfWork) { - // We shouldn't end up here because nextUnitOfWork - // should always be set while work is being performed. - throw error; - } - const trappedError = trapError(failedUnitOfWork, error); - handleErrors([trappedError]); - } + performAndHandleErrors(performAnimationWorkUnsafe); } function scheduleAnimationWork(root: FiberRoot, priorityLevel : PriorityLevel) { @@ -505,6 +479,46 @@ module.exports = function(config : HostConfig) { return root; } + function performSynchronousWorkUnsafe() { + // Perform work now + nextUnitOfWork = findNextUnitOfWork(); + while (nextUnitOfWork && + nextPriorityLevel === SynchronousPriority) { + nextUnitOfWork = performUnitOfWork(nextUnitOfWork, false); + // If there's no nextUnitForWork, we don't need to search for more + // because it shouldn't be possible to schedule sync work without + // immediately performing it + } + if (nextUnitOfWork) { + if (nextPriorityLevel > AnimationPriority) { + scheduleDeferredCallback(performDeferredWork); + return; + } + scheduleAnimationCallback(performAnimationWork); + } + } + + function performSynchronousWork() { + performAndHandleErrors(performSynchronousWorkUnsafe); + } + + function performAndHandleErrors(fn: (a: A) => void, a: A) { + try { + fn(a); + } catch (error) { + const failedUnitOfWork = nextUnitOfWork; + // Reset because it points to the error boundary: + nextUnitOfWork = null; + if (!failedUnitOfWork) { + // We shouldn't end up here because nextUnitOfWork + // should always be set while work is being performed. + throw error; + } + const trappedError = trapError(failedUnitOfWork, error); + handleErrors([trappedError]); + } + } + function handleErrors(initialTrappedErrors : Array) : void { let nextTrappedErrors = initialTrappedErrors; let firstUncaughtError = null; @@ -579,7 +593,8 @@ module.exports = function(config : HostConfig) { function scheduleWork(root : FiberRoot) { if (defaultPriority === SynchronousPriority) { - throw new Error('Not implemented yet'); + root.current.pendingWorkPriority = SynchronousPriority; + performSynchronousWork(); } if (defaultPriority === NoWork) { From 7b9ae92058150494d372cfe17a8c086a307ee677 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 28 Oct 2016 11:26:03 -0700 Subject: [PATCH 03/14] Updates should use the same priority context as top-level render A bit of restructuring so that setState uses whatever the current priority context is, like top-level render already does. Renamed defaultPriority to priorityContext, and added a new variable called defaultPriorityContext. Having two separate variables allows the default to be changed without interfering with the current context. --- .../shared/fiber/ReactFiberBeginWork.js | 2 +- .../shared/fiber/ReactFiberClassComponent.js | 13 +++-- .../shared/fiber/ReactFiberScheduler.js | 52 ++++++++++++++----- 3 files changed, 45 insertions(+), 22 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index bcc8958e5422..d8b41f77378c 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -48,7 +48,7 @@ var ReactFiberClassComponent = require('ReactFiberClassComponent'); module.exports = function( config : HostConfig, - scheduleUpdate : (fiber: Fiber, priorityLevel : PriorityLevel) => void + scheduleUpdate : (fiber: Fiber, priorityLevel : ?PriorityLevel) => void ) { const { diff --git a/src/renderers/shared/fiber/ReactFiberClassComponent.js b/src/renderers/shared/fiber/ReactFiberClassComponent.js index 2211dead1876..d775860d248c 100644 --- a/src/renderers/shared/fiber/ReactFiberClassComponent.js +++ b/src/renderers/shared/fiber/ReactFiberClassComponent.js @@ -16,7 +16,6 @@ import type { Fiber } from 'ReactFiber'; import type { PriorityLevel } from 'ReactPriorityLevel'; import type { UpdateQueue } from 'ReactFiberUpdateQueue'; -var { LowPriority } = require('ReactPriorityLevel'); var { createUpdateQueue, addToQueue, @@ -27,16 +26,16 @@ var { isMounted } = require('ReactFiberTreeReflection'); var ReactInstanceMap = require('ReactInstanceMap'); var shallowEqual = require('shallowEqual'); -module.exports = function(scheduleUpdate : (fiber: Fiber, priorityLevel : PriorityLevel) => void) { +module.exports = function(scheduleUpdate : (fiber: Fiber, priorityLevel : ?PriorityLevel) => void) { - function scheduleUpdateQueue(fiber: Fiber, updateQueue: UpdateQueue, priorityLevel : PriorityLevel) { + function scheduleUpdateQueue(fiber: Fiber, updateQueue: UpdateQueue) { fiber.updateQueue = updateQueue; // Schedule update on the alternate as well, since we don't know which tree // is current. if (fiber.alternate) { fiber.alternate.updateQueue = updateQueue; } - scheduleUpdate(fiber, priorityLevel); + scheduleUpdate(fiber); } // Class component state updater @@ -47,19 +46,19 @@ module.exports = function(scheduleUpdate : (fiber: Fiber, priorityLevel : Priori const updateQueue = fiber.updateQueue ? addToQueue(fiber.updateQueue, partialState) : createUpdateQueue(partialState); - scheduleUpdateQueue(fiber, updateQueue, LowPriority); + scheduleUpdateQueue(fiber, updateQueue); }, enqueueReplaceState(instance, state) { const fiber = ReactInstanceMap.get(instance); const updateQueue = createUpdateQueue(state); updateQueue.isReplace = true; - scheduleUpdateQueue(fiber, updateQueue, LowPriority); + scheduleUpdateQueue(fiber, updateQueue); }, enqueueForceUpdate(instance) { const fiber = ReactInstanceMap.get(instance); const updateQueue = fiber.updateQueue || createUpdateQueue(null); updateQueue.isForced = true; - scheduleUpdateQueue(fiber, updateQueue, LowPriority); + scheduleUpdateQueue(fiber, updateQueue); }, enqueueCallback(instance, callback) { const fiber = ReactInstanceMap.get(instance); diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 433a36c7f950..ab83161fde64 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -63,8 +63,10 @@ module.exports = function(config : HostConfig) { const scheduleAnimationCallback = config.scheduleAnimationCallback; const scheduleDeferredCallback = config.scheduleDeferredCallback; - // The default priority to use for updates. - let defaultPriority : PriorityLevel = LowPriority; + // The priority level to use when scheduling an update. + let priorityContext : (PriorityLevel | null) = null; + // The priority level to use if there is no priority context. + let defaultPriorityContext : PriorityLevel = LowPriority; // The next work in progress fiber that we're currently working on. let nextUnitOfWork : ?Fiber = null; @@ -485,7 +487,7 @@ module.exports = function(config : HostConfig) { while (nextUnitOfWork && nextPriorityLevel === SynchronousPriority) { nextUnitOfWork = performUnitOfWork(nextUnitOfWork, false); - // If there's no nextUnitForWork, we don't need to search for more + // If there's no nextUnitOfWork, we don't need to search for more // because it shouldn't be possible to schedule sync work without // immediately performing it } @@ -591,23 +593,45 @@ module.exports = function(config : HostConfig) { } } - function scheduleWork(root : FiberRoot) { - if (defaultPriority === SynchronousPriority) { + function scheduleWork(root : FiberRoot, priorityLevel : ?PriorityLevel) { + if (priorityLevel == null) { + priorityLevel = priorityContext !== null ? + priorityContext : + defaultPriorityContext; + } + + if (priorityLevel === SynchronousPriority) { root.current.pendingWorkPriority = SynchronousPriority; performSynchronousWork(); } - if (defaultPriority === NoWork) { + if (priorityLevel === NoWork) { return; } - if (defaultPriority > AnimationPriority) { - scheduleDeferredWork(root, defaultPriority); + if (priorityLevel > AnimationPriority) { + scheduleDeferredWork(root, priorityLevel); return; } - scheduleAnimationWork(root, defaultPriority); + scheduleAnimationWork(root, priorityLevel); } - function scheduleUpdate(fiber: Fiber, priorityLevel : PriorityLevel): void { + function scheduleUpdate(fiber: Fiber, priorityLevel : ?PriorityLevel): void { + // Use priority context if no priority is provided + if (priorityLevel == null) { + priorityLevel = priorityContext !== null ? + priorityContext : + defaultPriorityContext; + } + + // Don't bother bubbling the priority to the root if it is synchronous. Just + // perform it now. + if (priorityLevel === SynchronousPriority) { + fiber.pendingWorkPriority = SynchronousPriority; + nextUnitOfWork = fiber; + performSynchronousWork(); + return; + } + while (true) { if (fiber.pendingWorkPriority === NoWork || fiber.pendingWorkPriority >= priorityLevel) { @@ -622,7 +646,7 @@ module.exports = function(config : HostConfig) { if (!fiber.return) { if (fiber.tag === HostContainer) { const root : FiberRoot = (fiber.stateNode : any); - scheduleDeferredWork(root, priorityLevel); + scheduleWork(root, priorityLevel); return; } else { throw new Error('Invalid root'); @@ -633,12 +657,12 @@ module.exports = function(config : HostConfig) { } function performWithPriority(priorityLevel : PriorityLevel, fn : Function) { - const previousDefaultPriority = defaultPriority; - defaultPriority = priorityLevel; + const previousPriorityContext = priorityContext; + priorityContext = priorityLevel; try { fn(); } finally { - defaultPriority = previousDefaultPriority; + priorityContext = previousPriorityContext; } } From 78aef38bfa6865b409b30ce143c50635f93b3cc3 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 28 Oct 2016 11:44:52 -0700 Subject: [PATCH 04/14] Add config to enable sync scheduling by default I'll turn this on in ReactDOMFiber once I figure out batchedUpdates. --- src/renderers/shared/fiber/ReactFiberReconciler.js | 3 ++- src/renderers/shared/fiber/ReactFiberScheduler.js | 5 ++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js index d5995a3c65b0..d86c4946d4ed 100644 --- a/src/renderers/shared/fiber/ReactFiberReconciler.js +++ b/src/renderers/shared/fiber/ReactFiberReconciler.js @@ -56,8 +56,9 @@ export type HostConfig = { removeChild(parentInstance : I, child : I | TI) : void, scheduleAnimationCallback(callback : () => void) : void, - scheduleDeferredCallback(callback : (deadline : Deadline) => void) : void + scheduleDeferredCallback(callback : (deadline : Deadline) => void) : void, + useSyncScheduling ?: boolean, }; type OpaqueNode = Fiber; diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index ab83161fde64..139fe9d8bfd0 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -62,11 +62,14 @@ module.exports = function(config : HostConfig) { const scheduleAnimationCallback = config.scheduleAnimationCallback; const scheduleDeferredCallback = config.scheduleDeferredCallback; + const useSyncScheduling = config.useSyncScheduling; // The priority level to use when scheduling an update. let priorityContext : (PriorityLevel | null) = null; // The priority level to use if there is no priority context. - let defaultPriorityContext : PriorityLevel = LowPriority; + let defaultPriorityContext : PriorityLevel = useSyncScheduling ? + SynchronousPriority : + LowPriority; // The next work in progress fiber that we're currently working on. let nextUnitOfWork : ?Fiber = null; From ecf468e5567586192d61b0e426398878cb5cefcb Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 28 Oct 2016 13:26:25 -0700 Subject: [PATCH 05/14] Batch nested updates when in sync mode Almost working... --- src/renderers/dom/fiber/ReactDOMFiber.js | 2 + .../shared/fiber/ReactFiberClassComponent.js | 6 +- .../shared/fiber/ReactFiberScheduler.js | 57 +++++++++++++------ .../shared/fiber/ReactFiberUpdateQueue.js | 4 +- 4 files changed, 46 insertions(+), 23 deletions(-) diff --git a/src/renderers/dom/fiber/ReactDOMFiber.js b/src/renderers/dom/fiber/ReactDOMFiber.js index 8294883db55b..ca3b1f703b30 100644 --- a/src/renderers/dom/fiber/ReactDOMFiber.js +++ b/src/renderers/dom/fiber/ReactDOMFiber.js @@ -112,6 +112,8 @@ var DOMRenderer = ReactFiberReconciler({ scheduleDeferredCallback: window.requestIdleCallback, + useSyncScheduling: true, + }); var warned = false; diff --git a/src/renderers/shared/fiber/ReactFiberClassComponent.js b/src/renderers/shared/fiber/ReactFiberClassComponent.js index d775860d248c..7652ce146494 100644 --- a/src/renderers/shared/fiber/ReactFiberClassComponent.js +++ b/src/renderers/shared/fiber/ReactFiberClassComponent.js @@ -130,7 +130,7 @@ module.exports = function(scheduleUpdate : (fiber: Fiber, priorityLevel : ?Prior // process them now. const updateQueue = workInProgress.updateQueue; if (updateQueue) { - instance.state = mergeUpdateQueue(updateQueue, state, props); + instance.state = mergeUpdateQueue(updateQueue, instance, state, props); } } } @@ -174,7 +174,7 @@ module.exports = function(scheduleUpdate : (fiber: Fiber, priorityLevel : ?Prior // process them now. const newUpdateQueue = workInProgress.updateQueue; if (newUpdateQueue) { - newInstance.state = mergeUpdateQueue(newUpdateQueue, newState, newProps); + newInstance.state = mergeUpdateQueue(newUpdateQueue, newInstance, newState, newProps); } } return true; @@ -211,7 +211,7 @@ module.exports = function(scheduleUpdate : (fiber: Fiber, priorityLevel : ?Prior // TODO: Previous state can be null. let newState; if (updateQueue) { - newState = mergeUpdateQueue(updateQueue, previousState, newProps); + newState = mergeUpdateQueue(updateQueue, instance, previousState, newProps); } else { newState = previousState; } diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 139fe9d8bfd0..f93a849bc61c 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -71,6 +71,9 @@ module.exports = function(config : HostConfig) { SynchronousPriority : LowPriority; + // Whether updates should be batched. Only applies when using sync scheduling. + let shouldBatchUpdates : boolean = false; + // The next work in progress fiber that we're currently working on. let nextUnitOfWork : ?Fiber = null; let nextPriorityLevel : PriorityLevel = NoWork; @@ -490,9 +493,10 @@ module.exports = function(config : HostConfig) { while (nextUnitOfWork && nextPriorityLevel === SynchronousPriority) { nextUnitOfWork = performUnitOfWork(nextUnitOfWork, false); - // If there's no nextUnitOfWork, we don't need to search for more - // because it shouldn't be possible to schedule sync work without - // immediately performing it + + if (!nextUnitOfWork) { + nextUnitOfWork = findNextUnitOfWork(); + } } if (nextUnitOfWork) { if (nextPriorityLevel > AnimationPriority) { @@ -504,7 +508,35 @@ module.exports = function(config : HostConfig) { } function performSynchronousWork() { + if (useSyncScheduling) { + // Start batching updates + shouldBatchUpdates = true; + } performAndHandleErrors(performSynchronousWorkUnsafe); + shouldBatchUpdates = false; + } + + function scheduleSynchronousWork(root : FiberRoot) { + root.current.pendingWorkPriority = SynchronousPriority; + + if (root.isScheduled) { + // If we're already scheduled, we can bail out. + return; + } + root.isScheduled = true; + if (lastScheduledRoot) { + // Schedule ourselves to the end. + lastScheduledRoot.nextScheduledRoot = root; + lastScheduledRoot = root; + } else { + // We're the only work scheduled. + nextScheduledRoot = root; + lastScheduledRoot = root; + + if (!shouldBatchUpdates) { + performSynchronousWork(); + } + } } function performAndHandleErrors(fn: (a: A) => void, a: A) { @@ -560,10 +592,9 @@ module.exports = function(config : HostConfig) { // We will process an update caused by each error boundary synchronously. affectedBoundaries.forEach(boundary => { - // FIXME: We only specify LowPriority here so that setState() calls from the error - // boundaries are respected. Instead we should set default priority level or something - // like this. Reconsider this piece when synchronous scheduling is in place. - const priority = LowPriority; + const priority = priorityContext !== null ? + priorityContext : + defaultPriorityContext; const root = scheduleErrorBoundaryWork(boundary, priority); // This should use findNextUnitOfWork() when synchronous scheduling is implemented. let fiber = cloneFiber(root.current, priority); @@ -604,8 +635,7 @@ module.exports = function(config : HostConfig) { } if (priorityLevel === SynchronousPriority) { - root.current.pendingWorkPriority = SynchronousPriority; - performSynchronousWork(); + scheduleSynchronousWork(root); } if (priorityLevel === NoWork) { @@ -626,15 +656,6 @@ module.exports = function(config : HostConfig) { defaultPriorityContext; } - // Don't bother bubbling the priority to the root if it is synchronous. Just - // perform it now. - if (priorityLevel === SynchronousPriority) { - fiber.pendingWorkPriority = SynchronousPriority; - nextUnitOfWork = fiber; - performSynchronousWork(); - return; - } - while (true) { if (fiber.pendingWorkPriority === NoWork || fiber.pendingWorkPriority >= priorityLevel) { diff --git a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js index d352dd82c5f6..8e27e739811d 100644 --- a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js +++ b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js @@ -73,14 +73,14 @@ exports.callCallbacks = function(queue : UpdateQueue, context : any) { } }; -exports.mergeUpdateQueue = function(queue : UpdateQueue, prevState : any, props : any) : any { +exports.mergeUpdateQueue = function(queue : UpdateQueue, instance : any, prevState : any, props : any) : any { let node : ?UpdateQueueNode = queue; let state = queue.isReplace ? null : Object.assign({}, prevState); while (node) { let partialState; if (typeof node.partialState === 'function') { const updateFn = node.partialState; - partialState = updateFn(state, props); + partialState = updateFn.call(instance, state, props); } else { partialState = node.partialState; } From d153b1e27b132c51fb33464fa1208a91a58ca1fe Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 28 Oct 2016 22:53:37 -0700 Subject: [PATCH 06/14] Enqueue update and callback simultaneously Without this fix, in non-batched mode, the update is scheduled first and synchronously flushed before the callback is added to the queue. The callback isn't called until the next flush. --- src/isomorphic/classic/class/ReactClass.js | 5 ++ src/isomorphic/modern/class/ReactComponent.js | 11 ++++ .../shared/fiber/ReactFiberClassComponent.js | 27 +++++---- .../ReactCompositeComponentState-test.js | 57 ++++++++++++++----- 4 files changed, 73 insertions(+), 27 deletions(-) diff --git a/src/isomorphic/classic/class/ReactClass.js b/src/isomorphic/classic/class/ReactClass.js index 475f08d7d81b..f0a59650062b 100644 --- a/src/isomorphic/classic/class/ReactClass.js +++ b/src/isomorphic/classic/class/ReactClass.js @@ -728,6 +728,11 @@ var ReactClassMixin = { * type signature and the only use case for this, is to avoid that. */ replaceState: function(newState, callback) { + if (this.updater.isFiberUpdater) { + this.updater.enqueueReplaceState(this, newState, callback); + return; + } + this.updater.enqueueReplaceState(this, newState); if (callback) { this.updater.enqueueCallback(this, callback, 'replaceState'); diff --git a/src/isomorphic/modern/class/ReactComponent.js b/src/isomorphic/modern/class/ReactComponent.js index b93da63e94cf..1eac5e786dee 100644 --- a/src/isomorphic/modern/class/ReactComponent.js +++ b/src/isomorphic/modern/class/ReactComponent.js @@ -65,6 +65,12 @@ ReactComponent.prototype.setState = function(partialState, callback) { 'setState(...): takes an object of state variables to update or a ' + 'function which returns an object of state variables.' ); + + if (this.updater.isFiberUpdater) { + this.updater.enqueueSetState(this, partialState, callback); + return; + } + this.updater.enqueueSetState(this, partialState); if (callback) { this.updater.enqueueCallback(this, callback, 'setState'); @@ -86,6 +92,11 @@ ReactComponent.prototype.setState = function(partialState, callback) { * @protected */ ReactComponent.prototype.forceUpdate = function(callback) { + if (this.updater.isFiberUpdater) { + this.updater.enqueueForceUpdate(this, callback); + return; + } + this.updater.enqueueForceUpdate(this); if (callback) { this.updater.enqueueCallback(this, callback, 'forceUpdate'); diff --git a/src/renderers/shared/fiber/ReactFiberClassComponent.js b/src/renderers/shared/fiber/ReactFiberClassComponent.js index 7652ce146494..ac07fc852cdf 100644 --- a/src/renderers/shared/fiber/ReactFiberClassComponent.js +++ b/src/renderers/shared/fiber/ReactFiberClassComponent.js @@ -41,36 +41,35 @@ module.exports = function(scheduleUpdate : (fiber: Fiber, priorityLevel : ?Prior // Class component state updater const updater = { isMounted, - enqueueSetState(instance, partialState) { + enqueueSetState(instance, partialState, callback) { const fiber = ReactInstanceMap.get(instance); const updateQueue = fiber.updateQueue ? addToQueue(fiber.updateQueue, partialState) : createUpdateQueue(partialState); + if (callback) { + addCallbackToQueue(updateQueue, callback); + } scheduleUpdateQueue(fiber, updateQueue); }, - enqueueReplaceState(instance, state) { + enqueueReplaceState(instance, state, callback) { const fiber = ReactInstanceMap.get(instance); const updateQueue = createUpdateQueue(state); updateQueue.isReplace = true; + if (callback) { + addCallbackToQueue(updateQueue, callback); + } scheduleUpdateQueue(fiber, updateQueue); }, - enqueueForceUpdate(instance) { + enqueueForceUpdate(instance, callback) { const fiber = ReactInstanceMap.get(instance); const updateQueue = fiber.updateQueue || createUpdateQueue(null); updateQueue.isForced = true; - scheduleUpdateQueue(fiber, updateQueue); - }, - enqueueCallback(instance, callback) { - const fiber = ReactInstanceMap.get(instance); - let updateQueue = fiber.updateQueue ? - fiber.updateQueue : - createUpdateQueue(null); - addCallbackToQueue(updateQueue, callback); - fiber.updateQueue = updateQueue; - if (fiber.alternate) { - fiber.alternate.updateQueue = updateQueue; + if (callback) { + addCallbackToQueue(updateQueue, callback); } + scheduleUpdateQueue(fiber, updateQueue); }, + isFiberUpdater: true, }; function checkShouldComponentUpdate(workInProgress, oldProps, newProps, newState) { diff --git a/src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponentState-test.js b/src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponentState-test.js index c758aa5d2e66..ac55f596fc76 100644 --- a/src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponentState-test.js +++ b/src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponentState-test.js @@ -11,6 +11,8 @@ 'use strict'; +var ReactDOMFeatureFlags = require('ReactDOMFeatureFlags'); + var React; var ReactDOM; @@ -148,7 +150,7 @@ describe('ReactCompositeComponent-state', () => { ReactDOM.unmountComponentAtNode(container); - expect(stateListener.mock.calls.join('\n')).toEqual([ + let expected = [ // there is no state when getInitialState() is called ['getInitialState', null], ['componentWillMount-start', 'red'], @@ -167,17 +169,44 @@ describe('ReactCompositeComponent-state', () => { // componentDidMount() called setState({color:'yellow'}), which is async. // The update doesn't happen until the next flush. ['componentDidMount-end', 'orange'], - ['shouldComponentUpdate-currentState', 'orange'], - ['shouldComponentUpdate-nextState', 'yellow'], - ['componentWillUpdate-currentState', 'orange'], - ['componentWillUpdate-nextState', 'yellow'], - ['render', 'yellow'], - ['componentDidUpdate-currentState', 'yellow'], - ['componentDidUpdate-prevState', 'orange'], - ['setState-sunrise', 'yellow'], - ['setState-orange', 'yellow'], - ['setState-yellow', 'yellow'], - ['initial-callback', 'yellow'], + ]; + + if (ReactDOMFeatureFlags.useFiber) { + // The setState callbacks in componentWillMount, and the initial callback + // passed to ReactDOM.render, should be flushed right after component + // did mount: + expected.push( + ['setState-sunrise', 'orange'], // 1 + ['setState-orange', 'orange'], // 2 + ['initial-callback', 'orange'], // 3 + ['shouldComponentUpdate-currentState', 'orange'], + ['shouldComponentUpdate-nextState', 'yellow'], + ['componentWillUpdate-currentState', 'orange'], + ['componentWillUpdate-nextState', 'yellow'], + ['render', 'yellow'], + ['componentDidUpdate-currentState', 'yellow'], + ['componentDidUpdate-prevState', 'orange'], + ['setState-yellow', 'yellow'], + ); + } else { + // There is a bug in the stack reconciler where those callbacks are + // enqueued, but aren't called until the next flush. + expected.push( + ['shouldComponentUpdate-currentState', 'orange'], + ['shouldComponentUpdate-nextState', 'yellow'], + ['componentWillUpdate-currentState', 'orange'], + ['componentWillUpdate-nextState', 'yellow'], + ['render', 'yellow'], + ['componentDidUpdate-currentState', 'yellow'], + ['componentDidUpdate-prevState', 'orange'], + ['setState-sunrise', 'yellow'], // 1 + ['setState-orange', 'yellow'], // 2 + ['setState-yellow', 'yellow'], + ['initial-callback', 'yellow'] // 3 + ); + } + + expected.push( ['componentWillReceiveProps-start', 'yellow'], // setState({color:'green'}) only enqueues a pending state. ['componentWillReceiveProps-end', 'yellow'], @@ -213,7 +242,9 @@ describe('ReactCompositeComponent-state', () => { // unmountComponent() // state is available within `componentWillUnmount()` ['componentWillUnmount', 'blue'], - ].join('\n')); + ); + + expect(stateListener.mock.calls.join('\n')).toEqual(expected.join('\n')); }); it('should batch unmounts', () => { From b41883fc708cd24d77dcaa767cde814b50b457fe Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Sun, 30 Oct 2016 22:26:57 -0700 Subject: [PATCH 07/14] unstable_batchedUpdates Implements batchedUpdates and exposes as unstable_batchedUpdates. All nested synchronous updates are automatically batched. --- src/renderers/dom/fiber/ReactDOMFiber.js | 4 ++++ .../shared/fiber/ReactFiberReconciler.js | 9 +++++++- .../shared/fiber/ReactFiberScheduler.js | 22 ++++++++++++++----- 3 files changed, 28 insertions(+), 7 deletions(-) diff --git a/src/renderers/dom/fiber/ReactDOMFiber.js b/src/renderers/dom/fiber/ReactDOMFiber.js index ca3b1f703b30..6d59e5aed782 100644 --- a/src/renderers/dom/fiber/ReactDOMFiber.js +++ b/src/renderers/dom/fiber/ReactDOMFiber.js @@ -165,6 +165,10 @@ var ReactDOM = { return DOMRenderer.findHostInstance(component); }, + unstable_batchedUpdates(fn : Function) : void { + DOMRenderer.batchedUpdates(fn); + }, + }; module.exports = ReactDOM; diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js index d86c4946d4ed..f14559dfa806 100644 --- a/src/renderers/shared/fiber/ReactFiberReconciler.js +++ b/src/renderers/shared/fiber/ReactFiberReconciler.js @@ -68,6 +68,7 @@ export type Reconciler = { updateContainer(element : ReactElement, container : OpaqueNode) : void, unmountContainer(container : OpaqueNode) : void, performWithPriority(priorityLevel : PriorityLevel, fn : Function) : void, + batchedUpdates(fn: Function) : void, // Used to extract the return value from the initial render. Legacy API. getPublicRootInstance(container : OpaqueNode) : (ReactComponent | TI | I | null), @@ -78,7 +79,11 @@ export type Reconciler = { module.exports = function(config : HostConfig) : Reconciler { - var { scheduleWork, performWithPriority } = ReactFiberScheduler(config); + var { + scheduleWork, + performWithPriority, + batchedUpdates, + } = ReactFiberScheduler(config); return { @@ -142,6 +147,8 @@ module.exports = function(config : HostConfig) : performWithPriority, + batchedUpdates, + getPublicRootInstance(container : OpaqueNode) : (ReactComponent | I | TI | null) { const root : FiberRoot = (container.stateNode : any); const containerFiber = root.current; diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index f93a849bc61c..802978879449 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -508,12 +508,10 @@ module.exports = function(config : HostConfig) { } function performSynchronousWork() { - if (useSyncScheduling) { - // Start batching updates - shouldBatchUpdates = true; - } - performAndHandleErrors(performSynchronousWorkUnsafe); - shouldBatchUpdates = false; + // All nested updates are batched + batchedUpdates(() => { + performAndHandleErrors(performSynchronousWorkUnsafe); + }); } function scheduleSynchronousWork(root : FiberRoot) { @@ -534,6 +532,7 @@ module.exports = function(config : HostConfig) { lastScheduledRoot = root; if (!shouldBatchUpdates) { + // Unless in batched mode, perform work immediately performSynchronousWork(); } } @@ -690,9 +689,20 @@ module.exports = function(config : HostConfig) { } } + function batchedUpdates(fn: Function) { + const prev = shouldBatchUpdates; + shouldBatchUpdates = true; + try { + fn(); + } finally { + shouldBatchUpdates = prev; + } + } + return { scheduleWork: scheduleWork, scheduleDeferredWork: scheduleDeferredWork, performWithPriority: performWithPriority, + batchedUpdates: batchedUpdates, }; }; From 2e5e88c750568b10dc1eebd7419a915750428097 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Sun, 30 Oct 2016 22:34:55 -0700 Subject: [PATCH 08/14] Don't need defaultPriorityContext Turns out this isn't necessary. Simpler to keep it as one field. --- .../shared/fiber/ReactFiberScheduler.js | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 802978879449..dbd6d3e7ce12 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -65,9 +65,7 @@ module.exports = function(config : HostConfig) { const useSyncScheduling = config.useSyncScheduling; // The priority level to use when scheduling an update. - let priorityContext : (PriorityLevel | null) = null; - // The priority level to use if there is no priority context. - let defaultPriorityContext : PriorityLevel = useSyncScheduling ? + let priorityContext : PriorityLevel = useSyncScheduling ? SynchronousPriority : LowPriority; @@ -591,9 +589,7 @@ module.exports = function(config : HostConfig) { // We will process an update caused by each error boundary synchronously. affectedBoundaries.forEach(boundary => { - const priority = priorityContext !== null ? - priorityContext : - defaultPriorityContext; + const priority = priorityContext; const root = scheduleErrorBoundaryWork(boundary, priority); // This should use findNextUnitOfWork() when synchronous scheduling is implemented. let fiber = cloneFiber(root.current, priority); @@ -628,9 +624,7 @@ module.exports = function(config : HostConfig) { function scheduleWork(root : FiberRoot, priorityLevel : ?PriorityLevel) { if (priorityLevel == null) { - priorityLevel = priorityContext !== null ? - priorityContext : - defaultPriorityContext; + priorityLevel = priorityContext; } if (priorityLevel === SynchronousPriority) { @@ -650,9 +644,7 @@ module.exports = function(config : HostConfig) { function scheduleUpdate(fiber: Fiber, priorityLevel : ?PriorityLevel): void { // Use priority context if no priority is provided if (priorityLevel == null) { - priorityLevel = priorityContext !== null ? - priorityContext : - defaultPriorityContext; + priorityLevel = priorityContext; } while (true) { From 26731ea9671837899de80d7e71db0df93d5ca64f Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Sun, 30 Oct 2016 22:44:52 -0700 Subject: [PATCH 09/14] unstable_batchedUpdates should return value of callback --- src/renderers/dom/fiber/ReactDOMFiber.js | 4 ++-- src/renderers/shared/fiber/ReactFiberReconciler.js | 5 ++++- src/renderers/shared/fiber/ReactFiberScheduler.js | 4 ++-- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/renderers/dom/fiber/ReactDOMFiber.js b/src/renderers/dom/fiber/ReactDOMFiber.js index 6d59e5aed782..08fd6d7c59df 100644 --- a/src/renderers/dom/fiber/ReactDOMFiber.js +++ b/src/renderers/dom/fiber/ReactDOMFiber.js @@ -165,8 +165,8 @@ var ReactDOM = { return DOMRenderer.findHostInstance(component); }, - unstable_batchedUpdates(fn : Function) : void { - DOMRenderer.batchedUpdates(fn); + unstable_batchedUpdates(fn : () => A) : A { + return DOMRenderer.batchedUpdates(fn); }, }; diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js index f14559dfa806..7708e04c590c 100644 --- a/src/renderers/shared/fiber/ReactFiberReconciler.js +++ b/src/renderers/shared/fiber/ReactFiberReconciler.js @@ -68,7 +68,10 @@ export type Reconciler = { updateContainer(element : ReactElement, container : OpaqueNode) : void, unmountContainer(container : OpaqueNode) : void, performWithPriority(priorityLevel : PriorityLevel, fn : Function) : void, - batchedUpdates(fn: Function) : void, + /* eslint-disable no-undef */ + // FIXME: ESLint complains about type parameter + batchedUpdates(fn : () => A) : A, + /* eslint-enable no-undef */ // Used to extract the return value from the initial render. Legacy API. getPublicRootInstance(container : OpaqueNode) : (ReactComponent | TI | I | null), diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index dbd6d3e7ce12..ca942f62695a 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -681,11 +681,11 @@ module.exports = function(config : HostConfig) { } } - function batchedUpdates(fn: Function) { + function batchedUpdates(fn : () => A) : A { const prev = shouldBatchUpdates; shouldBatchUpdates = true; try { - fn(); + return fn(); } finally { shouldBatchUpdates = prev; } From 3121e051a849b1d1e497de3362b3d4bc317b902d Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 31 Oct 2016 09:59:48 -0700 Subject: [PATCH 10/14] At the end of a batch, perform any sync work that was scheduled --- src/renderers/shared/fiber/ReactFiberScheduler.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index ca942f62695a..957d275b0d1b 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -506,10 +506,14 @@ module.exports = function(config : HostConfig) { } function performSynchronousWork() { + const prev = shouldBatchUpdates; + shouldBatchUpdates = true; // All nested updates are batched - batchedUpdates(() => { + try { performAndHandleErrors(performSynchronousWorkUnsafe); - }); + } finally { + shouldBatchUpdates = prev; + } } function scheduleSynchronousWork(root : FiberRoot) { @@ -688,6 +692,10 @@ module.exports = function(config : HostConfig) { return fn(); } finally { shouldBatchUpdates = prev; + // If we've exited the batch, perform any scheduled sync work + if (!shouldBatchUpdates) { + performSynchronousWork(); + } } } From da45010cb7083cac22f54fcb4981039186e3f70b Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 31 Oct 2016 11:18:22 -0700 Subject: [PATCH 11/14] Remove first-class function Instead we'll branch on the priority level, like in scheduleWork. --- .../shared/fiber/ReactFiberReconciler.js | 2 +- .../shared/fiber/ReactFiberScheduler.js | 25 ++++++++++++++----- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js index 7708e04c590c..fd7fc1b58622 100644 --- a/src/renderers/shared/fiber/ReactFiberReconciler.js +++ b/src/renderers/shared/fiber/ReactFiberReconciler.js @@ -28,7 +28,7 @@ if (__DEV__) { var { findCurrentHostFiber } = require('ReactFiberTreeReflection'); -type Deadline = { +export type Deadline = { timeRemaining : () => number }; diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 957d275b0d1b..cbe39539e572 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -15,7 +15,7 @@ import type { TrappedError } from 'ReactFiberErrorBoundary'; import type { Fiber } from 'ReactFiber'; import type { FiberRoot } from 'ReactFiberRoot'; -import type { HostConfig } from 'ReactFiberReconciler'; +import type { HostConfig, Deadline } from 'ReactFiberReconciler'; import type { PriorityLevel } from 'ReactPriorityLevel'; var ReactFiberBeginWork = require('ReactFiberBeginWork'); @@ -379,7 +379,7 @@ module.exports = function(config : HostConfig) { } function performDeferredWork(deadline) { - performAndHandleErrors(performDeferredWorkUnsafe, deadline); + performAndHandleErrors(LowPriority, deadline); } function scheduleDeferredWork(root : FiberRoot, priority : PriorityLevel) { @@ -432,7 +432,7 @@ module.exports = function(config : HostConfig) { } function performAnimationWork() { - performAndHandleErrors(performAnimationWorkUnsafe); + performAndHandleErrors(AnimationPriority); } function scheduleAnimationWork(root: FiberRoot, priorityLevel : PriorityLevel) { @@ -510,7 +510,7 @@ module.exports = function(config : HostConfig) { shouldBatchUpdates = true; // All nested updates are batched try { - performAndHandleErrors(performSynchronousWorkUnsafe); + performAndHandleErrors(SynchronousPriority); } finally { shouldBatchUpdates = prev; } @@ -540,9 +540,22 @@ module.exports = function(config : HostConfig) { } } - function performAndHandleErrors(fn: (a: A) => void, a: A) { + function performAndHandleErrors(priorityLevel : PriorityLevel, deadline : ?Deadline) { + // The exact priority level doesn't matter, so long as it's in range of the + // work (sync, animation, deferred) being performed. try { - fn(a); + if (priorityLevel === SynchronousPriority) { + performSynchronousWorkUnsafe(); + } + if (priorityLevel > AnimationPriority) { + if (!deadline) { + throw new Error('No deadline'); + } else { + performDeferredWorkUnsafe(deadline); + } + return; + } + performAnimationWorkUnsafe(); } catch (error) { const failedUnitOfWork = nextUnitOfWork; // Reset because it points to the error boundary: From 2182b690046a8794079fc3817223e98bf64072b8 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 31 Oct 2016 13:38:47 -0700 Subject: [PATCH 12/14] batchedUpdates tests Covers some bugs I encountered while working on this feature. Introduces a syncUpdates API to ReactNoop. Is this something we'd want to expose to the reconciler? --- src/renderers/noop/ReactNoop.js | 4 + .../shared/fiber/ReactFiberReconciler.js | 4 + .../shared/fiber/ReactFiberScheduler.js | 11 +++ .../fiber/__tests__/ReactIncremental-test.js | 79 +++++++++++++++++++ 4 files changed, 98 insertions(+) diff --git a/src/renderers/noop/ReactNoop.js b/src/renderers/noop/ReactNoop.js index ea0b4ec655f7..f57bf2b9c82f 100644 --- a/src/renderers/noop/ReactNoop.js +++ b/src/renderers/noop/ReactNoop.js @@ -257,6 +257,10 @@ var ReactNoop = { NoopRenderer.performWithPriority(AnimationPriority, fn); }, + batchedUpdates: NoopRenderer.batchedUpdates, + + syncUpdates: NoopRenderer.syncUpdates, + // Logs the current state of the tree. dumpTree(rootID : string = DEFAULT_ROOT_ID) { const root = roots.get(rootID); diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js index fd7fc1b58622..e85f25bc469d 100644 --- a/src/renderers/shared/fiber/ReactFiberReconciler.js +++ b/src/renderers/shared/fiber/ReactFiberReconciler.js @@ -71,6 +71,7 @@ export type Reconciler = { /* eslint-disable no-undef */ // FIXME: ESLint complains about type parameter batchedUpdates(fn : () => A) : A, + syncUpdates(fn : () => A) : A, /* eslint-enable no-undef */ // Used to extract the return value from the initial render. Legacy API. @@ -86,6 +87,7 @@ module.exports = function(config : HostConfig) : scheduleWork, performWithPriority, batchedUpdates, + syncUpdates, } = ReactFiberScheduler(config); return { @@ -152,6 +154,8 @@ module.exports = function(config : HostConfig) : batchedUpdates, + syncUpdates, + getPublicRootInstance(container : OpaqueNode) : (ReactComponent | I | TI | null) { const root : FiberRoot = (container.stateNode : any); const containerFiber = root.current; diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index cbe39539e572..5fc00776c780 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -712,10 +712,21 @@ module.exports = function(config : HostConfig) { } } + function syncUpdates(fn : () => A) : A { + const previousPriorityContext = priorityContext; + priorityContext = SynchronousPriority; + try { + return fn(); + } finally { + priorityContext = previousPriorityContext; + } + } + return { scheduleWork: scheduleWork, scheduleDeferredWork: scheduleDeferredWork, performWithPriority: performWithPriority, batchedUpdates: batchedUpdates, + syncUpdates: syncUpdates, }; }; diff --git a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js index ed775a969a59..5df5c10caf80 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js @@ -1386,4 +1386,83 @@ describe('ReactIncremental', () => { ]); }); + it('performs batched updates at the end of the batch', () => { + var ops = []; + var instance; + + class Foo extends React.Component { + state = { n: 0 }; + render() { + instance = this; + return
; + } + } + + ReactNoop.render(); + ReactNoop.flush(); + ops = []; + + ReactNoop.syncUpdates(() => { + ReactNoop.batchedUpdates(() => { + instance.setState({ n: 1 }, () => ops.push('setState 1')); + instance.setState({ n: 2 }, () => ops.push('setState 2')); + ops.push('end batchedUpdates'); + }); + ops.push('end syncUpdates'); + }); + + // ReactNoop.flush() not needed because updates are synchronous + + expect(ops).toEqual([ + 'end batchedUpdates', + 'setState 1', + 'setState 2', + 'end syncUpdates', + ]); + expect(instance.state.n).toEqual(2); + }); + + it('can nest batchedUpdates', () => { + var ops = []; + var instance; + + class Foo extends React.Component { + state = { n: 0 }; + render() { + instance = this; + return
; + } + } + + ReactNoop.render(); + ReactNoop.flush(); + ops = []; + + ReactNoop.syncUpdates(() => { + ReactNoop.batchedUpdates(() => { + instance.setState({ n: 1 }, () => ops.push('setState 1')); + instance.setState({ n: 2 }, () => ops.push('setState 2')); + ReactNoop.batchedUpdates(() => { + instance.setState({ n: 3 }, () => ops.push('setState 3')); + instance.setState({ n: 4 }, () => ops.push('setState 4')); + ops.push('end inner batchedUpdates'); + }); + ops.push('end outer batchedUpdates'); + }); + ops.push('end syncUpdates'); + }); + + // ReactNoop.flush() not needed because updates are synchronous + + expect(ops).toEqual([ + 'end inner batchedUpdates', + 'end outer batchedUpdates', + 'setState 1', + 'setState 2', + 'setState 3', + 'setState 4', + 'end syncUpdates', + ]); + expect(instance.state.n).toEqual(4); + }); }); From c3ab541e0f73056ca1b442876da5ede24197ed28 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Tue, 1 Nov 2016 17:41:36 -0700 Subject: [PATCH 13/14] New effect type: Callback This solves the problem I had with enqueueSetState and enqueueCallback being separate. --- scripts/fiber/tests-failing.txt | 6 +- scripts/fiber/tests-passing.txt | 7 +- .../__tests__/ReactTransitionGroup-test.js | 6 +- src/isomorphic/classic/class/ReactClass.js | 5 -- src/isomorphic/modern/class/ReactComponent.js | 11 --- .../shared/fiber/ReactFiberBeginWork.js | 2 +- .../shared/fiber/ReactFiberClassComponent.js | 38 ++++++---- .../shared/fiber/ReactFiberCommitWork.js | 55 +++++++------- .../shared/fiber/ReactFiberCompleteWork.js | 19 ++++- .../shared/fiber/ReactFiberScheduler.js | 73 ++++++++++--------- .../shared/fiber/ReactFiberUpdateQueue.js | 15 +++- .../shared/fiber/ReactTypeOfSideEffect.js | 17 +++-- .../ReactIncrementalSideEffects-test.js | 29 ++++++++ 13 files changed, 168 insertions(+), 115 deletions(-) diff --git a/scripts/fiber/tests-failing.txt b/scripts/fiber/tests-failing.txt index 8c9b7a49041c..f91c436b8d3d 100644 --- a/scripts/fiber/tests-failing.txt +++ b/scripts/fiber/tests-failing.txt @@ -87,6 +87,9 @@ src/renderers/art/__tests__/ReactART-test.js * resolves refs before componentDidMount * resolves refs before componentDidUpdate +src/renderers/dom/__tests__/ReactDOMProduction-test.js +* should throw with an error code in production + src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js * should set style attribute when styles exist * should warn when using hyphenated style names @@ -369,7 +372,6 @@ src/renderers/dom/stack/client/__tests__/ReactMount-test.js * should account for escaping on a checksum mismatch * should warn if render removes React-rendered children * should warn if the unmounted node was rendered by another copy of React -* passes the correct callback context * tracks root instances * marks top-level mounts @@ -608,8 +610,6 @@ src/renderers/shared/stack/reconciler/__tests__/ReactUpdates-test.js * throws in setState if the update callback is not a function * throws in replaceState if the update callback is not a function * throws in forceUpdate if the update callback is not a function -* does not update one component twice in a batch (#2410) -* unstable_batchedUpdates should return value from a callback src/renderers/shared/stack/reconciler/__tests__/refs-test.js * Should increase refs with an increase in divs diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 995e16d47ef1..3046357ff1c1 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -438,7 +438,6 @@ src/renderers/dom/__tests__/ReactDOMProduction-test.js * should use prod React * should handle a simple flow * should call lifecycle methods -* should throw with an error code in production src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js * should render strings as children @@ -692,6 +691,7 @@ src/renderers/dom/stack/client/__tests__/ReactMount-test.js * should warn if mounting into dirty rendered markup * should not warn if mounting into non-empty node * should warn when mounting into document.body +* passes the correct callback context src/renderers/dom/stack/client/__tests__/ReactMountDestruction-test.js * should destroy a react root upon request @@ -788,6 +788,8 @@ src/renderers/shared/fiber/__tests__/ReactIncremental-test.js * calls componentWill* twice if an update render is aborted * does not call componentWillReceiveProps for state-only updates * skips will/DidUpdate when bailing unless an update was already in progress +* performs batched updates at the end of the batch +* can nest batchedUpdates src/renderers/shared/fiber/__tests__/ReactIncrementalReflection-test.js * handles isMounted even when the initial render is deferred @@ -833,6 +835,7 @@ src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js * can defer side-effects and reuse them later - complex * deprioritizes setStates that happens within a deprioritized tree * calls callback after update is flushed +* calls setState callback even if component bails out * calls componentWillUnmount after a deletion, even if nested * calls componentDidMount/Update after insertion/update * invokes ref callbacks after insertion/update/unmount @@ -1124,7 +1127,9 @@ src/renderers/shared/stack/reconciler/__tests__/ReactUpdates-test.js * should not reconcile children passed via props * should queue nested updates * calls componentWillReceiveProps setState callback properly +* does not update one component twice in a batch (#2410) * does not update one component twice in a batch (#6371) +* unstable_batchedUpdates should return value from a callback src/renderers/shared/stack/reconciler/__tests__/Transaction-test.js * should invoke closers with/only-with init returns diff --git a/src/addons/transitions/__tests__/ReactTransitionGroup-test.js b/src/addons/transitions/__tests__/ReactTransitionGroup-test.js index 90c0e0190066..0729be98f46e 100644 --- a/src/addons/transitions/__tests__/ReactTransitionGroup-test.js +++ b/src/addons/transitions/__tests__/ReactTransitionGroup-test.js @@ -97,10 +97,10 @@ describe('ReactTransitionGroup', () => { expect(log).toEqual(['didMount', 'willEnter', 'didEnter']); log = []; - instance.setState({count: 1}, function() { - expect(log).toEqual(['willLeave', 'didLeave', 'willUnmount']); - }); + instance.setState({count: 1}); }); + + expect(log).toEqual(['willLeave', 'didLeave', 'willUnmount']); }); it('should handle enter/leave/enter/leave correctly', () => { diff --git a/src/isomorphic/classic/class/ReactClass.js b/src/isomorphic/classic/class/ReactClass.js index f0a59650062b..475f08d7d81b 100644 --- a/src/isomorphic/classic/class/ReactClass.js +++ b/src/isomorphic/classic/class/ReactClass.js @@ -728,11 +728,6 @@ var ReactClassMixin = { * type signature and the only use case for this, is to avoid that. */ replaceState: function(newState, callback) { - if (this.updater.isFiberUpdater) { - this.updater.enqueueReplaceState(this, newState, callback); - return; - } - this.updater.enqueueReplaceState(this, newState); if (callback) { this.updater.enqueueCallback(this, callback, 'replaceState'); diff --git a/src/isomorphic/modern/class/ReactComponent.js b/src/isomorphic/modern/class/ReactComponent.js index 1eac5e786dee..b93da63e94cf 100644 --- a/src/isomorphic/modern/class/ReactComponent.js +++ b/src/isomorphic/modern/class/ReactComponent.js @@ -65,12 +65,6 @@ ReactComponent.prototype.setState = function(partialState, callback) { 'setState(...): takes an object of state variables to update or a ' + 'function which returns an object of state variables.' ); - - if (this.updater.isFiberUpdater) { - this.updater.enqueueSetState(this, partialState, callback); - return; - } - this.updater.enqueueSetState(this, partialState); if (callback) { this.updater.enqueueCallback(this, callback, 'setState'); @@ -92,11 +86,6 @@ ReactComponent.prototype.setState = function(partialState, callback) { * @protected */ ReactComponent.prototype.forceUpdate = function(callback) { - if (this.updater.isFiberUpdater) { - this.updater.enqueueForceUpdate(this, callback); - return; - } - this.updater.enqueueForceUpdate(this); if (callback) { this.updater.enqueueCallback(this, callback, 'forceUpdate'); diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index d8b41f77378c..e1c2b1284b17 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -48,7 +48,7 @@ var ReactFiberClassComponent = require('ReactFiberClassComponent'); module.exports = function( config : HostConfig, - scheduleUpdate : (fiber: Fiber, priorityLevel : ?PriorityLevel) => void + scheduleUpdate : (fiber: Fiber) => void ) { const { diff --git a/src/renderers/shared/fiber/ReactFiberClassComponent.js b/src/renderers/shared/fiber/ReactFiberClassComponent.js index ac07fc852cdf..db71854a1213 100644 --- a/src/renderers/shared/fiber/ReactFiberClassComponent.js +++ b/src/renderers/shared/fiber/ReactFiberClassComponent.js @@ -26,7 +26,7 @@ var { isMounted } = require('ReactFiberTreeReflection'); var ReactInstanceMap = require('ReactInstanceMap'); var shallowEqual = require('shallowEqual'); -module.exports = function(scheduleUpdate : (fiber: Fiber, priorityLevel : ?PriorityLevel) => void) { +module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { function scheduleUpdateQueue(fiber: Fiber, updateQueue: UpdateQueue) { fiber.updateQueue = updateQueue; @@ -41,35 +41,33 @@ module.exports = function(scheduleUpdate : (fiber: Fiber, priorityLevel : ?Prior // Class component state updater const updater = { isMounted, - enqueueSetState(instance, partialState, callback) { + enqueueSetState(instance, partialState) { const fiber = ReactInstanceMap.get(instance); const updateQueue = fiber.updateQueue ? addToQueue(fiber.updateQueue, partialState) : createUpdateQueue(partialState); - if (callback) { - addCallbackToQueue(updateQueue, callback); - } scheduleUpdateQueue(fiber, updateQueue); }, - enqueueReplaceState(instance, state, callback) { + enqueueReplaceState(instance, state) { const fiber = ReactInstanceMap.get(instance); const updateQueue = createUpdateQueue(state); updateQueue.isReplace = true; - if (callback) { - addCallbackToQueue(updateQueue, callback); - } scheduleUpdateQueue(fiber, updateQueue); }, - enqueueForceUpdate(instance, callback) { + enqueueForceUpdate(instance) { const fiber = ReactInstanceMap.get(instance); const updateQueue = fiber.updateQueue || createUpdateQueue(null); updateQueue.isForced = true; - if (callback) { - addCallbackToQueue(updateQueue, callback); - } scheduleUpdateQueue(fiber, updateQueue); }, - isFiberUpdater: true, + enqueueCallback(instance, callback) { + const fiber = ReactInstanceMap.get(instance); + let updateQueue = fiber.updateQueue ? + fiber.updateQueue : + createUpdateQueue(null); + addCallbackToQueue(updateQueue, callback); + scheduleUpdateQueue(fiber, updateQueue); + }, }; function checkShouldComponentUpdate(workInProgress, oldProps, newProps, newState) { @@ -210,11 +208,21 @@ module.exports = function(scheduleUpdate : (fiber: Fiber, priorityLevel : ?Prior // TODO: Previous state can be null. let newState; if (updateQueue) { - newState = mergeUpdateQueue(updateQueue, instance, previousState, newProps); + if (!updateQueue.hasUpdate) { + newState = previousState; + } else { + newState = mergeUpdateQueue(updateQueue, instance, previousState, newProps); + } } else { newState = previousState; } + if (oldProps === newProps && + previousState === newState && + updateQueue && !updateQueue.isForced) { + return false; + } + if (!checkShouldComponentUpdate( workInProgress, oldProps, diff --git a/src/renderers/shared/fiber/ReactFiberCommitWork.js b/src/renderers/shared/fiber/ReactFiberCommitWork.js index db844453c8ef..b23e5bdd6f8d 100644 --- a/src/renderers/shared/fiber/ReactFiberCommitWork.js +++ b/src/renderers/shared/fiber/ReactFiberCommitWork.js @@ -29,7 +29,8 @@ var { callCallbacks } = require('ReactFiberUpdateQueue'); var { Placement, - PlacementAndUpdate, + Update, + Callback, } = require('ReactTypeOfSideEffect'); module.exports = function(config : HostConfig) { @@ -102,8 +103,7 @@ module.exports = function(config : HostConfig) { // If it is not host node and, we might have a host node inside it. // Try to search down until we find one. // TODO: For coroutines, this will have to search the stateNode. - if (node.effectTag === Placement || - node.effectTag === PlacementAndUpdate) { + if (node.effectTag & Placement) { // If we don't have a child, try the siblings instead. continue siblings; } @@ -114,8 +114,7 @@ module.exports = function(config : HostConfig) { } } // Check if this host node is stable or about to be placed. - if (node.effectTag !== Placement && - node.effectTag !== PlacementAndUpdate) { + if (!(node.effectTag & Placement)) { // Found it! return node.stateNode; } @@ -329,41 +328,41 @@ module.exports = function(config : HostConfig) { case ClassComponent: { const instance = finishedWork.stateNode; let error = null; - if (!current) { - if (typeof instance.componentDidMount === 'function') { - error = tryCallComponentDidMount(instance); - } - } else { - if (typeof instance.componentDidUpdate === 'function') { - const prevProps = current.memoizedProps; - const prevState = current.memoizedState; - error = tryCallComponentDidUpdate(instance, prevProps, prevState); + if (finishedWork.effectTag & Update) { + if (!current) { + if (typeof instance.componentDidMount === 'function') { + error = tryCallComponentDidMount(instance); + } + } else { + if (typeof instance.componentDidUpdate === 'function') { + const prevProps = current.memoizedProps; + const prevState = current.memoizedState; + error = tryCallComponentDidUpdate(instance, prevProps, prevState); + } } + attachRef(current, finishedWork, instance); } - // Clear updates from current fiber. This must go before the callbacks - // are reset, in case an update is triggered from inside a callback. Is - // this safe? Relies on the assumption that work is only committed if - // the update queue is empty. + // Clear updates from current fiber. if (finishedWork.alternate) { finishedWork.alternate.updateQueue = null; } - if (finishedWork.callbackList) { - const { callbackList } = finishedWork; - finishedWork.callbackList = null; - callCallbacks(callbackList, instance); + if (finishedWork.effectTag & Callback) { + if (finishedWork.callbackList) { + callCallbacks(finishedWork.callbackList, instance); + finishedWork.callbackList = null; + } } - attachRef(current, finishedWork, instance); if (error) { return trapError(finishedWork, error); } return null; } case HostContainer: { - const instance = finishedWork.stateNode; - if (instance.callbackList) { - const { callbackList } = instance; - instance.callbackList = null; - callCallbacks(callbackList, instance.current.child.stateNode); + const rootFiber = finishedWork.stateNode; + if (rootFiber.callbackList) { + const { callbackList } = rootFiber; + rootFiber.callbackList = null; + callCallbacks(callbackList, rootFiber.current.child.stateNode); } } case HostComponent: { diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index 590332d5ce19..b0eea7c87e25 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -34,6 +34,7 @@ var { } = ReactTypeOfWork; var { Update, + Callback, } = ReactTypeOfSideEffect; module.exports = function(config : HostConfig) { @@ -48,6 +49,11 @@ module.exports = function(config : HostConfig) { workInProgress.effectTag |= Update; } + function markCallback(workInProgress : Fiber) { + // Tag the fiber with a callback effect. + workInProgress.effectTag |= Callback; + } + function transferOutput(child : ?Fiber, returnFiber : Fiber) { // If we have a single result, we just pass that through as the output to // avoid unnecessary traversal. When we have multiple output, we just pass @@ -124,19 +130,24 @@ module.exports = function(config : HostConfig) { // Also need to transfer the props, because pendingProps will be null // in the case of an update const { state, props } = workInProgress.stateNode; + const updateQueue = workInProgress.updateQueue; workInProgress.memoizedState = state; workInProgress.memoizedProps = props; - // Transfer update queue to callbackList field so callbacks can be - // called during commit phase. - workInProgress.callbackList = workInProgress.updateQueue; if (current) { if (current.memoizedProps !== workInProgress.memoizedProps || - current.memoizedState !== workInProgress.memoizedState) { + current.memoizedState !== workInProgress.memoizedState || + updateQueue && updateQueue.isForced) { markUpdate(workInProgress); } } else { markUpdate(workInProgress); } + if (updateQueue && updateQueue.hasCallback) { + // Transfer update queue to callbackList field so callbacks can be + // called during commit phase. + workInProgress.callbackList = updateQueue; + markCallback(workInProgress); + } return null; case HostContainer: transferOutput(workInProgress.child, workInProgress); diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 5fc00776c780..fb4e5e885b12 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -39,6 +39,11 @@ var { Update, PlacementAndUpdate, Deletion, + Callback, + PlacementAndCallback, + UpdateAndCallback, + PlacementAndUpdateAndCallback, + DeletionAndCallback, } = require('ReactTypeOfSideEffect'); var { @@ -52,9 +57,6 @@ if (__DEV__) { var timeHeuristicForUnitOfWork = 1; module.exports = function(config : HostConfig) { - // Use a closure to circumvent the circular dependency between the scheduler - // and ReactFiberBeginWork. Don't know if there's a better way to do this. - const { beginWork } = ReactFiberBeginWork(config, scheduleUpdate); const { completeWork } = ReactFiberCompleteWork(config); const { commitInsertion, commitDeletion, commitWork, commitLifeCycles } = @@ -139,28 +141,34 @@ module.exports = function(config : HostConfig) { let effectfulFiber = finishedWork.firstEffect; while (effectfulFiber) { switch (effectfulFiber.effectTag) { - case Placement: { + case Placement: + case PlacementAndCallback: { commitInsertion(effectfulFiber); - // Clear the effect tag so that we know that this is inserted, before + // Clear the "placement" from effect tag so that we know that this is inserted, before // any life-cycles like componentDidMount gets called. effectfulFiber.effectTag = NoEffect; break; } - case PlacementAndUpdate: { + case PlacementAndUpdate: + case PlacementAndUpdateAndCallback: { + // Placement commitInsertion(effectfulFiber); - const current = effectfulFiber.alternate; - commitWork(current, effectfulFiber); // Clear the "placement" from effect tag so that we know that this is inserted, before // any life-cycles like componentDidMount gets called. effectfulFiber.effectTag = Update; + + // Update + const current = effectfulFiber.alternate; + commitWork(current, effectfulFiber); break; } - case Update: { + case Update: + case UpdateAndCallback: const current = effectfulFiber.alternate; commitWork(current, effectfulFiber); break; - } - case Deletion: { + case Deletion: + case DeletionAndCallback: // Deletion might cause an error in componentWillUnmount(). // We will continue nevertheless and handle those later on. const trappedErrors = commitDeletion(effectfulFiber); @@ -176,8 +184,8 @@ module.exports = function(config : HostConfig) { } } break; - } } + effectfulFiber = effectfulFiber.nextEffect; } @@ -186,8 +194,7 @@ module.exports = function(config : HostConfig) { // already been invoked. effectfulFiber = finishedWork.firstEffect; while (effectfulFiber) { - if (effectfulFiber.effectTag === Update || - effectfulFiber.effectTag === PlacementAndUpdate) { + if (effectfulFiber.effectTag & (Update | Callback)) { const current = effectfulFiber.alternate; const trappedError = commitLifeCycles(current, effectfulFiber); if (trappedError) { @@ -540,22 +547,22 @@ module.exports = function(config : HostConfig) { } } - function performAndHandleErrors(priorityLevel : PriorityLevel, deadline : ?Deadline) { + function performAndHandleErrors(priorityLevel : PriorityLevel, deadline : null | Deadline) { // The exact priority level doesn't matter, so long as it's in range of the // work (sync, animation, deferred) being performed. try { if (priorityLevel === SynchronousPriority) { performSynchronousWorkUnsafe(); - } - if (priorityLevel > AnimationPriority) { + } else if (priorityLevel > AnimationPriority) { if (!deadline) { throw new Error('No deadline'); } else { performDeferredWorkUnsafe(deadline); } return; + } else { + performAnimationWorkUnsafe(); } - performAnimationWorkUnsafe(); } catch (error) { const failedUnitOfWork = nextUnitOfWork; // Reset because it points to the error boundary: @@ -639,31 +646,25 @@ module.exports = function(config : HostConfig) { } } - function scheduleWork(root : FiberRoot, priorityLevel : ?PriorityLevel) { - if (priorityLevel == null) { - priorityLevel = priorityContext; - } - - if (priorityLevel === SynchronousPriority) { - scheduleSynchronousWork(root); - } + function scheduleWork(root : FiberRoot) { + scheduleWorkAtPriority(root, priorityContext); + } + function scheduleWorkAtPriority(root : FiberRoot, priorityLevel : PriorityLevel) { if (priorityLevel === NoWork) { return; - } - if (priorityLevel > AnimationPriority) { + } else if (priorityLevel === SynchronousPriority) { + scheduleSynchronousWork(root); + } else if (priorityLevel <= AnimationPriority) { + scheduleAnimationWork(root, priorityLevel); + } else { scheduleDeferredWork(root, priorityLevel); return; } - scheduleAnimationWork(root, priorityLevel); } - function scheduleUpdate(fiber: Fiber, priorityLevel : ?PriorityLevel): void { - // Use priority context if no priority is provided - if (priorityLevel == null) { - priorityLevel = priorityContext; - } - + function scheduleUpdate(fiber : Fiber) { + const priorityLevel = priorityContext; while (true) { if (fiber.pendingWorkPriority === NoWork || fiber.pendingWorkPriority >= priorityLevel) { @@ -678,7 +679,7 @@ module.exports = function(config : HostConfig) { if (!fiber.return) { if (fiber.tag === HostContainer) { const root : FiberRoot = (fiber.stateNode : any); - scheduleWork(root, priorityLevel); + scheduleWorkAtPriority(root, priorityLevel); return; } else { throw new Error('Invalid root'); diff --git a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js index 8e27e739811d..b1d985771c4a 100644 --- a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js +++ b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js @@ -22,6 +22,8 @@ type UpdateQueueNode = { export type UpdateQueue = UpdateQueueNode & { isReplace: boolean, isForced: boolean, + hasUpdate: boolean, + hasCallback: boolean, tail: UpdateQueueNode }; @@ -33,6 +35,8 @@ exports.createUpdateQueue = function(partialState : mixed) : UpdateQueue { next: null, isReplace: false, isForced: false, + hasUpdate: partialState != null, + hasCallback: false, tail: (null : any), }; queue.tail = queue; @@ -48,6 +52,7 @@ function addToQueue(queue : UpdateQueue, partialState : mixed) : UpdateQueue { }; queue.tail.next = node; queue.tail = node; + queue.hasUpdate = queue.hasUpdate || (partialState == null); return queue; } @@ -59,15 +64,21 @@ exports.addCallbackToQueue = function(queue : UpdateQueue, callback: Function) : addToQueue(queue, null); } queue.tail.callback = callback; + queue.hasCallback = true; return queue; }; exports.callCallbacks = function(queue : UpdateQueue, context : any) { let node : ?UpdateQueueNode = queue; while (node) { - if (node.callback && !node.callbackWasCalled) { + const callback = node.callback; + if (callback && !node.callbackWasCalled) { node.callbackWasCalled = true; - node.callback.call(context); + if (typeof context !== 'undefined') { + callback.call(context); + } else { + callback(); + } } node = node.next; } diff --git a/src/renderers/shared/fiber/ReactTypeOfSideEffect.js b/src/renderers/shared/fiber/ReactTypeOfSideEffect.js index 55a9cabc42e7..19daf44853ff 100644 --- a/src/renderers/shared/fiber/ReactTypeOfSideEffect.js +++ b/src/renderers/shared/fiber/ReactTypeOfSideEffect.js @@ -12,12 +12,17 @@ 'use strict'; -export type TypeOfSideEffect = 0 | 1 | 2 | 3 | 4; +export type TypeOfSideEffect = 0 | 1 | 2 | 3 | 4 | 8 | 9 | 10 | 11 | 12; module.exports = { - NoEffect: 0, - Placement: 1, - Update: 2, - PlacementAndUpdate: 3, - Deletion: 4, + NoEffect: 0, // 0b0000 + Placement: 1, // 0b0001 + Update: 2, // 0b0010 + PlacementAndUpdate: 3, // 0b0011 + Deletion: 4, // 0b0100 + Callback: 8, // 0b1000 + PlacementAndCallback: 9, // 0b1001 + UpdateAndCallback: 10, // 0b1010 + PlacementAndUpdateAndCallback: 11, // 0b1011 + DeletionAndCallback: 12, // 0b1100 }; diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js index 6722ccdf33dd..9e64dc43c130 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js @@ -860,6 +860,35 @@ describe('ReactIncrementalSideEffects', () => { expect(called).toBe(true); }); + it('calls setState callback even if component bails out', () => { + let instance; + class Foo extends React.Component { + constructor() { + super(); + instance = this; + this.state = { text: 'foo' }; + } + shouldComponentUpdate(nextProps, nextState) { + return this.state.text !== nextState.text; + } + render() { + return ; + } + } + + ReactNoop.render(); + ReactNoop.flush(); + expect(ReactNoop.getChildren()).toEqual([ + span('foo'), + ]); + let called = false; + instance.setState({}, () => { + called = true; + }); + ReactNoop.flush(); + expect(called).toBe(true); + }); + // TODO: Test that callbacks are not lost if an update is preempted. it('calls componentWillUnmount after a deletion, even if nested', () => { From 95084bedbb4ba8eed9131af5ebf6d86e69b54124 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 2 Nov 2016 19:33:02 +0000 Subject: [PATCH 14/14] Fix lint by removing unneeded type import --- scripts/fiber/tests-failing.txt | 3 --- scripts/fiber/tests-passing.txt | 1 + src/renderers/shared/fiber/ReactFiberBeginWork.js | 2 +- src/renderers/shared/fiber/ReactFiberClassComponent.js | 1 - 4 files changed, 2 insertions(+), 5 deletions(-) diff --git a/scripts/fiber/tests-failing.txt b/scripts/fiber/tests-failing.txt index f91c436b8d3d..955d305f69e0 100644 --- a/scripts/fiber/tests-failing.txt +++ b/scripts/fiber/tests-failing.txt @@ -87,9 +87,6 @@ src/renderers/art/__tests__/ReactART-test.js * resolves refs before componentDidMount * resolves refs before componentDidUpdate -src/renderers/dom/__tests__/ReactDOMProduction-test.js -* should throw with an error code in production - src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js * should set style attribute when styles exist * should warn when using hyphenated style names diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 3046357ff1c1..b2d06adb7274 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -438,6 +438,7 @@ src/renderers/dom/__tests__/ReactDOMProduction-test.js * should use prod React * should handle a simple flow * should call lifecycle methods +* should throw with an error code in production src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js * should render strings as children diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index e1c2b1284b17..15ba59a79434 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -15,7 +15,7 @@ import type { ReactCoroutine } from 'ReactCoroutine'; import type { Fiber } from 'ReactFiber'; import type { HostConfig } from 'ReactFiberReconciler'; -import type { PriorityLevel } from 'ReactPriorityLevel'; +import type { PriorityLevel } from 'ReactPriorityLevel'; import ReactCurrentOwner from 'ReactCurrentOwner'; var { diff --git a/src/renderers/shared/fiber/ReactFiberClassComponent.js b/src/renderers/shared/fiber/ReactFiberClassComponent.js index db71854a1213..df1ceaa3bec1 100644 --- a/src/renderers/shared/fiber/ReactFiberClassComponent.js +++ b/src/renderers/shared/fiber/ReactFiberClassComponent.js @@ -13,7 +13,6 @@ 'use strict'; import type { Fiber } from 'ReactFiber'; -import type { PriorityLevel } from 'ReactPriorityLevel'; import type { UpdateQueue } from 'ReactFiberUpdateQueue'; var {