From a2a189bf65b882c026425a0d6a43488f92a569c8 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 8 Aug 2016 19:22:45 -0700 Subject: [PATCH 01/21] Move priority reset to the end and search pending work in work tree The priority level gets reset at the wrong time because I rely on mutating it at the wrong point. This moves it to the end of completed work as a second pass over the children to see what the highest priority is. This is inefficient for large sets but we can try to find a smarter way to do this later. E.g. passing it down the stack. This bug fix revealed another bug that I had flagged before that we're finding work to do in the "current" tree instead of the working tree. For trees that were paused, e.g. childInProgress trees, this won't work since don't have a current tree to search. Therefore I fixed findNextUnitOfWorkAtPriority to use workInProgress instead of current. --- src/renderers/shared/fiber/ReactChildFiber.js | 2 + src/renderers/shared/fiber/ReactFiber.js | 6 + .../shared/fiber/ReactFiberBeginWork.js | 34 ++--- .../shared/fiber/ReactFiberPendingWork.js | 130 ++++++++++-------- .../shared/fiber/ReactFiberReconciler.js | 2 + .../shared/fiber/ReactFiberScheduler.js | 34 +++-- 6 files changed, 117 insertions(+), 91 deletions(-) diff --git a/src/renderers/shared/fiber/ReactChildFiber.js b/src/renderers/shared/fiber/ReactChildFiber.js index 90ea04d0951..cb7b36cd4b4 100644 --- a/src/renderers/shared/fiber/ReactChildFiber.js +++ b/src/renderers/shared/fiber/ReactChildFiber.js @@ -63,6 +63,7 @@ function ChildReconciler(shouldClone) { // Will fix reconciliation properly later. const clone = shouldClone ? cloneFiber(existingChild, priority) : existingChild; if (!shouldClone) { + // TODO: This might be lowering the priority of nested unfinished work. clone.pendingWorkPriority = priority; } clone.pendingProps = element.props; @@ -134,6 +135,7 @@ function ChildReconciler(shouldClone) { // Get the clone of the existing fiber. const clone = shouldClone ? cloneFiber(existingChild, priority) : existingChild; if (!shouldClone) { + // TODO: This might be lowering the priority of nested unfinished work. clone.pendingWorkPriority = priority; } clone.pendingProps = element.props; diff --git a/src/renderers/shared/fiber/ReactFiber.js b/src/renderers/shared/fiber/ReactFiber.js index fb3d6f9723f..775c83ac62f 100644 --- a/src/renderers/shared/fiber/ReactFiber.js +++ b/src/renderers/shared/fiber/ReactFiber.js @@ -168,6 +168,9 @@ exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fi alt.pendingProps = fiber.pendingProps; alt.pendingWorkPriority = priorityLevel; + alt.memoizedProps = fiber.memoizedProps; + alt.output = fiber.output; + // Whenever we clone, we do so to get a new work in progress. // This ensures that we've reset these in the new tree. alt.nextEffect = null; @@ -189,6 +192,9 @@ exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fi alt.pendingProps = fiber.pendingProps; alt.pendingWorkPriority = priorityLevel; + alt.memoizedProps = fiber.memoizedProps; + alt.output = fiber.output; + alt.alternate = fiber; fiber.alternate = alt; return alt; diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 6c664784f07..c5e63336420 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -40,6 +40,8 @@ var { findNextUnitOfWorkAtPriority } = require('ReactFiberPendingWork'); module.exports = function(config : HostConfig) { function reconcileChildren(current, workInProgress, nextChildren) { + // TODO: Children needs to be able to reconcile in place if we are + // overriding progressed work. const priority = workInProgress.pendingWorkPriority; reconcileChildrenAtPriority(current, workInProgress, nextChildren, priority); } @@ -76,7 +78,6 @@ module.exports = function(config : HostConfig) { var props = workInProgress.pendingProps; var nextChildren = fn(props); reconcileChildren(current, workInProgress, nextChildren); - workInProgress.pendingWorkPriority = NoWork; } function updateClassComponent(current : ?Fiber, workInProgress : Fiber) { @@ -106,7 +107,6 @@ module.exports = function(config : HostConfig) { instance.props = props; var nextChildren = instance.render(); reconcileChildren(current, workInProgress, nextChildren); - workInProgress.pendingWorkPriority = NoWork; return workInProgress.childInProgress; } @@ -125,11 +125,9 @@ module.exports = function(config : HostConfig) { // becomes part of the render tree, even though it never completed. Its // `output` property is unpredictable because of it. reconcileChildrenAtPriority(current, workInProgress, nextChildren, OffscreenPriority); - workInProgress.pendingWorkPriority = OffscreenPriority; return null; } else { reconcileChildren(current, workInProgress, nextChildren); - workInProgress.pendingWorkPriority = NoWork; return workInProgress.childInProgress; } } @@ -153,7 +151,6 @@ module.exports = function(config : HostConfig) { } } reconcileChildren(current, workInProgress, value); - workInProgress.pendingWorkPriority = NoWork; } function updateCoroutineComponent(current, workInProgress) { @@ -162,10 +159,11 @@ module.exports = function(config : HostConfig) { throw new Error('Should be resolved by now'); } reconcileChildren(current, workInProgress, coroutine.children); - workInProgress.pendingWorkPriority = NoWork; } function reuseChildren(returnFiber : Fiber, firstChild : Fiber) { + // TODO on the TODO: Is this not necessary anymore because I moved the + // priority reset? // TODO: None of this should be necessary if structured better. // The returnFiber pointer only needs to be updated when we walk into this child // which we don't do right now. If the pending work priority indicated only @@ -210,7 +208,6 @@ module.exports = function(config : HostConfig) { workInProgress.output = current.output; const priorityLevel = workInProgress.pendingWorkPriority; workInProgress.pendingProps = null; - workInProgress.pendingWorkPriority = NoWork; workInProgress.stateNode = current.stateNode; workInProgress.childInProgress = current.childInProgress; if (current.child) { @@ -220,10 +217,10 @@ module.exports = function(config : HostConfig) { workInProgress.child = current.child; reuseChildren(workInProgress, workInProgress.child); if (workInProgress.pendingWorkPriority !== NoWork && workInProgress.pendingWorkPriority <= priorityLevel) { - // TODO: This passes the current node and reads the priority level and - // pending props from that. We want it to read our priority level and - // pending props from the work in progress. Needs restructuring. - return findNextUnitOfWorkAtPriority(current, priorityLevel); + return findNextUnitOfWorkAtPriority( + workInProgress, + workInProgress.pendingWorkPriority + ); } else { return null; } @@ -239,22 +236,13 @@ module.exports = function(config : HostConfig) { // looking for. In that case, we should be able to just bail out. const priorityLevel = workInProgress.pendingWorkPriority; workInProgress.pendingProps = null; - workInProgress.pendingWorkPriority = NoWork; workInProgress.firstEffect = null; workInProgress.nextEffect = null; workInProgress.lastEffect = null; - if (workInProgress.child) { - // On the way up here, we reset the child node to be the current one by - // cloning. However, it is really the original child that represents the - // already completed work. Therefore we have to reuse the alternate. - // But if we don't have a current, this was not cloned. This is super weird. - const child = !current ? workInProgress.child : workInProgress.child.alternate; - if (!child) { - throw new Error('We must have a current child to be able to use this.'); - } - workInProgress.child = child; + const child = workInProgress.child; + if (child) { // Ensure that the effects of reused work are preserved. reuseChildrenEffects(workInProgress, child); // If we bail out but still has work with the current priority in this @@ -299,7 +287,6 @@ module.exports = function(config : HostConfig) { reconcileChildren(current, workInProgress, workInProgress.pendingProps); // A yield component is just a placeholder, we can just run through the // next one immediately. - workInProgress.pendingWorkPriority = NoWork; if (workInProgress.childInProgress) { return beginWork( workInProgress.childInProgress.alternate, @@ -327,7 +314,6 @@ module.exports = function(config : HostConfig) { case YieldComponent: // A yield component is just a placeholder, we can just run through the // next one immediately. - workInProgress.pendingWorkPriority = NoWork; if (workInProgress.sibling) { return beginWork( workInProgress.sibling.alternate, diff --git a/src/renderers/shared/fiber/ReactFiberPendingWork.js b/src/renderers/shared/fiber/ReactFiberPendingWork.js index eb61c4595ff..fff76afec76 100644 --- a/src/renderers/shared/fiber/ReactFiberPendingWork.js +++ b/src/renderers/shared/fiber/ReactFiberPendingWork.js @@ -34,20 +34,50 @@ function cloneSiblings(current : Fiber, workInProgress : Fiber, returnFiber : Fi workInProgress.sibling = null; } -exports.findNextUnitOfWorkAtPriority = function(currentRoot : Fiber, priorityLevel : PriorityLevel) : ?Fiber { - let current = currentRoot; - while (current) { - if (current.pendingWorkPriority !== NoWork && - current.pendingWorkPriority <= priorityLevel) { +function cloneChildrenIfNeeded(workInProgress : Fiber) { + const current = workInProgress.alternate; + if (!current || workInProgress.child !== current.child) { + // If there is no alternate, then we don't need to clone the children. + // If the children of the alternate fiber is a different set, then we don't + // need to clone. We need to reset the return fiber though since we'll + // traverse down into them. + // TODO: I don't think it is actually possible for them to be anything but + // equal at this point because this fiber was just cloned. Can we skip this + // check? Similar question about the return fiber. + let child = workInProgress.child; + while (child) { + child.return = workInProgress; + child = child.sibling; + } + return; + } + // TODO: This used to reset the pending priority. Not sure if that is needed. + // workInProgress.pendingWorkPriority = current.pendingWorkPriority; + + // TODO: The below priority used to be set to NoWork which would've + // dropped work. This is currently unobservable but will become + // observable when the first sibling has lower priority work remaining + // than the next sibling. At that point we should add tests that catches + // this. + + const currentChild = current.child; + if (!currentChild) { + return; + } + workInProgress.child = cloneFiber( + currentChild, + currentChild.pendingWorkPriority + ); + cloneSiblings(currentChild, workInProgress.child, workInProgress); +} + +exports.findNextUnitOfWorkAtPriority = function(workRoot : Fiber, priorityLevel : PriorityLevel) : ?Fiber { + let workInProgress = workRoot; + while (workInProgress) { + if (workInProgress.pendingWorkPriority !== NoWork && + workInProgress.pendingWorkPriority <= priorityLevel) { // This node has work to do that fits our priority level criteria. - if (current.pendingProps !== null) { - // We found some work to do. We need to return the "work in progress" - // of this node which will be the alternate. - const workInProgress = current.alternate; - if (!workInProgress) { - throw new Error('Should have wip now'); - } - workInProgress.pendingProps = current.pendingProps; + if (workInProgress.pendingProps !== null) { return workInProgress; } @@ -56,71 +86,57 @@ exports.findNextUnitOfWorkAtPriority = function(currentRoot : Fiber, priorityLev // because it is the highest priority for the whole subtree. // TODO: Coroutines can have work in their stateNode which is another // type of child that needs to be searched for work. - if (current.childInProgress) { - let workInProgress = current.childInProgress; - while (workInProgress) { - workInProgress.return = current.alternate; - workInProgress = workInProgress.sibling; + if (workInProgress.childInProgress) { + let child = workInProgress.childInProgress; + while (child) { + child.return = workInProgress; + child = child.sibling; } - workInProgress = current.childInProgress; - while (workInProgress) { - // Don't bother drilling further down this tree if there is no child. - if (workInProgress.pendingWorkPriority !== NoWork && - workInProgress.pendingWorkPriority <= priorityLevel && - workInProgress.pendingProps !== null) { - return workInProgress; + child = workInProgress.childInProgress; + while (child) { + // Don't bother drilling further down this tree if there is no child + // with more content. + // TODO: Shouldn't this still drill down even though the first + // shallow level doesn't have anything pending on it. + if (child.pendingWorkPriority !== NoWork && + child.pendingWorkPriority <= priorityLevel && + child.pendingProps !== null) { + return child; } - workInProgress = workInProgress.sibling; + child = child.sibling; } - } else if (current.child) { - let currentChild = current.child; - currentChild.return = current; - // Ensure we have a work in progress copy to backtrack through. - let workInProgress = current.alternate; - if (!workInProgress) { - throw new Error('Should have wip now'); - } - workInProgress.pendingWorkPriority = current.pendingWorkPriority; - // TODO: The below priority used to be set to NoWork which would've - // dropped work. This is currently unobservable but will become - // observable when the first sibling has lower priority work remaining - // than the next sibling. At that point we should add tests that catches - // this. - workInProgress.child = cloneFiber( - currentChild, - currentChild.pendingWorkPriority - ); - cloneSiblings(currentChild, workInProgress.child, workInProgress); - current = currentChild; + } else if (workInProgress.child) { + cloneChildrenIfNeeded(workInProgress); + workInProgress = workInProgress.child; continue; } // If we match the priority but has no child and no work to do, // then we can safely reset the flag. - current.pendingWorkPriority = NoWork; + workInProgress.pendingWorkPriority = NoWork; } - if (current === currentRoot) { - if (current.pendingWorkPriority <= priorityLevel) { + if (workInProgress === workRoot) { + if (workInProgress.pendingWorkPriority <= priorityLevel) { // If this subtree had work left to do, we would have returned it by // now. This could happen if a child with pending work gets cleaned up // but we don't clear the flag then. It is safe to reset it now. - current.pendingWorkPriority = NoWork; + workInProgress.pendingWorkPriority = NoWork; } return null; } - while (!current.sibling) { - current = current.return; - if (!current) { + while (!workInProgress.sibling) { + workInProgress = workInProgress.return; + if (!workInProgress || workInProgress === workRoot) { return null; } - if (current.pendingWorkPriority <= priorityLevel) { + if (workInProgress.pendingWorkPriority <= priorityLevel) { // If this subtree had work left to do, we would have returned it by // now. This could happen if a child with pending work gets cleaned up // but we don't clear the flag then. It is safe to reset it now. - current.pendingWorkPriority = NoWork; + workInProgress.pendingWorkPriority = NoWork; } } - current.sibling.return = current.return; - current = current.sibling; + workInProgress.sibling.return = workInProgress.return; + workInProgress = workInProgress.sibling; } return null; }; diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js index 4ece60a0f2e..aa39d1b48d3 100644 --- a/src/renderers/shared/fiber/ReactFiberReconciler.js +++ b/src/renderers/shared/fiber/ReactFiberReconciler.js @@ -70,6 +70,8 @@ module.exports = function(config : HostConfig) : Reconci const root = createFiberRoot(containerInfo); const container = root.current; // TODO: Use pending work/state instead of props. + // TODO: This should not override the pendingWorkPriority if there is + // higher priority work in the subtree. container.pendingProps = element; container.pendingWorkPriority = LowPriority; diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index b6e71289689..e469d5c0ca8 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -66,22 +66,25 @@ module.exports = function(config : HostConfig) { // roots for high priority work before moving on to lower priorities. let root = nextScheduledRoot; while (root) { - cloneFiber(root.current, root.current.pendingWorkPriority); + let rootInProgress = cloneFiber( + root.current, + root.current.pendingWorkPriority + ); // Find the highest possible priority work to do. // This loop is unrolled just to satisfy Flow's enum constraint. // We could make arbitrary many idle priority levels but having // too many just means flushing changes too often. - let work = findNextUnitOfWorkAtPriority(root.current, HighPriority); + let work = findNextUnitOfWorkAtPriority(rootInProgress, HighPriority); if (work) { nextPriorityLevel = HighPriority; return work; } - work = findNextUnitOfWorkAtPriority(root.current, LowPriority); + work = findNextUnitOfWorkAtPriority(rootInProgress, LowPriority); if (work) { nextPriorityLevel = LowPriority; return work; } - work = findNextUnitOfWorkAtPriority(root.current, OffscreenPriority); + work = findNextUnitOfWorkAtPriority(rootInProgress, OffscreenPriority); if (work) { nextPriorityLevel = OffscreenPriority; return work; @@ -114,6 +117,21 @@ module.exports = function(config : HostConfig) { } } + function resetWorkPriority(workInProgress : Fiber) { + let newPriority = NoWork; + let child = workInProgress.childInProgress || workInProgress.child; + while (child) { + // Ensure that remaining work priority bubbles up. + if (child.pendingWorkPriority !== NoWork && + (newPriority === NoWork || + newPriority > child.pendingWorkPriority)) { + newPriority = child.pendingWorkPriority; + } + child = child.sibling; + } + workInProgress.pendingWorkPriority = newPriority; + } + function completeUnitOfWork(workInProgress : Fiber) : ?Fiber { while (true) { // The current, flushed, state of this fiber is the alternate. @@ -123,6 +141,8 @@ module.exports = function(config : HostConfig) { const current = workInProgress.alternate; const next = completeWork(current, workInProgress); + resetWorkPriority(workInProgress); + // The work is now done. We don't need this anymore. This flags // to the system not to redo any work here. workInProgress.pendingProps = null; @@ -130,12 +150,6 @@ module.exports = function(config : HostConfig) { const returnFiber = workInProgress.return; if (returnFiber) { - // Ensure that remaining work priority bubbles up. - if (workInProgress.pendingWorkPriority !== NoWork && - (returnFiber.pendingWorkPriority === NoWork || - returnFiber.pendingWorkPriority > workInProgress.pendingWorkPriority)) { - returnFiber.pendingWorkPriority = workInProgress.pendingWorkPriority; - } // Ensure that the first and last effect of the parent corresponds // to the children's first and last effect. This probably relies on // children completing in order. From fc52de91dbd2f5d3c5d41f7949f4b54d9e71a8c5 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Thu, 1 Sep 2016 20:49:21 -0700 Subject: [PATCH 02/21] Drop Separate findPendingWork Phase The findPendingWork phase is the same as just walking the tree the normal way effectively. It only makes things more complex to think about. We might possibly be able to write a few special branches to optimize it but for now it doesn't seem necessary. --- src/renderers/shared/fiber/ReactChildFiber.js | 54 +++++++ src/renderers/shared/fiber/ReactFiber.js | 4 +- .../shared/fiber/ReactFiberBeginWork.js | 110 +++++++++----- .../shared/fiber/ReactFiberPendingWork.js | 142 ------------------ .../shared/fiber/ReactFiberScheduler.js | 77 ++++------ .../fiber/__tests__/ReactIncremental-test.js | 6 +- .../ReactIncrementalSideEffects-test.js | 3 +- 7 files changed, 167 insertions(+), 229 deletions(-) delete mode 100644 src/renderers/shared/fiber/ReactFiberPendingWork.js diff --git a/src/renderers/shared/fiber/ReactChildFiber.js b/src/renderers/shared/fiber/ReactChildFiber.js index cb7b36cd4b4..8a3ad31d416 100644 --- a/src/renderers/shared/fiber/ReactChildFiber.js +++ b/src/renderers/shared/fiber/ReactChildFiber.js @@ -221,3 +221,57 @@ function ChildReconciler(shouldClone) { exports.reconcileChildFibers = ChildReconciler(true); exports.reconcileChildFibersInPlace = ChildReconciler(false); + + +function cloneSiblings(current : Fiber, workInProgress : Fiber, returnFiber : Fiber) { + workInProgress.return = returnFiber; + while (current.sibling) { + current = current.sibling; + workInProgress = workInProgress.sibling = cloneFiber( + current, + current.pendingWorkPriority + ); + workInProgress.return = returnFiber; + } + workInProgress.sibling = null; +} + +exports.cloneChildFibers = function(workInProgress : Fiber) { + if (!workInProgress.child) { + return; + } + const current = workInProgress.alternate; + if (!current || workInProgress.child !== current.child) { + // If there is no alternate, then we don't need to clone the children. + // If the children of the alternate fiber is a different set, then we don't + // need to clone. We need to reset the return fiber though since we'll + // traverse down into them. + // TODO: I don't think it is actually possible for them to be anything but + // equal at this point because this fiber was just cloned. Can we skip this + // check? Similar question about the return fiber. + let child = workInProgress.child; + while (child) { + child.return = workInProgress; + child = child.sibling; + } + return; + } + // TODO: This used to reset the pending priority. Not sure if that is needed. + // workInProgress.pendingWorkPriority = current.pendingWorkPriority; + + // TODO: The below priority used to be set to NoWork which would've + // dropped work. This is currently unobservable but will become + // observable when the first sibling has lower priority work remaining + // than the next sibling. At that point we should add tests that catches + // this. + + const currentChild = current.child; + if (!currentChild) { + return; + } + workInProgress.child = cloneFiber( + currentChild, + currentChild.pendingWorkPriority + ); + cloneSiblings(currentChild, workInProgress.child, workInProgress); +} diff --git a/src/renderers/shared/fiber/ReactFiber.js b/src/renderers/shared/fiber/ReactFiber.js index 775c83ac62f..6cecc90b7cc 100644 --- a/src/renderers/shared/fiber/ReactFiber.js +++ b/src/renderers/shared/fiber/ReactFiber.js @@ -164,7 +164,7 @@ exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fi alt.child = fiber.child; alt.childInProgress = fiber.childInProgress; alt.sibling = fiber.sibling; - alt.ref = alt.ref; + alt.ref = fiber.ref; alt.pendingProps = fiber.pendingProps; alt.pendingWorkPriority = priorityLevel; @@ -187,7 +187,7 @@ exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fi alt.child = fiber.child; alt.childInProgress = fiber.childInProgress; alt.sibling = fiber.sibling; - alt.ref = alt.ref; + alt.ref = fiber.ref; // pendingProps is here for symmetry but is unnecessary in practice for now. alt.pendingProps = fiber.pendingProps; alt.pendingWorkPriority = priorityLevel; diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index c5e63336420..7d48b0bc8af 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -19,6 +19,7 @@ import type { HostConfig } from 'ReactFiberReconciler'; var { reconcileChildFibers, reconcileChildFibersInPlace, + cloneChildFibers, } = require('ReactChildFiber'); var ReactTypeOfWork = require('ReactTypeOfWork'); var { @@ -35,7 +36,6 @@ var { NoWork, OffscreenPriority, } = require('ReactPriorityLevel'); -var { findNextUnitOfWorkAtPriority } = require('ReactFiberPendingWork'); module.exports = function(config : HostConfig) { @@ -64,9 +64,10 @@ module.exports = function(config : HostConfig) { priorityLevel ); } else { + // TODO: workInProgress.childInProgress = reconcileChildFibers( workInProgress, - current ? current.child : null, + current ? current.child : workInProgress.child, nextChildren, priorityLevel ); @@ -76,8 +77,10 @@ module.exports = function(config : HostConfig) { function updateFunctionalComponent(current, workInProgress) { var fn = workInProgress.type; var props = workInProgress.pendingProps; + var nextChildren = fn(props); reconcileChildren(current, workInProgress, nextChildren); + return workInProgress.childInProgress; } function updateClassComponent(current : ?Fiber, workInProgress : Fiber) { @@ -104,9 +107,11 @@ module.exports = function(config : HostConfig) { } } } + instance.props = props; var nextChildren = instance.render(); reconcileChildren(current, workInProgress, nextChildren); + return workInProgress.childInProgress; } @@ -125,6 +130,22 @@ module.exports = function(config : HostConfig) { // becomes part of the render tree, even though it never completed. Its // `output` property is unpredictable because of it. reconcileChildrenAtPriority(current, workInProgress, nextChildren, OffscreenPriority); + workInProgress.child = current ? current.child : null; + let child = workInProgress.childInProgress; + while (child) { + const currentChild = child.alternate; + if (currentChild) { + child.child = currentChild.child; + child.childInProgress = currentChild.childInProgress; + child.memoizedProps = currentChild.memoizedProps; + child.output = currentChild.output; + } + child.nextEffect = null; + child.firstEffect = null; + child.lastEffect = null; + + child = child.sibling; + } return null; } else { reconcileChildren(current, workInProgress, nextChildren); @@ -151,6 +172,7 @@ module.exports = function(config : HostConfig) { } } reconcileChildren(current, workInProgress, value); + return workInProgress.childInProgress; } function updateCoroutineComponent(current, workInProgress) { @@ -175,10 +197,13 @@ module.exports = function(config : HostConfig) { // Update the returnFiber of the child to the newest fiber. child.return = returnFiber; // Retain the priority if there's any work left to do in the children. - if (child.pendingWorkPriority !== NoWork && + /*if (child.pendingWorkPriority !== NoWork && (returnFiber.pendingWorkPriority === NoWork || returnFiber.pendingWorkPriority > child.pendingWorkPriority)) { returnFiber.pendingWorkPriority = child.pendingWorkPriority; + }*/ + if (!child.pendingProps && !child.memoizedProps) { + throw new Error('Should have memoized props by now'); } } while (child = child.sibling); } @@ -200,6 +225,7 @@ module.exports = function(config : HostConfig) { } while (child = child.sibling); } +/* function bailoutOnCurrent(current : Fiber, workInProgress : Fiber) : ?Fiber { // The most likely scenario is that the previous copy of the tree contains // the same props as the new one. In that case, we can just copy the output @@ -207,41 +233,39 @@ module.exports = function(config : HostConfig) { workInProgress.memoizedProps = workInProgress.pendingProps; workInProgress.output = current.output; const priorityLevel = workInProgress.pendingWorkPriority; - workInProgress.pendingProps = null; + // workInProgress.pendingProps = null; workInProgress.stateNode = current.stateNode; - workInProgress.childInProgress = current.childInProgress; - if (current.child) { - // If we bail out but still has work with the current priority in this - // subtree, we need to go find it right now. If we don't, we won't flush - // it until the next tick. - workInProgress.child = current.child; - reuseChildren(workInProgress, workInProgress.child); - if (workInProgress.pendingWorkPriority !== NoWork && workInProgress.pendingWorkPriority <= priorityLevel) { - return findNextUnitOfWorkAtPriority( - workInProgress, - workInProgress.pendingWorkPriority - ); - } else { - return null; - } - } else { - workInProgress.child = null; - return null; - } + + workInProgress.nextEffect = null; + workInProgress.firstEffect = null; + workInProgress.lastEffect = null; + + workInProgress.childInProgress = null; // current.childInProgress; + workInProgress.child = current.child; + + cloneChildFibers(workInProgress); + + // TODO: Maybe bailout with null if the children priority flag indicate + // that there is no nested work. + return workInProgress.child; } +*/ function bailoutOnAlreadyFinishedWork(current, workInProgress : Fiber) : ?Fiber { // If we started this work before, and finished it, or if we're in a // ping-pong update scenario, this version could already be what we're // looking for. In that case, we should be able to just bail out. const priorityLevel = workInProgress.pendingWorkPriority; - workInProgress.pendingProps = null; + // workInProgress.pendingProps = null; workInProgress.firstEffect = null; workInProgress.nextEffect = null; workInProgress.lastEffect = null; const child = workInProgress.child; + if (workInProgress.childInProgress) { + throw new Error('Child in progress means we cannot bail here.'); + } if (child) { // Ensure that the effects of reused work are preserved. reuseChildrenEffects(workInProgress, child); @@ -249,18 +273,31 @@ module.exports = function(config : HostConfig) { // subtree, we need to go find it right now. If we don't, we won't flush // it until the next tick. reuseChildren(workInProgress, child); - if (workInProgress.pendingWorkPriority !== NoWork && - workInProgress.pendingWorkPriority <= priorityLevel) { - // TODO: This passes the current node and reads the priority level and - // pending props from that. We want it to read our priority level and - // pending props from the work in progress. Needs restructuring. - return findNextUnitOfWorkAtPriority(workInProgress, priorityLevel); - } + // TODO: Maybe bailout with null if the children priority flag indicate + // that there is no nested work. + return workInProgress.child; } return null; } - function beginWork(current : ?Fiber, workInProgress : Fiber) : ?Fiber { + function beginWork(current : ?Fiber, workInProgress : Fiber, priorityLevel) : ?Fiber { + if (!workInProgress.pendingProps) { + throw new Error('should have pending props here'); + } + + if (workInProgress.pendingWorkPriority === NoWork || + workInProgress.pendingWorkPriority > priorityLevel) { + + if (current) { + workInProgress.child = current.child; + workInProgress.childInProgress = current.childInProgress; + workInProgress.memoizedProps = current.memoizedProps; + workInProgress.output = current.output; + } + + return null; + } + // The current, flushed, state of this fiber is the alternate. // Ideally nothing should rely on this, but relying on it here // means that we don't need an additional field on the work in @@ -276,11 +313,9 @@ module.exports = function(config : HostConfig) { switch (workInProgress.tag) { case IndeterminateComponent: - mountIndeterminateComponent(current, workInProgress); - return workInProgress.childInProgress; + return mountIndeterminateComponent(current, workInProgress); case FunctionalComponent: - updateFunctionalComponent(current, workInProgress); - return workInProgress.childInProgress; + return updateFunctionalComponent(current, workInProgress); case ClassComponent: return updateClassComponent(current, workInProgress); case HostContainer: @@ -295,6 +330,9 @@ module.exports = function(config : HostConfig) { } return null; case HostComponent: + if (workInProgress.stateNode && config.beginUpdate) { + config.beginUpdate(workInProgress.stateNode); + } return updateHostComponent(current, workInProgress); case CoroutineHandlerPhase: // This is a restart. Reset the tag to the initial phase. diff --git a/src/renderers/shared/fiber/ReactFiberPendingWork.js b/src/renderers/shared/fiber/ReactFiberPendingWork.js deleted file mode 100644 index fff76afec76..00000000000 --- a/src/renderers/shared/fiber/ReactFiberPendingWork.js +++ /dev/null @@ -1,142 +0,0 @@ -/** - * 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. - * - * @providesModule ReactFiberPendingWork - * @flow - */ - -'use strict'; - -import type { Fiber } from 'ReactFiber'; -import type { PriorityLevel } from 'ReactPriorityLevel'; - -var { cloneFiber } = require('ReactFiber'); - -var { - NoWork, -} = require('ReactPriorityLevel'); - -function cloneSiblings(current : Fiber, workInProgress : Fiber, returnFiber : Fiber) { - workInProgress.return = returnFiber; - while (current.sibling) { - current = current.sibling; - workInProgress = workInProgress.sibling = cloneFiber( - current, - current.pendingWorkPriority - ); - workInProgress.return = returnFiber; - } - workInProgress.sibling = null; -} - -function cloneChildrenIfNeeded(workInProgress : Fiber) { - const current = workInProgress.alternate; - if (!current || workInProgress.child !== current.child) { - // If there is no alternate, then we don't need to clone the children. - // If the children of the alternate fiber is a different set, then we don't - // need to clone. We need to reset the return fiber though since we'll - // traverse down into them. - // TODO: I don't think it is actually possible for them to be anything but - // equal at this point because this fiber was just cloned. Can we skip this - // check? Similar question about the return fiber. - let child = workInProgress.child; - while (child) { - child.return = workInProgress; - child = child.sibling; - } - return; - } - // TODO: This used to reset the pending priority. Not sure if that is needed. - // workInProgress.pendingWorkPriority = current.pendingWorkPriority; - - // TODO: The below priority used to be set to NoWork which would've - // dropped work. This is currently unobservable but will become - // observable when the first sibling has lower priority work remaining - // than the next sibling. At that point we should add tests that catches - // this. - - const currentChild = current.child; - if (!currentChild) { - return; - } - workInProgress.child = cloneFiber( - currentChild, - currentChild.pendingWorkPriority - ); - cloneSiblings(currentChild, workInProgress.child, workInProgress); -} - -exports.findNextUnitOfWorkAtPriority = function(workRoot : Fiber, priorityLevel : PriorityLevel) : ?Fiber { - let workInProgress = workRoot; - while (workInProgress) { - if (workInProgress.pendingWorkPriority !== NoWork && - workInProgress.pendingWorkPriority <= priorityLevel) { - // This node has work to do that fits our priority level criteria. - if (workInProgress.pendingProps !== null) { - return workInProgress; - } - - // If we have a child let's see if any of our children has work to do. - // Only bother doing this at all if the current priority level matches - // because it is the highest priority for the whole subtree. - // TODO: Coroutines can have work in their stateNode which is another - // type of child that needs to be searched for work. - if (workInProgress.childInProgress) { - let child = workInProgress.childInProgress; - while (child) { - child.return = workInProgress; - child = child.sibling; - } - child = workInProgress.childInProgress; - while (child) { - // Don't bother drilling further down this tree if there is no child - // with more content. - // TODO: Shouldn't this still drill down even though the first - // shallow level doesn't have anything pending on it. - if (child.pendingWorkPriority !== NoWork && - child.pendingWorkPriority <= priorityLevel && - child.pendingProps !== null) { - return child; - } - child = child.sibling; - } - } else if (workInProgress.child) { - cloneChildrenIfNeeded(workInProgress); - workInProgress = workInProgress.child; - continue; - } - // If we match the priority but has no child and no work to do, - // then we can safely reset the flag. - workInProgress.pendingWorkPriority = NoWork; - } - if (workInProgress === workRoot) { - if (workInProgress.pendingWorkPriority <= priorityLevel) { - // If this subtree had work left to do, we would have returned it by - // now. This could happen if a child with pending work gets cleaned up - // but we don't clear the flag then. It is safe to reset it now. - workInProgress.pendingWorkPriority = NoWork; - } - return null; - } - while (!workInProgress.sibling) { - workInProgress = workInProgress.return; - if (!workInProgress || workInProgress === workRoot) { - return null; - } - if (workInProgress.pendingWorkPriority <= priorityLevel) { - // If this subtree had work left to do, we would have returned it by - // now. This could happen if a child with pending work gets cleaned up - // but we don't clear the flag then. It is safe to reset it now. - workInProgress.pendingWorkPriority = NoWork; - } - } - workInProgress.sibling.return = workInProgress.return; - workInProgress = workInProgress.sibling; - } - return null; -}; diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index e469d5c0ca8..96da4563514 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -22,7 +22,6 @@ var ReactFiberCompleteWork = require('ReactFiberCompleteWork'); var ReactFiberCommitWork = require('ReactFiberCommitWork'); var { cloneFiber } = require('ReactFiber'); -var { findNextUnitOfWorkAtPriority } = require('ReactFiberPendingWork'); var { NoWork, @@ -65,36 +64,23 @@ module.exports = function(config : HostConfig) { // TODO: This is scanning one root at a time. It should be scanning all // roots for high priority work before moving on to lower priorities. let root = nextScheduledRoot; + let highestPriorityRoot = null; + let highestPriorityLevel = NoWork; while (root) { - let rootInProgress = cloneFiber( - root.current, - root.current.pendingWorkPriority - ); - // Find the highest possible priority work to do. - // This loop is unrolled just to satisfy Flow's enum constraint. - // We could make arbitrary many idle priority levels but having - // too many just means flushing changes too often. - let work = findNextUnitOfWorkAtPriority(rootInProgress, HighPriority); - if (work) { - nextPriorityLevel = HighPriority; - return work; - } - work = findNextUnitOfWorkAtPriority(rootInProgress, LowPriority); - if (work) { - nextPriorityLevel = LowPriority; - return work; - } - work = findNextUnitOfWorkAtPriority(rootInProgress, OffscreenPriority); - if (work) { - nextPriorityLevel = OffscreenPriority; - return work; + if (highestPriorityLevel === NoWork || + highestPriorityLevel > root.current.pendingWorkPriority) { + highestPriorityLevel = root.current.pendingWorkPriority; + highestPriorityRoot = root; } // We didn't find anything to do in this root, so let's try the next one. root = root.nextScheduledRoot; } - root = nextScheduledRoot; - while (root) { - root = root.nextScheduledRoot; + if (highestPriorityRoot) { + nextPriorityLevel = highestPriorityLevel; + return cloneFiber( + highestPriorityRoot.current, + highestPriorityLevel + ); } nextPriorityLevel = NoWork; @@ -119,7 +105,7 @@ module.exports = function(config : HostConfig) { function resetWorkPriority(workInProgress : Fiber) { let newPriority = NoWork; - let child = workInProgress.childInProgress || workInProgress.child; + let child = workInProgress.child; while (child) { // Ensure that remaining work priority bubbles up. if (child.pendingWorkPriority !== NoWork && @@ -139,13 +125,25 @@ module.exports = function(config : HostConfig) { // means that we don't need an additional field on the work in // progress. const current = workInProgress.alternate; - const next = completeWork(current, workInProgress); + let next = null; - resetWorkPriority(workInProgress); + // If this bailed at a lower priority. + // TODO: This branch is currently needed if a particular type of component + // ends up being a priority lowering. We should probably know that already + // before entering begin work. + if (workInProgress.pendingWorkPriority === NoWork || + workInProgress.pendingWorkPriority > nextPriorityLevel) { + // This fiber was ignored. We need to fall through to the next fiber + // and leave the pending props and work untouched on this fiber. + } else { + next = completeWork(current, workInProgress); - // The work is now done. We don't need this anymore. This flags - // to the system not to redo any work here. - workInProgress.pendingProps = null; + resetWorkPriority(workInProgress); + + // The work is now done. We don't need this anymore. This flags + // to the system not to redo any work here. + workInProgress.pendingProps = null; + } const returnFiber = workInProgress.return; @@ -173,14 +171,6 @@ module.exports = function(config : HostConfig) { } else if (returnFiber) { // If there's no more work in this returnFiber. Complete the returnFiber. workInProgress = returnFiber; - // If we're stepping up through the child, that means we can now commit - // this work. We should only do this when we're stepping upwards because - // completing a downprioritized item is not the same as completing its - // children. - if (workInProgress.childInProgress) { - workInProgress.child = workInProgress.childInProgress; - workInProgress.childInProgress = null; - } continue; } else { // If we're at the root, there's no more work to do. We can flush it. @@ -205,16 +195,13 @@ module.exports = function(config : HostConfig) { } function performUnitOfWork(workInProgress : Fiber) : ?Fiber { - // Ignore work if there is nothing to do. - if (workInProgress.pendingProps === null) { - return completeUnitOfWork(workInProgress); - } // The current, flushed, state of this fiber is the alternate. // Ideally nothing should rely on this, but relying on it here // means that we don't need an additional field on the work in // progress. const current = workInProgress.alternate; - const next = beginWork(current, workInProgress); + const next = beginWork(current, workInProgress, nextPriorityLevel); + if (next) { // If this spawns new work, do that next. return next; diff --git a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js index 65d3b09e0ab..303ef658bfc 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js @@ -410,7 +410,7 @@ describe('ReactIncremental', function() { // Init ReactNoop.render(); - ReactNoop.flushLowPri(55); + ReactNoop.flushLowPri(55 + 25); // We only finish the higher priority work. So the low pri content // has not yet finished mounting. @@ -432,7 +432,7 @@ describe('ReactIncremental', function() { // Make a quick update which will schedule low priority work to // update the middle content. ReactNoop.render(); - ReactNoop.flushLowPri(30); + ReactNoop.flushLowPri(30 + 25); expect(ops).toEqual(['Foo', 'Bar']); @@ -526,7 +526,7 @@ describe('ReactIncremental', function() { ops = []; // The middle content is now pending rendering... - ReactNoop.flushLowPri(30); + ReactNoop.flushLowPri(30 + 25); expect(ops).toEqual(['Content', 'Middle', 'Bar']); // One more Middle left. ops = []; diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js index c6068855610..ebaa321a888 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js @@ -177,7 +177,7 @@ describe('ReactIncrementalSideEffects', function() { // render some higher priority work. The middle content will bailout so // it remains untouched which means that it should reuse it next time. ReactNoop.render(); - ReactNoop.flush(30); + ReactNoop.flush(); // Since we did nothing to the middle subtree during the interuption, // we should be able to reuse the reconciliation work that we already did @@ -270,6 +270,7 @@ describe('ReactIncrementalSideEffects', function() { ]); }); + // TODO: Test that side-effects are not cut off when a work in progress node // moves to "current" without flushing due to having lower priority. Does this // even happen? Maybe a child doesn't get processed because it is lower prio? From ae6e68ebfcc58fb3c6b62afecb8c6fd48635bb7e Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Thu, 1 Sep 2016 20:50:47 -0700 Subject: [PATCH 03/21] Drop childInProgress I will try a slightly different but similar strategy. --- src/renderers/shared/fiber/ReactFiber.js | 9 -- .../shared/fiber/ReactFiberBeginWork.js | 127 +++++++----------- 2 files changed, 47 insertions(+), 89 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiber.js b/src/renderers/shared/fiber/ReactFiber.js index 6cecc90b7cc..9703905cb6e 100644 --- a/src/renderers/shared/fiber/ReactFiber.js +++ b/src/renderers/shared/fiber/ReactFiber.js @@ -89,7 +89,6 @@ export type Fiber = Instance & { firstEffect: ?Fiber, lastEffect: ?Fiber, - // This will be used to quickly determine if a subtree has no pending changes. pendingWorkPriority: PriorityLevel, @@ -98,10 +97,6 @@ export type Fiber = Instance & { // memory if we need to. alternate: ?Fiber, - // Keeps track of the children that are currently being processed but have not - // yet completed. - childInProgress: ?Fiber, - // Conceptual aliases // workInProgress : Fiber -> alternate The alternate used for reuse happens // to be the same as work in progress. @@ -140,8 +135,6 @@ var createFiber = function(tag : TypeOfWork, key : null | string) : Fiber { pendingWorkPriority: NoWork, - childInProgress: null, - alternate: null, }; @@ -162,7 +155,6 @@ exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fi if (alt) { alt.stateNode = fiber.stateNode; alt.child = fiber.child; - alt.childInProgress = fiber.childInProgress; alt.sibling = fiber.sibling; alt.ref = fiber.ref; alt.pendingProps = fiber.pendingProps; @@ -185,7 +177,6 @@ exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fi alt.type = fiber.type; alt.stateNode = fiber.stateNode; alt.child = fiber.child; - alt.childInProgress = fiber.childInProgress; alt.sibling = fiber.sibling; alt.ref = fiber.ref; // pendingProps is here for symmetry but is unnecessary in practice for now. diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 7d48b0bc8af..9b2ec521fcc 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -40,34 +40,28 @@ var { module.exports = function(config : HostConfig) { function reconcileChildren(current, workInProgress, nextChildren) { - // TODO: Children needs to be able to reconcile in place if we are - // overriding progressed work. - const priority = workInProgress.pendingWorkPriority; - reconcileChildrenAtPriority(current, workInProgress, nextChildren, priority); - } - - function reconcileChildrenAtPriority(current, workInProgress, nextChildren, priorityLevel) { - if (current && current.childInProgress) { - workInProgress.childInProgress = reconcileChildFibersInPlace( - workInProgress, - current.childInProgress, - nextChildren, - priorityLevel - ); - // This is now invalid because we reused nodes. - current.childInProgress = null; - } else if (workInProgress.childInProgress) { - workInProgress.childInProgress = reconcileChildFibersInPlace( + const priorityLevel = workInProgress.pendingWorkPriority; + // At this point any memoization is no longer valid since we'll have changed + // the children. + workInProgress.memoizedProps = null; + if (current && current.child === workInProgress.child) { + // If the current child is the same as the work in progress, it means that + // we haven't yet started any work on these children. Therefore, we use + // the clone algorithm to create a copy of all the current children. + workInProgress.child = reconcileChildFibers( workInProgress, - workInProgress.childInProgress, + workInProgress.child, nextChildren, priorityLevel ); } else { - // TODO: - workInProgress.childInProgress = reconcileChildFibers( + // If, on the other hand, we don't have a current fiber or if it is + // already using a clone, that means we've already begun some work on this + // tree and we can continue where we left off by reconciling against the + // existing children. + workInProgress.child = reconcileChildFibersInPlace( workInProgress, - current ? current.child : workInProgress.child, + workInProgress.child, nextChildren, priorityLevel ); @@ -80,7 +74,7 @@ module.exports = function(config : HostConfig) { var nextChildren = fn(props); reconcileChildren(current, workInProgress, nextChildren); - return workInProgress.childInProgress; + return workInProgress.child; } function updateClassComponent(current : ?Fiber, workInProgress : Fiber) { @@ -97,13 +91,13 @@ module.exports = function(config : HostConfig) { return bailoutOnCurrent(current, workInProgress); } } - if (!workInProgress.childInProgress && workInProgress.memoizedProps) { + if (workInProgress.memoizedProps) { // Reset the props, in case this is a ping-pong case rather than a // completed update case. For the completed update case, the instance // props will already be the memoizedProps. instance.props = workInProgress.memoizedProps; if (!instance.shouldComponentUpdate(props)) { - return bailoutOnAlreadyFinishedWork(current, workInProgress); + // return bailoutOnAlreadyFinishedWork(current, workInProgress); } } } @@ -112,44 +106,20 @@ module.exports = function(config : HostConfig) { var nextChildren = instance.render(); reconcileChildren(current, workInProgress, nextChildren); - return workInProgress.childInProgress; + return workInProgress.child; } function updateHostComponent(current, workInProgress) { - var nextChildren = workInProgress.pendingProps.children; - - let priority = workInProgress.pendingWorkPriority; - if (workInProgress.pendingProps.hidden && priority !== OffscreenPriority) { - // If this host component is hidden, we can reconcile its children at - // the lowest priority and bail out from this particular pass. Unless, we're - // currently reconciling the lowest priority. - // If we have a child in progress already, we reconcile against that set - // to retain any work within it. We'll recreate any component that was in - // the current set and next set but not in the previous in progress set. - // TODO: This attaches a node that hasn't completed rendering so it - // becomes part of the render tree, even though it never completed. Its - // `output` property is unpredictable because of it. - reconcileChildrenAtPriority(current, workInProgress, nextChildren, OffscreenPriority); - workInProgress.child = current ? current.child : null; - let child = workInProgress.childInProgress; - while (child) { - const currentChild = child.alternate; - if (currentChild) { - child.child = currentChild.child; - child.childInProgress = currentChild.childInProgress; - child.memoizedProps = currentChild.memoizedProps; - child.output = currentChild.output; - } - child.nextEffect = null; - child.firstEffect = null; - child.lastEffect = null; - - child = child.sibling; - } - return null; + if (workInProgress.pendingProps.hidden && + workInProgress.pendingWorkPriority !== OffscreenPriority) { + // If this host component is hidden, we can bail out and ignore this. + // We'll rerender it later at the lower priority. + workInProgress.pendingWorkPriority = OffscreenPriority; + return bailoutOnLowPriority(current, workInProgress); } else { + var nextChildren = workInProgress.pendingProps.children; reconcileChildren(current, workInProgress, nextChildren); - return workInProgress.childInProgress; + return workInProgress.child; } } @@ -172,7 +142,7 @@ module.exports = function(config : HostConfig) { } } reconcileChildren(current, workInProgress, value); - return workInProgress.childInProgress; + return workInProgress.child; } function updateCoroutineComponent(current, workInProgress) { @@ -240,7 +210,6 @@ module.exports = function(config : HostConfig) { workInProgress.firstEffect = null; workInProgress.lastEffect = null; - workInProgress.childInProgress = null; // current.childInProgress; workInProgress.child = current.child; cloneChildFibers(workInProgress); @@ -263,9 +232,6 @@ module.exports = function(config : HostConfig) { workInProgress.lastEffect = null; const child = workInProgress.child; - if (workInProgress.childInProgress) { - throw new Error('Child in progress means we cannot bail here.'); - } if (child) { // Ensure that the effects of reused work are preserved. reuseChildrenEffects(workInProgress, child); @@ -280,6 +246,15 @@ module.exports = function(config : HostConfig) { return null; } + function bailoutOnLowPriority(current, workInProgress) { + if (current) { + workInProgress.child = current.child; + workInProgress.memoizedProps = current.memoizedProps; + workInProgress.output = current.output; + } + return null; + } + function beginWork(current : ?Fiber, workInProgress : Fiber, priorityLevel) : ?Fiber { if (!workInProgress.pendingProps) { throw new Error('should have pending props here'); @@ -287,15 +262,7 @@ module.exports = function(config : HostConfig) { if (workInProgress.pendingWorkPriority === NoWork || workInProgress.pendingWorkPriority > priorityLevel) { - - if (current) { - workInProgress.child = current.child; - workInProgress.childInProgress = current.childInProgress; - workInProgress.memoizedProps = current.memoizedProps; - workInProgress.output = current.output; - } - - return null; + return bailoutOnLowPriority(current, workInProgress); } // The current, flushed, state of this fiber is the alternate. @@ -306,7 +273,7 @@ module.exports = function(config : HostConfig) { return bailoutOnCurrent(current, workInProgress); } - if (!workInProgress.childInProgress && + if (workInProgress.memoizedProps && workInProgress.pendingProps === workInProgress.memoizedProps) { return bailoutOnAlreadyFinishedWork(current, workInProgress); } @@ -322,10 +289,10 @@ module.exports = function(config : HostConfig) { reconcileChildren(current, workInProgress, workInProgress.pendingProps); // A yield component is just a placeholder, we can just run through the // next one immediately. - if (workInProgress.childInProgress) { + if (workInProgress.child) { return beginWork( - workInProgress.childInProgress.alternate, - workInProgress.childInProgress + workInProgress.child.alternate, + workInProgress.child ); } return null; @@ -342,13 +309,13 @@ module.exports = function(config : HostConfig) { updateCoroutineComponent(current, workInProgress); // This doesn't take arbitrary time so we could synchronously just begin // eagerly do the work of workInProgress.child as an optimization. - if (workInProgress.childInProgress) { + if (workInProgress.child) { return beginWork( - workInProgress.childInProgress.alternate, - workInProgress.childInProgress + workInProgress.child.alternate, + workInProgress.child ); } - return workInProgress.childInProgress; + return workInProgress.child; case YieldComponent: // A yield component is just a placeholder, we can just run through the // next one immediately. From 6c77992b61b8160ffb70e13684a7f0f57c87e9c9 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Thu, 1 Sep 2016 20:56:27 -0700 Subject: [PATCH 04/21] Explain why ReactFiber is a POJO instead of constructor --- src/renderers/shared/fiber/ReactFiber.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/renderers/shared/fiber/ReactFiber.js b/src/renderers/shared/fiber/ReactFiber.js index 9703905cb6e..453ee56d5ff 100644 --- a/src/renderers/shared/fiber/ReactFiber.js +++ b/src/renderers/shared/fiber/ReactFiber.js @@ -103,6 +103,19 @@ export type Fiber = Instance & { }; +// This is a constructor of a POJO instead of a constructor function for a few +// reasons: +// 1) Nobody should add any instance methods on this. Instance methods can be +// more difficult to predict when they get optimized and they are almost +// never inlined properly in static compilers. +// 2) Nobody should rely on `instanceof Fiber` for type testing. We should +// always know when it is a fiber. +// 3) We can easily go from a createFiber call to calling a constructor if that +// is faster. The opposite is not true. +// 4) We might want to experiment with using numeric keys since they are easier +// to optimize in a non-JIT environment. +// 5) It should be easy to port this to a C struct and keep a C implementation +// compatible. var createFiber = function(tag : TypeOfWork, key : null | string) : Fiber { return { From dbd367d506cdb6a2e947f9ed5d0808abc51b480f Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Thu, 1 Sep 2016 20:58:21 -0700 Subject: [PATCH 05/21] Add shouldComponentUpdate to functional components I'll probably end up reverting this before the bigger release since it is not part of the official public API. However, it is useful to be able to compare the performance between functional components and classes. --- src/renderers/shared/fiber/ReactFiberBeginWork.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 9b2ec521fcc..ddcf88f32af 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -72,6 +72,16 @@ module.exports = function(config : HostConfig) { var fn = workInProgress.type; var props = workInProgress.pendingProps; + // TODO: Disable this before release, since it is not part of the public API + // I use this for testing to compare the relative overhead of classes. + if (typeof fn.shouldComponentUpdate === 'function') { + if (workInProgress.memoizedProps !== null) { + if (!fn.shouldComponentUpdate(workInProgress.memoizedProps, props)) { + return bailoutOnAlreadyFinishedWork(current, workInProgress); + } + } + } + var nextChildren = fn(props); reconcileChildren(current, workInProgress, nextChildren); return workInProgress.child; From fef8294b376b13d6ef56ef2c62a10752eebbb855 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Thu, 1 Sep 2016 21:04:44 -0700 Subject: [PATCH 06/21] Progressed work --- src/renderers/noop/ReactNoop.js | 16 +- src/renderers/shared/fiber/ReactChildFiber.js | 11 +- src/renderers/shared/fiber/ReactFiber.js | 34 +++- .../shared/fiber/ReactFiberBeginWork.js | 171 ++++++++---------- .../shared/fiber/ReactFiberCompleteWork.js | 8 +- .../shared/fiber/ReactFiberScheduler.js | 36 ++-- .../ReactIncrementalSideEffects-test.js | 79 ++++++++ 7 files changed, 213 insertions(+), 142 deletions(-) diff --git a/src/renderers/noop/ReactNoop.js b/src/renderers/noop/ReactNoop.js index 82e64b53579..390c4f6ac38 100644 --- a/src/renderers/noop/ReactNoop.js +++ b/src/renderers/noop/ReactNoop.js @@ -170,16 +170,12 @@ var ReactNoop = { ' '.repeat(depth) + '- ' + (fiber.type ? fiber.type.name || fiber.type : '[root]'), '[' + fiber.pendingWorkPriority + (fiber.pendingProps ? '*' : '') + ']' ); - const childInProgress = fiber.childInProgress; - if (childInProgress) { - if (childInProgress === fiber.child) { - console.log(' '.repeat(depth + 1) + 'ERROR: IN PROGRESS == CURRENT'); - } else { - console.log(' '.repeat(depth + 1) + 'IN PROGRESS'); - logFiber(childInProgress, depth + 1); - if (fiber.child) { - console.log(' '.repeat(depth + 1) + 'CURRENT'); - } + const childInProgress = fiber.progressedChild; + if (childInProgress && childInProgress !== fiber.child) { + console.log(' '.repeat(depth + 1) + 'IN PROGRESS: ' + fiber.progressedPriority); + logFiber(childInProgress, depth + 1); + if (fiber.child) { + console.log(' '.repeat(depth + 1) + 'CURRENT'); } } if (fiber.child) { diff --git a/src/renderers/shared/fiber/ReactChildFiber.js b/src/renderers/shared/fiber/ReactChildFiber.js index 8a3ad31d416..699b0e78830 100644 --- a/src/renderers/shared/fiber/ReactChildFiber.js +++ b/src/renderers/shared/fiber/ReactChildFiber.js @@ -67,7 +67,7 @@ function ChildReconciler(shouldClone) { clone.pendingWorkPriority = priority; } clone.pendingProps = element.props; - clone.child = existingChild.child; + // clone.child = existingChild.child; clone.sibling = null; clone.return = returnFiber; previousSibling.sibling = clone; @@ -139,7 +139,7 @@ function ChildReconciler(shouldClone) { clone.pendingWorkPriority = priority; } clone.pendingProps = element.props; - clone.child = existingChild.child; + // clone.child = existingChild.child; clone.sibling = null; clone.return = returnFiber; return clone; @@ -246,9 +246,6 @@ exports.cloneChildFibers = function(workInProgress : Fiber) { // If the children of the alternate fiber is a different set, then we don't // need to clone. We need to reset the return fiber though since we'll // traverse down into them. - // TODO: I don't think it is actually possible for them to be anything but - // equal at this point because this fiber was just cloned. Can we skip this - // check? Similar question about the return fiber. let child = workInProgress.child; while (child) { child.return = workInProgress; @@ -265,7 +262,7 @@ exports.cloneChildFibers = function(workInProgress : Fiber) { // than the next sibling. At that point we should add tests that catches // this. - const currentChild = current.child; + const currentChild = workInProgress.child; if (!currentChild) { return; } @@ -274,4 +271,4 @@ exports.cloneChildFibers = function(workInProgress : Fiber) { currentChild.pendingWorkPriority ); cloneSiblings(currentChild, workInProgress.child, workInProgress); -} +}; diff --git a/src/renderers/shared/fiber/ReactFiber.js b/src/renderers/shared/fiber/ReactFiber.js index 453ee56d5ff..967c6f78b07 100644 --- a/src/renderers/shared/fiber/ReactFiber.js +++ b/src/renderers/shared/fiber/ReactFiber.js @@ -92,6 +92,17 @@ export type Fiber = Instance & { // This will be used to quickly determine if a subtree has no pending changes. pendingWorkPriority: PriorityLevel, + // This value represents the priority level that was last used to process this + // component. This indicates whether it is better to continue from the + // progressed work or if it is better to continue from the current state. + progressedPriority: PriorityLevel, + + // If work bails out on a Fiber that already had some work started at a lower + // priority, then we need to store the progressed work somewhere. This holds + // the started child set until we need to get back to working on it. It may + // or may not be the same as the "current" child. + progressedChild: ?Fiber, + // This is a pooled version of a Fiber. Every fiber that gets updated will // eventually have a pair. There are cases when we can clean up pairs to save // memory if we need to. @@ -147,6 +158,8 @@ var createFiber = function(tag : TypeOfWork, key : null | string) : Fiber { lastEffect: null, pendingWorkPriority: NoWork, + progressedPriority: NoWork, + progressedChild: null, alternate: null, @@ -158,7 +171,16 @@ function shouldConstruct(Component) { } // This is used to create an alternate fiber to do work on. +// TODO: Rename to createWorkInProgressFiber or something like that. exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fiber { + // We clone to get a work in progress. That means that this fiber is the + // current. To make it safe to reuse that fiber later on as work in progress + // we need to reset its work in progress flag now. We don't have an + // opportunity to do this earlier since we don't traverse the tree when + // the work in progress tree becomes the current tree. + // fiber.progressedPriority = NoWork; + // fiber.progressedChild = null; + // We use a double buffering pooling technique because we know that we'll only // ever need at most two versions of a tree. We pool the "other" unused node // that we're free to reuse. This is lazily created to avoid allocating extra @@ -167,12 +189,12 @@ exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fi let alt = fiber.alternate; if (alt) { alt.stateNode = fiber.stateNode; - alt.child = fiber.child; - alt.sibling = fiber.sibling; + alt.sibling = fiber.sibling; // This should always be overridden. TODO: null alt.ref = fiber.ref; - alt.pendingProps = fiber.pendingProps; + alt.pendingProps = fiber.pendingProps; // TODO: Pass as argument. alt.pendingWorkPriority = priorityLevel; + alt.child = fiber.child; alt.memoizedProps = fiber.memoizedProps; alt.output = fiber.output; @@ -190,15 +212,19 @@ exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fi alt.type = fiber.type; alt.stateNode = fiber.stateNode; alt.child = fiber.child; - alt.sibling = fiber.sibling; + alt.sibling = fiber.sibling; // This should always be overridden. TODO: null alt.ref = fiber.ref; // pendingProps is here for symmetry but is unnecessary in practice for now. + // TODO: Pass in the new pendingProps as an argument maybe? alt.pendingProps = fiber.pendingProps; alt.pendingWorkPriority = priorityLevel; alt.memoizedProps = fiber.memoizedProps; alt.output = fiber.output; + alt.progressedChild = fiber.progressedChild; + alt.progressedPriority = fiber.progressedPriority; + alt.alternate = fiber; fiber.alternate = alt; return alt; diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index ddcf88f32af..feabc7838ba 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -15,6 +15,7 @@ import type { ReactCoroutine } from 'ReactCoroutine'; import type { Fiber } from 'ReactFiber'; import type { HostConfig } from 'ReactFiberReconciler'; +import type { PriorityLevel } from 'ReactPriorityLevel'; var { reconcileChildFibers, @@ -39,8 +40,24 @@ var { module.exports = function(config : HostConfig) { + function markChildAsProgressed(current, workInProgress, priorityLevel) { + // We now have clones. Let's store them as the currently progressed work. + workInProgress.progressedChild = workInProgress.child; + workInProgress.progressedPriority = priorityLevel; + if (current) { + // We also store it on the current. When the alternate swaps in we can + // continue from this point. + current.progressedChild = workInProgress.progressedChild; + current.progressedPriority = workInProgress.progressedPriority; + } + } + function reconcileChildren(current, workInProgress, nextChildren) { const priorityLevel = workInProgress.pendingWorkPriority; + reconcileChildrenAtPriority(current, workInProgress, nextChildren, priorityLevel); + } + + function reconcileChildrenAtPriority(current, workInProgress, nextChildren, priorityLevel) { // At this point any memoization is no longer valid since we'll have changed // the children. workInProgress.memoizedProps = null; @@ -66,6 +83,7 @@ module.exports = function(config : HostConfig) { priorityLevel ); } + markChildAsProgressed(current, workInProgress, priorityLevel); } function updateFunctionalComponent(current, workInProgress) { @@ -94,20 +112,13 @@ module.exports = function(config : HostConfig) { var ctor = workInProgress.type; workInProgress.stateNode = instance = new ctor(props); } else if (typeof instance.shouldComponentUpdate === 'function') { - if (current && current.memoizedProps) { - // Revert to the last flushed props, incase we aborted an update. - instance.props = current.memoizedProps; - if (!instance.shouldComponentUpdate(props)) { - return bailoutOnCurrent(current, workInProgress); - } - } - if (workInProgress.memoizedProps) { + if (workInProgress.memoizedProps !== null) { // Reset the props, in case this is a ping-pong case rather than a // completed update case. For the completed update case, the instance // props will already be the memoizedProps. instance.props = workInProgress.memoizedProps; if (!instance.shouldComponentUpdate(props)) { - // return bailoutOnAlreadyFinishedWork(current, workInProgress); + return bailoutOnAlreadyFinishedWork(current, workInProgress); } } } @@ -120,14 +131,29 @@ module.exports = function(config : HostConfig) { } function updateHostComponent(current, workInProgress) { + const nextChildren = workInProgress.pendingProps.children; if (workInProgress.pendingProps.hidden && workInProgress.pendingWorkPriority !== OffscreenPriority) { - // If this host component is hidden, we can bail out and ignore this. - // We'll rerender it later at the lower priority. - workInProgress.pendingWorkPriority = OffscreenPriority; - return bailoutOnLowPriority(current, workInProgress); + // If this host component is hidden, we can bail out on the children. + // We'll rerender the children later at the lower priority. + + // It is unfortunate that we have to do the reconciliation of these + // children already since that will add them to the tree even though + // they are not actually done yet. If this is a large set it is also + // confusing that this takes time to do right now instead of later. + + if (workInProgress.progressedPriority === OffscreenPriority) { + // If we already made some progress on the offscreen priority before, + // then we should continue from where we left off. + workInProgress.child = workInProgress.progressedChild; + } + + // Reconcile the children and stash them for later work. + reconcileChildrenAtPriority(current, workInProgress, nextChildren, OffscreenPriority); + workInProgress.child = current ? current.child : null; + // Abort and don't process children yet. + return null; } else { - var nextChildren = workInProgress.pendingProps.children; reconcileChildren(current, workInProgress, nextChildren); return workInProgress.child; } @@ -163,31 +189,7 @@ module.exports = function(config : HostConfig) { reconcileChildren(current, workInProgress, coroutine.children); } - function reuseChildren(returnFiber : Fiber, firstChild : Fiber) { - // TODO on the TODO: Is this not necessary anymore because I moved the - // priority reset? - // TODO: None of this should be necessary if structured better. - // The returnFiber pointer only needs to be updated when we walk into this child - // which we don't do right now. If the pending work priority indicated only - // if a child has work rather than if the node has work, then we would know - // by a single lookup on workInProgress rather than having to go through - // each child. - let child = firstChild; - do { - // Update the returnFiber of the child to the newest fiber. - child.return = returnFiber; - // Retain the priority if there's any work left to do in the children. - /*if (child.pendingWorkPriority !== NoWork && - (returnFiber.pendingWorkPriority === NoWork || - returnFiber.pendingWorkPriority > child.pendingWorkPriority)) { - returnFiber.pendingWorkPriority = child.pendingWorkPriority; - }*/ - if (!child.pendingProps && !child.memoizedProps) { - throw new Error('Should have memoized props by now'); - } - } while (child = child.sibling); - } - + /* function reuseChildrenEffects(returnFiber : Fiber, firstChild : Fiber) { let child = firstChild; do { @@ -204,57 +206,29 @@ module.exports = function(config : HostConfig) { } } while (child = child.sibling); } + */ -/* - function bailoutOnCurrent(current : Fiber, workInProgress : Fiber) : ?Fiber { - // The most likely scenario is that the previous copy of the tree contains - // the same props as the new one. In that case, we can just copy the output - // and children from that node. - workInProgress.memoizedProps = workInProgress.pendingProps; - workInProgress.output = current.output; + function bailoutOnAlreadyFinishedWork(current, workInProgress : Fiber) : ?Fiber { const priorityLevel = workInProgress.pendingWorkPriority; - // workInProgress.pendingProps = null; - workInProgress.stateNode = current.stateNode; - - workInProgress.nextEffect = null; - workInProgress.firstEffect = null; - workInProgress.lastEffect = null; - workInProgress.child = current.child; + // TODO: We should ideally be able to bail out early if the children have no + // more work to do. However, since we don't have a separation of this + // Fiber's priority and its children yet - we don't know without doing lots + // of the same work we do anyway. Once we have that separation we can just + // bail out here if the children has no more work at this priority level. + // if (workInProgress.priorityOfChildren <= priorityLevel) { + // // If there are side-effects in these children that have not yet been + // // committed we need to ensure that they get properly transferred up. + // if (current && current.child !== workInProgress.child) { + // reuseChildrenEffects(workInProgress, child); + // } + // return null; + // } cloneChildFibers(workInProgress); - - // TODO: Maybe bailout with null if the children priority flag indicate - // that there is no nested work. + markChildAsProgressed(current, workInProgress, priorityLevel); return workInProgress.child; } -*/ - - function bailoutOnAlreadyFinishedWork(current, workInProgress : Fiber) : ?Fiber { - // If we started this work before, and finished it, or if we're in a - // ping-pong update scenario, this version could already be what we're - // looking for. In that case, we should be able to just bail out. - const priorityLevel = workInProgress.pendingWorkPriority; - // workInProgress.pendingProps = null; - - workInProgress.firstEffect = null; - workInProgress.nextEffect = null; - workInProgress.lastEffect = null; - - const child = workInProgress.child; - if (child) { - // Ensure that the effects of reused work are preserved. - reuseChildrenEffects(workInProgress, child); - // If we bail out but still has work with the current priority in this - // subtree, we need to go find it right now. If we don't, we won't flush - // it until the next tick. - reuseChildren(workInProgress, child); - // TODO: Maybe bailout with null if the children priority flag indicate - // that there is no nested work. - return workInProgress.child; - } - return null; - } function bailoutOnLowPriority(current, workInProgress) { if (current) { @@ -265,26 +239,22 @@ module.exports = function(config : HostConfig) { return null; } - function beginWork(current : ?Fiber, workInProgress : Fiber, priorityLevel) : ?Fiber { - if (!workInProgress.pendingProps) { - throw new Error('should have pending props here'); - } - + function beginWork(current : ?Fiber, workInProgress : Fiber, priorityLevel : PriorityLevel) : ?Fiber { if (workInProgress.pendingWorkPriority === NoWork || workInProgress.pendingWorkPriority > priorityLevel) { return bailoutOnLowPriority(current, workInProgress); } - // The current, flushed, state of this fiber is the alternate. - // Ideally nothing should rely on this, but relying on it here - // means that we don't need an additional field on the work in - // progress. - if (current && workInProgress.pendingProps === current.memoizedProps) { - return bailoutOnCurrent(current, workInProgress); + if (workInProgress.progressedPriority === priorityLevel) { + // If we have progressed work on this priority level already, we can + // proceed this that as the child. + workInProgress.child = workInProgress.progressedChild; } - if (workInProgress.memoizedProps && - workInProgress.pendingProps === workInProgress.memoizedProps) { + if (workInProgress.pendingProps === null || ( + workInProgress.memoizedProps !== null && + workInProgress.pendingProps === workInProgress.memoizedProps + )) { return bailoutOnAlreadyFinishedWork(current, workInProgress); } @@ -302,7 +272,8 @@ module.exports = function(config : HostConfig) { if (workInProgress.child) { return beginWork( workInProgress.child.alternate, - workInProgress.child + workInProgress.child, + priorityLevel ); } return null; @@ -322,7 +293,8 @@ module.exports = function(config : HostConfig) { if (workInProgress.child) { return beginWork( workInProgress.child.alternate, - workInProgress.child + workInProgress.child, + priorityLevel ); } return workInProgress.child; @@ -332,7 +304,8 @@ module.exports = function(config : HostConfig) { if (workInProgress.sibling) { return beginWork( workInProgress.sibling.alternate, - workInProgress.sibling + workInProgress.sibling, + priorityLevel ); } return null; diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index 53421116b48..14e95a0d735 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -162,10 +162,16 @@ module.exports = function(config : HostConfig) { // This returns true if there was something to update. markForPreEffect(workInProgress); } + // TODO: Is this actually ever going to change? Why set it every time? workInProgress.output = instance; } else { if (!newProps) { - throw new Error('We must have new props for new mounts.'); + if (workInProgress.stateNode === null) { + throw new Error('We must have new props for new mounts.'); + } else { + // This can happen when we abort work. + return null; + } } const instance = createInstance(workInProgress.type, newProps, children); // TODO: This seems like unnecessary duplication. diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 96da4563514..057f9f7455e 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -25,9 +25,6 @@ var { cloneFiber } = require('ReactFiber'); var { NoWork, - HighPriority, - LowPriority, - OffscreenPriority, } = require('ReactPriorityLevel'); var timeHeuristicForUnitOfWork = 1; @@ -105,7 +102,10 @@ module.exports = function(config : HostConfig) { function resetWorkPriority(workInProgress : Fiber) { let newPriority = NoWork; - let child = workInProgress.child; + // progressedChild is going to be the child set with the highest priority. + // Either it is the same as child, or it just bailed out because it choose + // not to do the work. + let child = workInProgress.progressedChild; while (child) { // Ensure that remaining work priority bubbles up. if (child.pendingWorkPriority !== NoWork && @@ -125,25 +125,13 @@ module.exports = function(config : HostConfig) { // means that we don't need an additional field on the work in // progress. const current = workInProgress.alternate; - let next = null; + const next = completeWork(current, workInProgress); - // If this bailed at a lower priority. - // TODO: This branch is currently needed if a particular type of component - // ends up being a priority lowering. We should probably know that already - // before entering begin work. - if (workInProgress.pendingWorkPriority === NoWork || - workInProgress.pendingWorkPriority > nextPriorityLevel) { - // This fiber was ignored. We need to fall through to the next fiber - // and leave the pending props and work untouched on this fiber. - } else { - next = completeWork(current, workInProgress); - - resetWorkPriority(workInProgress); + resetWorkPriority(workInProgress); - // The work is now done. We don't need this anymore. This flags - // to the system not to redo any work here. - workInProgress.pendingProps = null; - } + // The work is now done. We don't need this anymore. This flags + // to the system not to redo any work here. + workInProgress.pendingProps = null; const returnFiber = workInProgress.return; @@ -175,6 +163,12 @@ module.exports = function(config : HostConfig) { } else { // If we're at the root, there's no more work to do. We can flush it. const root : FiberRoot = (workInProgress.stateNode : any); + if (root.current === workInProgress) { + throw new Error( + 'Cannot commit the same tree as before. This is probably a bug ' + + 'related to the return field.' + ); + } root.current = workInProgress; // TODO: We can be smarter here and only look for more work in the // "next" scheduled work since we've already scanned passed. That diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js index ebaa321a888..2fd25691567 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js @@ -270,6 +270,85 @@ describe('ReactIncrementalSideEffects', function() { ]); }); + it('can defer side-effects and resume them later on', function() { + class Bar extends React.Component { + shouldComponentUpdate(nextProps) { + return this.props.idx !== nextProps; + } + render() { + return ; + } + } + function Foo(props) { + return ( +
+ + +
+ ); + } + ReactNoop.render(); + ReactNoop.flushLowPri(40 + 25); + expect(ReactNoop.root.children).toEqual([ + div( + span(0), + div(/*the spans are down-prioritized and not rendered yet*/) + ), + ]); + ReactNoop.render(); + ReactNoop.flushLowPri(35 + 25); + expect(ReactNoop.root.children).toEqual([ + div( + span(1), + div(/*still not rendered yet*/) + ), + ]); + ReactNoop.flushLowPri(30 + 25); + expect(ReactNoop.root.children).toEqual([ + div( + span(1), + div( + // Now we had enough time to finish the spans. + span(0), + span(1) + ) + ), + ]); + var innerSpanA = ReactNoop.root.children[0].children[1].children[1]; + ReactNoop.render(); + ReactNoop.flushLowPri(30 + 25); + expect(ReactNoop.root.children).toEqual([ + div( + span(2), + div( + // Still same old numbers. + span(0), + span(1) + ) + ), + ]); + ReactNoop.flushLowPri(30); + expect(ReactNoop.root.children).toEqual([ + div( + span(2), + div( + // New numbers. + span(1), + span(2) + ) + ), + ]); + + var innerSpanB = ReactNoop.root.children[0].children[1].children[1]; + // This should have been an update to an existing instance, not recreation. + // We verify that by ensuring that the child instance was the same as + // before. + expect(innerSpanA).toBe(innerSpanB); + }); + // TODO: Test that side-effects are not cut off when a work in progress node // moves to "current" without flushing due to having lower priority. Does this From aa95b65a40fcbe6f49dd2a083e0e093ffddb7fcd Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Sat, 23 Jul 2016 18:59:56 -0700 Subject: [PATCH 07/21] setState for Fiber Updates are scheduled by setting a work priority on the fiber and bubbling it to the root. Because the instance does not know which tree is current at any given time, the update is scheduled on both fiber alternates. Need to add more unit tests to cover edge cases. --- src/renderers/shared/fiber/ReactFiber.js | 8 +++ .../shared/fiber/ReactFiberBeginWork.js | 66 ++++++++++++++++++- .../shared/fiber/ReactFiberCompleteWork.js | 1 + .../shared/fiber/ReactFiberScheduler.js | 19 +++++- .../fiber/__tests__/ReactIncremental-test.js | 32 +++++++++ 5 files changed, 121 insertions(+), 5 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiber.js b/src/renderers/shared/fiber/ReactFiber.js index 967c6f78b07..c2b8ea202a6 100644 --- a/src/renderers/shared/fiber/ReactFiber.js +++ b/src/renderers/shared/fiber/ReactFiber.js @@ -76,6 +76,10 @@ export type Fiber = Instance & { pendingProps: any, // This type will be more specific once we overload the tag. // TODO: I think that there is a way to merge pendingProps and memoizedProps. memoizedProps: any, // The props used to create the output. + // Local state for class components. May need better naming to disambiguate + // from stateNode. + pendingState: any, + memoizedState: any, // The state used to create the output. // Output is the return value of this fiber, or a linked list of return values // if this returns multiple values. Such as a fragment. output: any, // This type will be more specific once we overload the tag. @@ -151,6 +155,8 @@ var createFiber = function(tag : TypeOfWork, key : null | string) : Fiber { pendingProps: null, memoizedProps: null, + pendingState: null, + memoizedState: null, output: null, nextEffect: null, @@ -192,6 +198,7 @@ exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fi alt.sibling = fiber.sibling; // This should always be overridden. TODO: null alt.ref = fiber.ref; alt.pendingProps = fiber.pendingProps; // TODO: Pass as argument. + alt.pendingState = fiber.pendingState; alt.pendingWorkPriority = priorityLevel; alt.child = fiber.child; @@ -217,6 +224,7 @@ exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fi // pendingProps is here for symmetry but is unnecessary in practice for now. // TODO: Pass in the new pendingProps as an argument maybe? alt.pendingProps = fiber.pendingProps; + alt.pendingState = fiber.pendingState; alt.pendingWorkPriority = priorityLevel; alt.memoizedProps = fiber.memoizedProps; diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index feabc7838ba..45439a2bf1d 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -14,7 +14,9 @@ import type { ReactCoroutine } from 'ReactCoroutine'; import type { Fiber } from 'ReactFiber'; +import type { FiberRoot } from 'ReactFiberRoot'; import type { HostConfig } from 'ReactFiberReconciler'; +import type { Scheduler } from 'ReactFiberScheduler'; import type { PriorityLevel } from 'ReactPriorityLevel'; var { @@ -22,6 +24,7 @@ var { reconcileChildFibersInPlace, cloneChildFibers, } = require('ReactChildFiber'); +var { LowPriority } = require('ReactPriorityLevel'); var ReactTypeOfWork = require('ReactTypeOfWork'); var { IndeterminateComponent, @@ -38,7 +41,7 @@ var { OffscreenPriority, } = require('ReactPriorityLevel'); -module.exports = function(config : HostConfig) { +module.exports = function(config : HostConfig, getScheduler: () => Scheduler) { function markChildAsProgressed(current, workInProgress, priorityLevel) { // We now have clones. Let's store them as the currently progressed work. @@ -105,25 +108,81 @@ module.exports = function(config : HostConfig) { return workInProgress.child; } + function updateFiber(fiber: Fiber, state: any, priorityLevel : PriorityLevel): void { + const { scheduleLowPriWork } = getScheduler(); + fiber.pendingState = state; + + while (true) { + if (fiber.pendingWorkPriority === NoWork || + fiber.pendingWorkPriority >= priorityLevel) { + fiber.pendingWorkPriority = priorityLevel; + } + // Duck type root + if (fiber.stateNode && fiber.stateNode.containerInfo) { + const root : FiberRoot = (fiber.stateNode : any); + scheduleLowPriWork(root, priorityLevel); + return; + } + if (!fiber.return) { + throw new Error('No root!'); + } + fiber = fiber.return; + } + } + + // Class component state updater + const updater = { + enqueueSetState(instance, partialState) { + const fiber = instance._fiber; + + const prevState = fiber.pendingState || fiber.memoizedState; + const state = Object.assign({}, prevState, partialState); + + // Must schedule an update on both alternates, because we don't know tree + // is current. + updateFiber(fiber, state, LowPriority); + if (fiber.alternate) { + updateFiber(fiber.alternate, state, LowPriority); + } + }, + }; + function updateClassComponent(current : ?Fiber, workInProgress : Fiber) { + // A class component update is the result of either new props or new state. + // Account for the possibly of missing pending props or state by falling + // back to the most recent props or state. var props = workInProgress.pendingProps; + var state = workInProgress.pendingState; + if (!props && current) { + props = current.memoizedProps; + } + if (!state && current) { + state = current.memoizedState; + } + var instance = workInProgress.stateNode; if (!instance) { var ctor = workInProgress.type; workInProgress.stateNode = instance = new ctor(props); + state = workInProgress.pendingState = instance.state || null; + // The instance needs access to the fiber so that it can schedule updates + instance._fiber = workInProgress; + instance.updater = updater; } else if (typeof instance.shouldComponentUpdate === 'function') { if (workInProgress.memoizedProps !== null) { // Reset the props, in case this is a ping-pong case rather than a // completed update case. For the completed update case, the instance // props will already be the memoizedProps. instance.props = workInProgress.memoizedProps; - if (!instance.shouldComponentUpdate(props)) { + instance.state = workInProgress.memoizedState; + if (!instance.shouldComponentUpdate(props, state)) { return bailoutOnAlreadyFinishedWork(current, workInProgress); } } } instance.props = props; + instance.state = state; var nextChildren = instance.render(); reconcileChildren(current, workInProgress, nextChildren); @@ -253,7 +312,8 @@ module.exports = function(config : HostConfig) { if (workInProgress.pendingProps === null || ( workInProgress.memoizedProps !== null && - workInProgress.pendingProps === workInProgress.memoizedProps + workInProgress.pendingProps === workInProgress.memoizedProps && + workInProgress.pendingState === workInProgress.memoizedState )) { return bailoutOnAlreadyFinishedWork(current, workInProgress); } diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index 14e95a0d735..350135e9c3b 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -67,6 +67,7 @@ module.exports = function(config : HostConfig) { // the linked list of fibers that has the individual output values. returnFiber.output = (child && !child.sibling) ? child.output : child; returnFiber.memoizedProps = returnFiber.pendingProps; + returnFiber.memoizedState = returnFiber.pendingState; } function recursivelyFillYields(yields, output : ?Fiber | ?ReifiedYield) { diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 057f9f7455e..6a27414a52b 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -29,9 +29,19 @@ var { var timeHeuristicForUnitOfWork = 1; +export type Scheduler = { + scheduleLowPriWork: (root : FiberRoot, priority : PriorityLevel) => void +}; + 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. + let scheduler; + function getScheduler(): Scheduler { + return scheduler; + } - const { beginWork } = ReactFiberBeginWork(config); + const { beginWork } = ReactFiberBeginWork(config, getScheduler); const { completeWork } = ReactFiberCompleteWork(config); const { commitWork } = ReactFiberCommitWork(config); @@ -132,6 +142,10 @@ module.exports = function(config : HostConfig) { // The work is now done. We don't need this anymore. This flags // to the system not to redo any work here. workInProgress.pendingProps = null; + workInProgress.pendingState = null; + if (current) { + current.pendingState = null; + } const returnFiber = workInProgress.return; @@ -258,7 +272,8 @@ module.exports = function(config : HostConfig) { } */ - return { + scheduler = { scheduleLowPriWork: scheduleLowPriWork, }; + return scheduler; }; diff --git a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js index 303ef658bfc..9cce4f8e347 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js @@ -556,4 +556,36 @@ describe('ReactIncremental', function() { expect(ops).toEqual(['Content', 'Bar', 'Middle']); }); + + it('can update in the middle of a tree using setState', () => { + let instance; + let states = []; + + class Bar extends React.Component { + constructor() { + super(); + this.state = { string: 'a' }; + instance = this; + } + render() { + states.push(this.state.string); + return
{this.props.children}
; + } + } + + function Foo() { + return ( +
+ +
+ ); + } + + ReactNoop.render(); + ReactNoop.flush(); + expect(states).toEqual(['a']); + instance.setState({ string: 'b' }); + ReactNoop.flush(); + expect(states).toEqual(['a', 'b']); + }); }); From 0ed4b44198fcae8174d043f56998e948311c0410 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Sun, 24 Jul 2016 22:49:47 -0700 Subject: [PATCH 08/21] Use queue for pendingState Changes the type of pendingState to be a linked list of partial state objects. --- src/renderers/shared/fiber/ReactFiber.js | 10 ++-- .../shared/fiber/ReactFiberBeginWork.js | 52 +++++++++++++------ .../shared/fiber/ReactFiberCompleteWork.js | 5 +- .../shared/fiber/ReactFiberPendingState.js | 38 ++++++++++++++ .../fiber/__tests__/ReactIncremental-test.js | 40 +++++++++++--- 5 files changed, 119 insertions(+), 26 deletions(-) create mode 100644 src/renderers/shared/fiber/ReactFiberPendingState.js diff --git a/src/renderers/shared/fiber/ReactFiber.js b/src/renderers/shared/fiber/ReactFiber.js index c2b8ea202a6..e5ec0b6b091 100644 --- a/src/renderers/shared/fiber/ReactFiber.js +++ b/src/renderers/shared/fiber/ReactFiber.js @@ -15,6 +15,7 @@ import type { ReactCoroutine, ReactYield } from 'ReactCoroutine'; import type { TypeOfWork } from 'ReactTypeOfWork'; import type { PriorityLevel } from 'ReactPriorityLevel'; +import type { PendingState } from 'ReactFiberPendingState'; var ReactTypeOfWork = require('ReactTypeOfWork'); var { @@ -76,10 +77,11 @@ export type Fiber = Instance & { pendingProps: any, // This type will be more specific once we overload the tag. // TODO: I think that there is a way to merge pendingProps and memoizedProps. memoizedProps: any, // The props used to create the output. - // Local state for class components. May need better naming to disambiguate - // from stateNode. - pendingState: any, - memoizedState: any, // The state used to create the output. + // Local state for class components. Either null or a linked list of partial + // state objects. + pendingState: PendingState, + // The state used to create the output. This is a full state object. + memoizedState: any, // Output is the return value of this fiber, or a linked list of return values // if this returns multiple values. Such as a fragment. output: any, // This type will be more specific once we overload the tag. diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 45439a2bf1d..310ee556aa6 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -18,6 +18,7 @@ import type { FiberRoot } from 'ReactFiberRoot'; import type { HostConfig } from 'ReactFiberReconciler'; import type { Scheduler } from 'ReactFiberScheduler'; import type { PriorityLevel } from 'ReactPriorityLevel'; +import type { PendingState } from 'ReactFiberPendingState'; var { reconcileChildFibers, @@ -40,6 +41,10 @@ var { NoWork, OffscreenPriority, } = require('ReactPriorityLevel'); +var { + createPendingState, + mergePendingState, +} = require('ReactFiberPendingState'); module.exports = function(config : HostConfig, getScheduler: () => Scheduler) { @@ -108,10 +113,9 @@ module.exports = function(config : HostConfig, getSchedu return workInProgress.child; } - function updateFiber(fiber: Fiber, state: any, priorityLevel : PriorityLevel): void { + function scheduleUpdate(fiber: Fiber, pendingState: PendingState, priorityLevel : PriorityLevel): void { const { scheduleLowPriWork } = getScheduler(); - fiber.pendingState = state; - + fiber.pendingState = pendingState; while (true) { if (fiber.pendingWorkPriority === NoWork || fiber.pendingWorkPriority >= priorityLevel) { @@ -134,37 +138,55 @@ module.exports = function(config : HostConfig, getSchedu const updater = { enqueueSetState(instance, partialState) { const fiber = instance._fiber; - - const prevState = fiber.pendingState || fiber.memoizedState; - const state = Object.assign({}, prevState, partialState); + let pendingState = fiber.pendingState; + + // Append to pending state queue + const nextPendingStateNode = createPendingState(partialState); + if (pendingState === null) { + pendingState = nextPendingStateNode; + } else { + if (pendingState.tail === null) { + pendingState.next = nextPendingStateNode; + } else { + pendingState.tail.next = nextPendingStateNode; + } + pendingState.tail = nextPendingStateNode; + } // Must schedule an update on both alternates, because we don't know tree // is current. - updateFiber(fiber, state, LowPriority); + scheduleUpdate(fiber, pendingState, LowPriority); if (fiber.alternate) { - updateFiber(fiber.alternate, state, LowPriority); + scheduleUpdate(fiber.alternate, pendingState, LowPriority); } }, }; function updateClassComponent(current : ?Fiber, workInProgress : Fiber) { // A class component update is the result of either new props or new state. - // Account for the possibly of missing pending props or state by falling - // back to the most recent props or state. + // Account for the possibly of missing pending props by falling back to the + // memoized props. var props = workInProgress.pendingProps; - var state = workInProgress.pendingState; if (!props && current) { props = current.memoizedProps; } - if (!state && current) { - state = current.memoizedState; + // Compute the state using the memoized state and the pending state queue. + var pendingState = workInProgress.pendingState; + var state; + if (!current) { + state = mergePendingState(null, pendingState); + } else { + state = mergePendingState(current.memoizedState, pendingState); } var instance = workInProgress.stateNode; if (!instance) { var ctor = workInProgress.type; workInProgress.stateNode = instance = new ctor(props); - state = workInProgress.pendingState = instance.state || null; + state = instance.state || null; + // The initial state must be added to the pending state queue in case + // setState is called before the initial render. + workInProgress.pendingState = createPendingState(state); // The instance needs access to the fiber so that it can schedule updates instance._fiber = workInProgress; instance.updater = updater; @@ -313,7 +335,7 @@ module.exports = function(config : HostConfig, getSchedu if (workInProgress.pendingProps === null || ( workInProgress.memoizedProps !== null && workInProgress.pendingProps === workInProgress.memoizedProps && - workInProgress.pendingState === workInProgress.memoizedState + workInProgress.pendingState === null )) { return bailoutOnAlreadyFinishedWork(current, workInProgress); } diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index 350135e9c3b..10f0571186d 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -67,7 +67,6 @@ module.exports = function(config : HostConfig) { // the linked list of fibers that has the individual output values. returnFiber.output = (child && !child.sibling) ? child.output : child; returnFiber.memoizedProps = returnFiber.pendingProps; - returnFiber.memoizedState = returnFiber.pendingState; } function recursivelyFillYields(yields, output : ?Fiber | ?ReifiedYield) { @@ -133,6 +132,10 @@ module.exports = function(config : HostConfig) { return null; case ClassComponent: transferOutput(workInProgress.child, workInProgress); + // Don't use the pending state queue to compute the memoized state. We + // already merged it and assigned it to the instance. Copy it from there. + const state = workInProgress.stateNode.state; + workInProgress.memoizedState = state; return null; case HostContainer: transferOutput(workInProgress.child, workInProgress); diff --git a/src/renderers/shared/fiber/ReactFiberPendingState.js b/src/renderers/shared/fiber/ReactFiberPendingState.js new file mode 100644 index 00000000000..10c6615b7c7 --- /dev/null +++ b/src/renderers/shared/fiber/ReactFiberPendingState.js @@ -0,0 +1,38 @@ +/** + * 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. + * + * @providesModule ReactFiberPendingState + * @flow + */ + +'use strict'; + +export type PendingState = { + partialState: any, + next: PendingState, + tail: PendingState +} | null; + +exports.createPendingState = function(partialState: mixed): PendingState { + return { + partialState, + next: null, + tail: null, + }; +}; + +exports.mergePendingState = function(prevState: any, queue: PendingState): any { + if (queue === null) { + return prevState; + } + let state = Object.assign({}, prevState, queue.partialState); + while (queue = queue.next) { + state = Object.assign(state, queue.partialState); + } + return state; +}; diff --git a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js index 9cce4f8e347..b09cd373006 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js @@ -559,16 +559,42 @@ describe('ReactIncremental', function() { it('can update in the middle of a tree using setState', () => { let instance; - let states = []; + class Bar extends React.Component { + constructor() { + super(); + this.state = { a: 'a' }; + instance = this; + } + render() { + return
{this.props.children}
; + } + } + function Foo() { + return ( +
+ +
+ ); + } + + ReactNoop.render(); + ReactNoop.flush(); + expect(instance.state).toEqual({ a: 'a' }); + instance.setState({ b: 'b' }); + ReactNoop.flush(); + expect(instance.state).toEqual({ a: 'a', b: 'b' }); + }); + + it('can queue multiple state updates', () => { + let instance; class Bar extends React.Component { constructor() { super(); - this.state = { string: 'a' }; + this.state = { a: 'a' }; instance = this; } render() { - states.push(this.state.string); return
{this.props.children}
; } } @@ -583,9 +609,11 @@ describe('ReactIncremental', function() { ReactNoop.render(); ReactNoop.flush(); - expect(states).toEqual(['a']); - instance.setState({ string: 'b' }); + // Call setState multiple times before flushing + instance.setState({ b: 'b' }); + instance.setState({ c: 'c' }); + instance.setState({ d: 'd' }); ReactNoop.flush(); - expect(states).toEqual(['a', 'b']); + expect(instance.state).toEqual({ a: 'a', b: 'b', c: 'c', d: 'd' }); }); }); From 676d831ee61f29f4611f4113f154760dd413af62 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 25 Jul 2016 09:09:45 -0700 Subject: [PATCH 09/21] Updater form of setState Add support for setState((state, props) => newState). Rename pendingState to stateQueue. --- src/renderers/shared/fiber/ReactFiber.js | 13 +++-- .../shared/fiber/ReactFiberBeginWork.js | 43 +++++++--------- .../shared/fiber/ReactFiberCompleteWork.js | 4 +- .../shared/fiber/ReactFiberPendingState.js | 38 -------------- .../shared/fiber/ReactFiberScheduler.js | 4 +- .../shared/fiber/ReactFiberStateQueue.js | 51 +++++++++++++++++++ .../fiber/__tests__/ReactIncremental-test.js | 37 ++++++++++++++ 7 files changed, 117 insertions(+), 73 deletions(-) delete mode 100644 src/renderers/shared/fiber/ReactFiberPendingState.js create mode 100644 src/renderers/shared/fiber/ReactFiberStateQueue.js diff --git a/src/renderers/shared/fiber/ReactFiber.js b/src/renderers/shared/fiber/ReactFiber.js index e5ec0b6b091..33128d85dd5 100644 --- a/src/renderers/shared/fiber/ReactFiber.js +++ b/src/renderers/shared/fiber/ReactFiber.js @@ -15,7 +15,7 @@ import type { ReactCoroutine, ReactYield } from 'ReactCoroutine'; import type { TypeOfWork } from 'ReactTypeOfWork'; import type { PriorityLevel } from 'ReactPriorityLevel'; -import type { PendingState } from 'ReactFiberPendingState'; +import type { StateQueue } from 'ReactFiberStateQueue'; var ReactTypeOfWork = require('ReactTypeOfWork'); var { @@ -77,9 +77,8 @@ export type Fiber = Instance & { pendingProps: any, // This type will be more specific once we overload the tag. // TODO: I think that there is a way to merge pendingProps and memoizedProps. memoizedProps: any, // The props used to create the output. - // Local state for class components. Either null or a linked list of partial - // state objects. - pendingState: PendingState, + // A queue of local state updates. + stateQueue: StateQueue, // The state used to create the output. This is a full state object. memoizedState: any, // Output is the return value of this fiber, or a linked list of return values @@ -157,7 +156,7 @@ var createFiber = function(tag : TypeOfWork, key : null | string) : Fiber { pendingProps: null, memoizedProps: null, - pendingState: null, + stateQueue: null, memoizedState: null, output: null, @@ -200,7 +199,7 @@ exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fi alt.sibling = fiber.sibling; // This should always be overridden. TODO: null alt.ref = fiber.ref; alt.pendingProps = fiber.pendingProps; // TODO: Pass as argument. - alt.pendingState = fiber.pendingState; + alt.stateQueue = fiber.stateQueue; alt.pendingWorkPriority = priorityLevel; alt.child = fiber.child; @@ -226,7 +225,7 @@ exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fi // pendingProps is here for symmetry but is unnecessary in practice for now. // TODO: Pass in the new pendingProps as an argument maybe? alt.pendingProps = fiber.pendingProps; - alt.pendingState = fiber.pendingState; + alt.stateQueue = fiber.stateQueue; alt.pendingWorkPriority = priorityLevel; alt.memoizedProps = fiber.memoizedProps; diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 310ee556aa6..eda478c126f 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -18,7 +18,7 @@ import type { FiberRoot } from 'ReactFiberRoot'; import type { HostConfig } from 'ReactFiberReconciler'; import type { Scheduler } from 'ReactFiberScheduler'; import type { PriorityLevel } from 'ReactPriorityLevel'; -import type { PendingState } from 'ReactFiberPendingState'; +import type { StateQueue } from 'ReactFiberStateQueue'; var { reconcileChildFibers, @@ -42,11 +42,12 @@ var { OffscreenPriority, } = require('ReactPriorityLevel'); var { - createPendingState, - mergePendingState, -} = require('ReactFiberPendingState'); + createStateQueue, + addToQueue, + mergeStateQueue, +} = require('ReactFiberStateQueue'); -module.exports = function(config : HostConfig, getScheduler: () => Scheduler) { +module.exports = function(config : HostConfig, getScheduler : () => Scheduler) { function markChildAsProgressed(current, workInProgress, priorityLevel) { // We now have clones. Let's store them as the currently progressed work. @@ -113,9 +114,9 @@ module.exports = function(config : HostConfig, getSchedu return workInProgress.child; } - function scheduleUpdate(fiber: Fiber, pendingState: PendingState, priorityLevel : PriorityLevel): void { + function scheduleUpdate(fiber: Fiber, stateQueue: StateQueue, priorityLevel : PriorityLevel): void { const { scheduleLowPriWork } = getScheduler(); - fiber.pendingState = pendingState; + fiber.stateQueue = stateQueue; while (true) { if (fiber.pendingWorkPriority === NoWork || fiber.pendingWorkPriority >= priorityLevel) { @@ -138,26 +139,20 @@ module.exports = function(config : HostConfig, getSchedu const updater = { enqueueSetState(instance, partialState) { const fiber = instance._fiber; - let pendingState = fiber.pendingState; + let stateQueue = fiber.stateQueue; // Append to pending state queue - const nextPendingStateNode = createPendingState(partialState); - if (pendingState === null) { - pendingState = nextPendingStateNode; + if (stateQueue === null) { + stateQueue = createStateQueue(partialState); } else { - if (pendingState.tail === null) { - pendingState.next = nextPendingStateNode; - } else { - pendingState.tail.next = nextPendingStateNode; - } - pendingState.tail = nextPendingStateNode; + addToQueue(stateQueue, partialState); } // Must schedule an update on both alternates, because we don't know tree // is current. - scheduleUpdate(fiber, pendingState, LowPriority); + scheduleUpdate(fiber, stateQueue, LowPriority); if (fiber.alternate) { - scheduleUpdate(fiber.alternate, pendingState, LowPriority); + scheduleUpdate(fiber.alternate, stateQueue, LowPriority); } }, }; @@ -171,12 +166,12 @@ module.exports = function(config : HostConfig, getSchedu props = current.memoizedProps; } // Compute the state using the memoized state and the pending state queue. - var pendingState = workInProgress.pendingState; + var stateQueue = workInProgress.stateQueue; var state; if (!current) { - state = mergePendingState(null, pendingState); + state = mergeStateQueue(null, props, stateQueue); } else { - state = mergePendingState(current.memoizedState, pendingState); + state = mergeStateQueue(current.memoizedState, props, stateQueue); } var instance = workInProgress.stateNode; @@ -186,7 +181,7 @@ module.exports = function(config : HostConfig, getSchedu state = instance.state || null; // The initial state must be added to the pending state queue in case // setState is called before the initial render. - workInProgress.pendingState = createPendingState(state); + workInProgress.stateQueue = createStateQueue(state); // The instance needs access to the fiber so that it can schedule updates instance._fiber = workInProgress; instance.updater = updater; @@ -335,7 +330,7 @@ module.exports = function(config : HostConfig, getSchedu if (workInProgress.pendingProps === null || ( workInProgress.memoizedProps !== null && workInProgress.pendingProps === workInProgress.memoizedProps && - workInProgress.pendingState === null + workInProgress.stateQueue === null )) { return bailoutOnAlreadyFinishedWork(current, workInProgress); } diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index 10f0571186d..fdeb1f5e56c 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -132,8 +132,8 @@ module.exports = function(config : HostConfig) { return null; case ClassComponent: transferOutput(workInProgress.child, workInProgress); - // Don't use the pending state queue to compute the memoized state. We - // already merged it and assigned it to the instance. Copy it from there. + // Don't use the state queue to compute the memoized state. We already + // merged it and assigned it to the instance. Copy it from there. const state = workInProgress.stateNode.state; workInProgress.memoizedState = state; return null; diff --git a/src/renderers/shared/fiber/ReactFiberPendingState.js b/src/renderers/shared/fiber/ReactFiberPendingState.js deleted file mode 100644 index 10c6615b7c7..00000000000 --- a/src/renderers/shared/fiber/ReactFiberPendingState.js +++ /dev/null @@ -1,38 +0,0 @@ -/** - * 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. - * - * @providesModule ReactFiberPendingState - * @flow - */ - -'use strict'; - -export type PendingState = { - partialState: any, - next: PendingState, - tail: PendingState -} | null; - -exports.createPendingState = function(partialState: mixed): PendingState { - return { - partialState, - next: null, - tail: null, - }; -}; - -exports.mergePendingState = function(prevState: any, queue: PendingState): any { - if (queue === null) { - return prevState; - } - let state = Object.assign({}, prevState, queue.partialState); - while (queue = queue.next) { - state = Object.assign(state, queue.partialState); - } - return state; -}; diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 6a27414a52b..ef99eb262d3 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -142,9 +142,9 @@ module.exports = function(config : HostConfig) { // The work is now done. We don't need this anymore. This flags // to the system not to redo any work here. workInProgress.pendingProps = null; - workInProgress.pendingState = null; + workInProgress.stateQueue = null; if (current) { - current.pendingState = null; + current.stateQueue = null; } const returnFiber = workInProgress.return; diff --git a/src/renderers/shared/fiber/ReactFiberStateQueue.js b/src/renderers/shared/fiber/ReactFiberStateQueue.js new file mode 100644 index 00000000000..e8d6c7385e2 --- /dev/null +++ b/src/renderers/shared/fiber/ReactFiberStateQueue.js @@ -0,0 +1,51 @@ +/** + * 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. + * + * @providesModule ReactFiberStateQueue + * @flow + */ + +'use strict'; + +export type StateQueue = { + partialState: any, + next: StateQueue, + tail: StateQueue +} | null; + +exports.createStateQueue = function(partialState : mixed) : StateQueue { + return { + partialState, + next: null, + tail: null, + }; +}; + +exports.addToQueue = function(queue : StateQueue, partialState : mixed) { + const node = exports.createStateQueue(partialState); + if (queue.tail === null) { + queue.next = node; + } else { + queue.tail.next = node; + } + queue.tail = node; +} + +exports.mergeStateQueue = function(prevState : any, props : any, queue : StateQueue) : any { + if (queue === null) { + return prevState; + } + let state = Object.assign({}, prevState); + do { + const partialState = typeof queue.partialState === 'function' ? + queue.partialState(state, props) : + queue.partialState; + state = Object.assign(state, partialState); + } while (queue = queue.next); + return state; +}; diff --git a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js index b09cd373006..f9e5f08b906 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js @@ -616,4 +616,41 @@ describe('ReactIncremental', function() { ReactNoop.flush(); expect(instance.state).toEqual({ a: 'a', b: 'b', c: 'c', d: 'd' }); }); + + it('can use updater form of setState', () => { + let instance; + class Bar extends React.Component { + constructor() { + super(); + this.state = { num: 1 }; + instance = this; + } + render() { + return
{this.props.children}
; + } + } + + function Foo({ multiplier }) { + return ( +
+ +
+ ); + } + + function updater(state, props) { + return { num: state.num * props.multiplier }; + } + + ReactNoop.render(); + ReactNoop.flush(); + expect(instance.state.num).toEqual(1); + instance.setState(updater); + ReactNoop.flush(); + expect(instance.state.num).toEqual(2); + ReactNoop.render(); + instance.setState(updater); + ReactNoop.flush(); + expect(instance.state.num).toEqual(6); + }); }); From 1ba89ef4e8abfd5e2ff27f1958b40f0b2d836dbe Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 25 Jul 2016 09:27:59 -0700 Subject: [PATCH 10/21] Clean up Rather than bubble up both trees, bubble up once and assign to the alternate at each level. Extract logic for adding to the queue to the StateQueue module. --- .../shared/fiber/ReactFiberBeginWork.js | 28 +++++++++---------- .../shared/fiber/ReactFiberStateQueue.js | 8 ++++-- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index eda478c126f..a5b4ccb57ca 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -117,11 +117,24 @@ module.exports = function(config : HostConfig, getSchedu function scheduleUpdate(fiber: Fiber, stateQueue: StateQueue, priorityLevel : PriorityLevel): void { const { scheduleLowPriWork } = getScheduler(); fiber.stateQueue = stateQueue; + // Schedule update on the alternate as well, since we don't know which tree + // is current. + // $FlowFixMe: Intersection issue. Don't know why it's only happening here. + const { alternate } = fiber; + if (alternate !== null) { + alternate.stateQueue = stateQueue; + } while (true) { if (fiber.pendingWorkPriority === NoWork || fiber.pendingWorkPriority >= priorityLevel) { fiber.pendingWorkPriority = priorityLevel; } + if (alternate !== null) { + if (alternate.pendingWorkPriority === NoWork || + alternate.pendingWorkPriority >= priorityLevel) { + alternate.pendingWorkPriority = priorityLevel; + } + } // Duck type root if (fiber.stateNode && fiber.stateNode.containerInfo) { const root : FiberRoot = (fiber.stateNode : any); @@ -139,21 +152,8 @@ module.exports = function(config : HostConfig, getSchedu const updater = { enqueueSetState(instance, partialState) { const fiber = instance._fiber; - let stateQueue = fiber.stateQueue; - - // Append to pending state queue - if (stateQueue === null) { - stateQueue = createStateQueue(partialState); - } else { - addToQueue(stateQueue, partialState); - } - - // Must schedule an update on both alternates, because we don't know tree - // is current. + const stateQueue = addToQueue(fiber.stateQueue, partialState); scheduleUpdate(fiber, stateQueue, LowPriority); - if (fiber.alternate) { - scheduleUpdate(fiber.alternate, stateQueue, LowPriority); - } }, }; diff --git a/src/renderers/shared/fiber/ReactFiberStateQueue.js b/src/renderers/shared/fiber/ReactFiberStateQueue.js index e8d6c7385e2..9ba30633611 100644 --- a/src/renderers/shared/fiber/ReactFiberStateQueue.js +++ b/src/renderers/shared/fiber/ReactFiberStateQueue.js @@ -26,15 +26,19 @@ exports.createStateQueue = function(partialState : mixed) : StateQueue { }; }; -exports.addToQueue = function(queue : StateQueue, partialState : mixed) { +exports.addToQueue = function(queue : StateQueue, partialState : mixed): StateQueue { const node = exports.createStateQueue(partialState); + if (queue === null) { + return node; + } if (queue.tail === null) { queue.next = node; } else { queue.tail.next = node; } queue.tail = node; -} + return queue; +}; exports.mergeStateQueue = function(prevState : any, props : any, queue : StateQueue) : any { if (queue === null) { From d0e805cde803d7926f321354c4748e83977b9256 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 25 Jul 2016 09:36:08 -0700 Subject: [PATCH 11/21] Fix stateQueue typing --- src/renderers/shared/fiber/ReactFiber.js | 2 +- src/renderers/shared/fiber/ReactFiberBeginWork.js | 8 ++++++-- src/renderers/shared/fiber/ReactFiberStateQueue.js | 13 +++++-------- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiber.js b/src/renderers/shared/fiber/ReactFiber.js index 33128d85dd5..d6dc2bf3375 100644 --- a/src/renderers/shared/fiber/ReactFiber.js +++ b/src/renderers/shared/fiber/ReactFiber.js @@ -78,7 +78,7 @@ export type Fiber = Instance & { // TODO: I think that there is a way to merge pendingProps and memoizedProps. memoizedProps: any, // The props used to create the output. // A queue of local state updates. - stateQueue: StateQueue, + stateQueue: ?StateQueue, // The state used to create the output. This is a full state object. memoizedState: any, // Output is the return value of this fiber, or a linked list of return values diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index a5b4ccb57ca..f763409a7f1 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -152,7 +152,9 @@ module.exports = function(config : HostConfig, getSchedu const updater = { enqueueSetState(instance, partialState) { const fiber = instance._fiber; - const stateQueue = addToQueue(fiber.stateQueue, partialState); + const stateQueue = fiber.stateQueue ? + addToQueue(fiber.stateQueue, partialState) : + createStateQueue(partialState); scheduleUpdate(fiber, stateQueue, LowPriority); }, }; @@ -181,7 +183,9 @@ module.exports = function(config : HostConfig, getSchedu state = instance.state || null; // The initial state must be added to the pending state queue in case // setState is called before the initial render. - workInProgress.stateQueue = createStateQueue(state); + if (state !== null) { + workInProgress.stateQueue = createStateQueue(state); + } // The instance needs access to the fiber so that it can schedule updates instance._fiber = workInProgress; instance.updater = updater; diff --git a/src/renderers/shared/fiber/ReactFiberStateQueue.js b/src/renderers/shared/fiber/ReactFiberStateQueue.js index 9ba30633611..59320028107 100644 --- a/src/renderers/shared/fiber/ReactFiberStateQueue.js +++ b/src/renderers/shared/fiber/ReactFiberStateQueue.js @@ -14,9 +14,9 @@ export type StateQueue = { partialState: any, - next: StateQueue, - tail: StateQueue -} | null; + next: StateQueue | null, + tail: StateQueue | null +}; exports.createStateQueue = function(partialState : mixed) : StateQueue { return { @@ -28,9 +28,6 @@ exports.createStateQueue = function(partialState : mixed) : StateQueue { exports.addToQueue = function(queue : StateQueue, partialState : mixed): StateQueue { const node = exports.createStateQueue(partialState); - if (queue === null) { - return node; - } if (queue.tail === null) { queue.next = node; } else { @@ -40,8 +37,8 @@ exports.addToQueue = function(queue : StateQueue, partialState : mixed): StateQu return queue; }; -exports.mergeStateQueue = function(prevState : any, props : any, queue : StateQueue) : any { - if (queue === null) { +exports.mergeStateQueue = function(prevState : any, props : any, queue : ?StateQueue) : any { + if (!queue) { return prevState; } let state = Object.assign({}, prevState); From af9ab5c8a5197e76142a2e83811b6d25b72f5a6d Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Tue, 26 Jul 2016 10:28:45 -0700 Subject: [PATCH 12/21] Clean up Use a union type for the head of StateQueue. --- .../shared/fiber/ReactFiberBeginWork.js | 23 +++++-------- .../shared/fiber/ReactFiberStateQueue.js | 34 ++++++++++++------- .../fiber/__tests__/ReactIncremental-test.js | 2 +- 3 files changed, 32 insertions(+), 27 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index f763409a7f1..32068740df8 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -119,20 +119,18 @@ module.exports = function(config : HostConfig, getSchedu fiber.stateQueue = stateQueue; // Schedule update on the alternate as well, since we don't know which tree // is current. - // $FlowFixMe: Intersection issue. Don't know why it's only happening here. - const { alternate } = fiber; - if (alternate !== null) { - alternate.stateQueue = stateQueue; + if (fiber.alternate !== null) { + fiber.alternate.stateQueue = stateQueue; } while (true) { if (fiber.pendingWorkPriority === NoWork || fiber.pendingWorkPriority >= priorityLevel) { fiber.pendingWorkPriority = priorityLevel; } - if (alternate !== null) { - if (alternate.pendingWorkPriority === NoWork || - alternate.pendingWorkPriority >= priorityLevel) { - alternate.pendingWorkPriority = priorityLevel; + if (fiber.alternate !== null) { + if (fiber.alternate.pendingWorkPriority === NoWork || + fiber.alternate.pendingWorkPriority >= priorityLevel) { + fiber.alternate.pendingWorkPriority = priorityLevel; } } // Duck type root @@ -169,12 +167,9 @@ module.exports = function(config : HostConfig, getSchedu } // Compute the state using the memoized state and the pending state queue. var stateQueue = workInProgress.stateQueue; - var state; - if (!current) { - state = mergeStateQueue(null, props, stateQueue); - } else { - state = mergeStateQueue(current.memoizedState, props, stateQueue); - } + var state = current ? + mergeStateQueue(stateQueue, current.memoizedState, props) : + mergeStateQueue(stateQueue, null, props); var instance = workInProgress.stateNode; if (!instance) { diff --git a/src/renderers/shared/fiber/ReactFiberStateQueue.js b/src/renderers/shared/fiber/ReactFiberStateQueue.js index 59320028107..beceeb07dd2 100644 --- a/src/renderers/shared/fiber/ReactFiberStateQueue.js +++ b/src/renderers/shared/fiber/ReactFiberStateQueue.js @@ -12,23 +12,32 @@ 'use strict'; -export type StateQueue = { +type StateQueueNode = { partialState: any, - next: StateQueue | null, - tail: StateQueue | null + callback: ?Function, + next: ?StateQueueNode, +}; + +export type StateQueue = StateQueueNode & { + tail: ?StateQueueNode }; exports.createStateQueue = function(partialState : mixed) : StateQueue { return { partialState, + callback: null, next: null, tail: null, }; }; -exports.addToQueue = function(queue : StateQueue, partialState : mixed): StateQueue { - const node = exports.createStateQueue(partialState); - if (queue.tail === null) { +exports.addToQueue = function(queue : StateQueue, partialState : mixed) : StateQueue { + const node = { + partialState, + callback: null, + next: null, + }; + if (!queue.tail) { queue.next = node; } else { queue.tail.next = node; @@ -37,16 +46,17 @@ exports.addToQueue = function(queue : StateQueue, partialState : mixed): StateQu return queue; }; -exports.mergeStateQueue = function(prevState : any, props : any, queue : ?StateQueue) : any { - if (!queue) { +exports.mergeStateQueue = function(queue : ?StateQueue, prevState : any, props : any) : any { + let node : ?StateQueueNode = queue; + if (!node) { return prevState; } let state = Object.assign({}, prevState); do { - const partialState = typeof queue.partialState === 'function' ? - queue.partialState(state, props) : - queue.partialState; + const partialState = typeof node.partialState === 'function' ? + node.partialState(state, props) : + node.partialState; state = Object.assign(state, partialState); - } while (queue = queue.next); + } while (node = node.next); return state; }; diff --git a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js index f9e5f08b906..d16fe5a2ee3 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js @@ -648,8 +648,8 @@ describe('ReactIncremental', function() { instance.setState(updater); ReactNoop.flush(); expect(instance.state.num).toEqual(2); - ReactNoop.render(); instance.setState(updater); + ReactNoop.render(); ReactNoop.flush(); expect(instance.state.num).toEqual(6); }); From 2065ee32163e7042e1f1dd6bebadc784ed807f8a Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Tue, 26 Jul 2016 13:23:11 -0700 Subject: [PATCH 13/21] Use ReactInstanceMap Move ReactInstanceMap to src/renderers/shared/shared to indicate that this logic is shared across implementations. --- src/renderers/shared/fiber/ReactFiberBeginWork.js | 5 +++-- .../shared/{stack/reconciler => shared}/ReactInstanceMap.js | 0 2 files changed, 3 insertions(+), 2 deletions(-) rename src/renderers/shared/{stack/reconciler => shared}/ReactInstanceMap.js (100%) diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 32068740df8..2d0afdf0415 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -46,6 +46,7 @@ var { addToQueue, mergeStateQueue, } = require('ReactFiberStateQueue'); +var ReactInstanceMap = require('ReactInstanceMap'); module.exports = function(config : HostConfig, getScheduler : () => Scheduler) { @@ -149,7 +150,7 @@ module.exports = function(config : HostConfig, getSchedu // Class component state updater const updater = { enqueueSetState(instance, partialState) { - const fiber = instance._fiber; + const fiber = ReactInstanceMap.get(instance); const stateQueue = fiber.stateQueue ? addToQueue(fiber.stateQueue, partialState) : createStateQueue(partialState); @@ -182,7 +183,7 @@ module.exports = function(config : HostConfig, getSchedu workInProgress.stateQueue = createStateQueue(state); } // The instance needs access to the fiber so that it can schedule updates - instance._fiber = workInProgress; + ReactInstanceMap.set(instance, workInProgress); instance.updater = updater; } else if (typeof instance.shouldComponentUpdate === 'function') { if (workInProgress.memoizedProps !== null) { diff --git a/src/renderers/shared/stack/reconciler/ReactInstanceMap.js b/src/renderers/shared/shared/ReactInstanceMap.js similarity index 100% rename from src/renderers/shared/stack/reconciler/ReactInstanceMap.js rename to src/renderers/shared/shared/ReactInstanceMap.js From 5eb2c05a5f1b2cce6573f85f7119f8b6273aac85 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Wed, 27 Jul 2016 13:07:08 -0700 Subject: [PATCH 14/21] Ensure that setState update function's context is undefined --- src/renderers/shared/fiber/ReactFiberStateQueue.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberStateQueue.js b/src/renderers/shared/fiber/ReactFiberStateQueue.js index beceeb07dd2..d410f8d0d69 100644 --- a/src/renderers/shared/fiber/ReactFiberStateQueue.js +++ b/src/renderers/shared/fiber/ReactFiberStateQueue.js @@ -53,9 +53,13 @@ exports.mergeStateQueue = function(queue : ?StateQueue, prevState : any, props : } let state = Object.assign({}, prevState); do { - const partialState = typeof node.partialState === 'function' ? - node.partialState(state, props) : - node.partialState; + let partialState; + if (typeof node.partialState === 'function') { + const updateFn = node.partialState; + partialState = updateFn(state, props); + } else { + partialState = node.partialState; + } state = Object.assign(state, partialState); } while (node = node.next); return state; From 3121e8ffd41fa6da5cb0ceb7fc8528d321042c0e Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Thu, 4 Aug 2016 14:27:55 -0700 Subject: [PATCH 15/21] Rename stateQueue -> updateQueue Also cleans up some types. --- src/renderers/shared/fiber/ReactFiber.js | 10 ++-- .../shared/fiber/ReactFiberBeginWork.js | 39 +++++++-------- .../shared/fiber/ReactFiberScheduler.js | 4 +- ...StateQueue.js => ReactFiberUpdateQueue.js} | 47 +++++++++++-------- 4 files changed, 54 insertions(+), 46 deletions(-) rename src/renderers/shared/fiber/{ReactFiberStateQueue.js => ReactFiberUpdateQueue.js} (51%) diff --git a/src/renderers/shared/fiber/ReactFiber.js b/src/renderers/shared/fiber/ReactFiber.js index d6dc2bf3375..495c615c95e 100644 --- a/src/renderers/shared/fiber/ReactFiber.js +++ b/src/renderers/shared/fiber/ReactFiber.js @@ -15,7 +15,7 @@ import type { ReactCoroutine, ReactYield } from 'ReactCoroutine'; import type { TypeOfWork } from 'ReactTypeOfWork'; import type { PriorityLevel } from 'ReactPriorityLevel'; -import type { StateQueue } from 'ReactFiberStateQueue'; +import type { UpdateQueue } from 'ReactFiberUpdateQueue'; var ReactTypeOfWork = require('ReactTypeOfWork'); var { @@ -78,7 +78,7 @@ export type Fiber = Instance & { // TODO: I think that there is a way to merge pendingProps and memoizedProps. memoizedProps: any, // The props used to create the output. // A queue of local state updates. - stateQueue: ?StateQueue, + updateQueue: ?UpdateQueue, // The state used to create the output. This is a full state object. memoizedState: any, // Output is the return value of this fiber, or a linked list of return values @@ -156,7 +156,7 @@ var createFiber = function(tag : TypeOfWork, key : null | string) : Fiber { pendingProps: null, memoizedProps: null, - stateQueue: null, + updateQueue: null, memoizedState: null, output: null, @@ -199,7 +199,7 @@ exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fi alt.sibling = fiber.sibling; // This should always be overridden. TODO: null alt.ref = fiber.ref; alt.pendingProps = fiber.pendingProps; // TODO: Pass as argument. - alt.stateQueue = fiber.stateQueue; + alt.updateQueue = fiber.updateQueue; alt.pendingWorkPriority = priorityLevel; alt.child = fiber.child; @@ -225,7 +225,7 @@ exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fi // pendingProps is here for symmetry but is unnecessary in practice for now. // TODO: Pass in the new pendingProps as an argument maybe? alt.pendingProps = fiber.pendingProps; - alt.stateQueue = fiber.stateQueue; + alt.updateQueue = fiber.updateQueue; alt.pendingWorkPriority = priorityLevel; alt.memoizedProps = fiber.memoizedProps; diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 2d0afdf0415..a270cbd0b6b 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -18,7 +18,7 @@ import type { FiberRoot } from 'ReactFiberRoot'; import type { HostConfig } from 'ReactFiberReconciler'; import type { Scheduler } from 'ReactFiberScheduler'; import type { PriorityLevel } from 'ReactPriorityLevel'; -import type { StateQueue } from 'ReactFiberStateQueue'; +import type { UpdateQueue } from 'ReactFiberUpdateQueue'; var { reconcileChildFibers, @@ -42,10 +42,10 @@ var { OffscreenPriority, } = require('ReactPriorityLevel'); var { - createStateQueue, + createUpdateQueue, addToQueue, - mergeStateQueue, -} = require('ReactFiberStateQueue'); + mergeUpdateQueue, +} = require('ReactFiberUpdateQueue'); var ReactInstanceMap = require('ReactInstanceMap'); module.exports = function(config : HostConfig, getScheduler : () => Scheduler) { @@ -115,13 +115,13 @@ module.exports = function(config : HostConfig, getSchedu return workInProgress.child; } - function scheduleUpdate(fiber: Fiber, stateQueue: StateQueue, priorityLevel : PriorityLevel): void { + function scheduleUpdate(fiber: Fiber, updateQueue: UpdateQueue, priorityLevel : PriorityLevel): void { const { scheduleLowPriWork } = getScheduler(); - fiber.stateQueue = stateQueue; + fiber.updateQueue = updateQueue; // Schedule update on the alternate as well, since we don't know which tree // is current. if (fiber.alternate !== null) { - fiber.alternate.stateQueue = stateQueue; + fiber.alternate.updateQueue = updateQueue; } while (true) { if (fiber.pendingWorkPriority === NoWork || @@ -151,10 +151,10 @@ module.exports = function(config : HostConfig, getSchedu const updater = { enqueueSetState(instance, partialState) { const fiber = ReactInstanceMap.get(instance); - const stateQueue = fiber.stateQueue ? - addToQueue(fiber.stateQueue, partialState) : - createStateQueue(partialState); - scheduleUpdate(fiber, stateQueue, LowPriority); + const updateQueue = fiber.updateQueue ? + addToQueue(fiber.updateQueue, partialState) : + createUpdateQueue(partialState); + scheduleUpdate(fiber, updateQueue, LowPriority); }, }; @@ -166,21 +166,22 @@ module.exports = function(config : HostConfig, getSchedu if (!props && current) { props = current.memoizedProps; } - // Compute the state using the memoized state and the pending state queue. - var stateQueue = workInProgress.stateQueue; - var state = current ? - mergeStateQueue(stateQueue, current.memoizedState, props) : - mergeStateQueue(stateQueue, null, props); + // Compute the state using the memoized state and the update queue. + var updateQueue = workInProgress.updateQueue; + var previousState = current ? current.memoizedState : null; + var state = updateQueue ? + mergeUpdateQueue(updateQueue, previousState, props) : + previousState; var instance = workInProgress.stateNode; if (!instance) { var ctor = workInProgress.type; workInProgress.stateNode = instance = new ctor(props); state = instance.state || null; - // The initial state must be added to the pending state queue in case + // The initial state must be added to the update queue in case // setState is called before the initial render. if (state !== null) { - workInProgress.stateQueue = createStateQueue(state); + workInProgress.updateQueue = createUpdateQueue(state); } // The instance needs access to the fiber so that it can schedule updates ReactInstanceMap.set(instance, workInProgress); @@ -330,7 +331,7 @@ module.exports = function(config : HostConfig, getSchedu if (workInProgress.pendingProps === null || ( workInProgress.memoizedProps !== null && workInProgress.pendingProps === workInProgress.memoizedProps && - workInProgress.stateQueue === null + workInProgress.updateQueue === null )) { return bailoutOnAlreadyFinishedWork(current, workInProgress); } diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index ef99eb262d3..7b4d5edc97a 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -142,9 +142,9 @@ module.exports = function(config : HostConfig) { // The work is now done. We don't need this anymore. This flags // to the system not to redo any work here. workInProgress.pendingProps = null; - workInProgress.stateQueue = null; + workInProgress.updateQueue = null; if (current) { - current.stateQueue = null; + current.updateQueue = null; } const returnFiber = workInProgress.return; diff --git a/src/renderers/shared/fiber/ReactFiberStateQueue.js b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js similarity index 51% rename from src/renderers/shared/fiber/ReactFiberStateQueue.js rename to src/renderers/shared/fiber/ReactFiberUpdateQueue.js index d410f8d0d69..57329b25229 100644 --- a/src/renderers/shared/fiber/ReactFiberStateQueue.js +++ b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js @@ -6,53 +6,59 @@ * 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. * - * @providesModule ReactFiberStateQueue + * @providesModule ReactFiberUpdateQueue * @flow */ 'use strict'; -type StateQueueNode = { +type UpdateQueueNode = { partialState: any, callback: ?Function, - next: ?StateQueueNode, + next: ?UpdateQueueNode, }; -export type StateQueue = StateQueueNode & { - tail: ?StateQueueNode +export type UpdateQueue = UpdateQueueNode & { + tail: UpdateQueueNode }; -exports.createStateQueue = function(partialState : mixed) : StateQueue { - return { +exports.createUpdateQueue = function(partialState : mixed) : UpdateQueue { + const queue = { partialState, callback: null, next: null, - tail: null, + tail: (null : any), }; + queue.tail = queue; + return queue; }; -exports.addToQueue = function(queue : StateQueue, partialState : mixed) : StateQueue { +exports.addToQueue = function(queue : UpdateQueue, partialState : mixed) : UpdateQueue { const node = { partialState, callback: null, next: null, }; - if (!queue.tail) { - queue.next = node; - } else { - queue.tail.next = node; - } + queue.tail.next = node; queue.tail = node; return queue; }; -exports.mergeStateQueue = function(queue : ?StateQueue, prevState : any, props : any) : any { - let node : ?StateQueueNode = queue; - if (!node) { - return prevState; +exports.callCallbacks = function(queue : UpdateQueue, partialState : mixed) { + let node : ?UpdateQueueNode = queue; + while (node) { + if (node.callback) { + const { callback } = node; + callback(); + } + node = node.next; } +}; + +exports.mergeUpdateQueue = function(queue : UpdateQueue, prevState : any, props : any) : any { + let node : ?UpdateQueueNode = queue; let state = Object.assign({}, prevState); - do { + while (node) { let partialState; if (typeof node.partialState === 'function') { const updateFn = node.partialState; @@ -61,6 +67,7 @@ exports.mergeStateQueue = function(queue : ?StateQueue, prevState : any, props : partialState = node.partialState; } state = Object.assign(state, partialState); - } while (node = node.next); + node = node.next; + } return state; }; From 50c9298b5cd9520ffd0ee614541231e8a1a46b67 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Thu, 4 Aug 2016 17:56:48 -0700 Subject: [PATCH 16/21] Update callbacks Callbacks are stored on the same queue as updates. They care called during the commit phase, after the updates have been flushed. Because the update queue is cleared during the completion phase (before commit), a new field has been added to fiber called callbackList. The queue is transferred from updateQueue to callbackList during completion. During commit, the list is reset. Need a test to confirm that callbacks are not lost if an update is preempted. --- src/renderers/shared/fiber/ReactFiber.js | 5 ++++ .../shared/fiber/ReactFiberBeginWork.js | 12 ++++++++ .../shared/fiber/ReactFiberCommitWork.js | 9 ++++++ .../shared/fiber/ReactFiberCompleteWork.js | 11 +++++-- .../shared/fiber/ReactFiberUpdateQueue.js | 20 ++++++++++--- .../ReactIncrementalSideEffects-test.js | 30 +++++++++++++++++++ 6 files changed, 80 insertions(+), 7 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiber.js b/src/renderers/shared/fiber/ReactFiber.js index 495c615c95e..7dadfbe47a9 100644 --- a/src/renderers/shared/fiber/ReactFiber.js +++ b/src/renderers/shared/fiber/ReactFiber.js @@ -81,6 +81,8 @@ export type Fiber = Instance & { updateQueue: ?UpdateQueue, // The state used to create the output. This is a full state object. memoizedState: any, + // Linked list of callbacks to call after updates are committed. + callbackList: ?UpdateQueue, // Output is the return value of this fiber, or a linked list of return values // if this returns multiple values. Such as a fragment. output: any, // This type will be more specific once we overload the tag. @@ -158,6 +160,7 @@ var createFiber = function(tag : TypeOfWork, key : null | string) : Fiber { memoizedProps: null, updateQueue: null, memoizedState: null, + callbackList: null, output: null, nextEffect: null, @@ -200,6 +203,7 @@ exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fi alt.ref = fiber.ref; alt.pendingProps = fiber.pendingProps; // TODO: Pass as argument. alt.updateQueue = fiber.updateQueue; + alt.callbackList = fiber.callbackList; alt.pendingWorkPriority = priorityLevel; alt.child = fiber.child; @@ -226,6 +230,7 @@ exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fi // TODO: Pass in the new pendingProps as an argument maybe? alt.pendingProps = fiber.pendingProps; alt.updateQueue = fiber.updateQueue; + alt.callbackList = fiber.callbackList; alt.pendingWorkPriority = priorityLevel; alt.memoizedProps = fiber.memoizedProps; diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index a270cbd0b6b..4e4128e44d6 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -44,6 +44,7 @@ var { var { createUpdateQueue, addToQueue, + addCallbackToQueue, mergeUpdateQueue, } = require('ReactFiberUpdateQueue'); var ReactInstanceMap = require('ReactInstanceMap'); @@ -156,6 +157,17 @@ module.exports = function(config : HostConfig, getSchedu createUpdateQueue(partialState); scheduleUpdate(fiber, updateQueue, LowPriority); }, + 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; + } + }, }; function updateClassComponent(current : ?Fiber, workInProgress : Fiber) { diff --git a/src/renderers/shared/fiber/ReactFiberCommitWork.js b/src/renderers/shared/fiber/ReactFiberCommitWork.js index c7a9f2f7109..b0de664837b 100644 --- a/src/renderers/shared/fiber/ReactFiberCommitWork.js +++ b/src/renderers/shared/fiber/ReactFiberCommitWork.js @@ -22,6 +22,7 @@ var { HostContainer, HostComponent, } = ReactTypeOfWork; +var { callCallbacks } = require('ReactFiberUpdateQueue'); module.exports = function(config : HostConfig) { @@ -31,6 +32,14 @@ module.exports = function(config : HostConfig) { function commitWork(finishedWork : Fiber) : void { switch (finishedWork.tag) { case ClassComponent: { + if (finishedWork.callbackList) { + const { callbackList } = finishedWork; + finishedWork.callbackList = null; + if (finishedWork.alternate) { + finishedWork.alternate.callbackList = null; + } + callCallbacks(callbackList, finishedWork.stateNode); + } // TODO: Fire componentDidMount/componentDidUpdate, update refs return; } diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index fdeb1f5e56c..6d6f9598c41 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -46,7 +46,6 @@ module.exports = function(config : HostConfig) { } } - /* // TODO: It's possible this will create layout thrash issues because mutations // of the DOM and life-cycles are interleaved. E.g. if a componentDidMount // of a sibling reads, then the next sibling updates and reads etc. @@ -59,7 +58,6 @@ module.exports = function(config : HostConfig) { } workInProgress.lastEffect = workInProgress; } - */ function transferOutput(child : ?Fiber, returnFiber : Fiber) { // If we have a single result, we just pass that through as the output to @@ -133,9 +131,16 @@ module.exports = function(config : HostConfig) { case ClassComponent: transferOutput(workInProgress.child, workInProgress); // Don't use the state queue to compute the memoized state. We already - // merged it and assigned it to the instance. Copy it from there. + // merged it and assigned it to the instance. Transfer it from there. const state = workInProgress.stateNode.state; workInProgress.memoizedState = state; + // Transfer update queue to callbackList field so callbacks can be + // called during commit phase. + workInProgress.callbackList = workInProgress.updateQueue; + if (current) { + current.callbackList = workInProgress.callbackList; + } + markForPostEffect(workInProgress); return null; case HostContainer: transferOutput(workInProgress.child, workInProgress); diff --git a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js index 57329b25229..159b0ceb095 100644 --- a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js +++ b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js @@ -15,6 +15,7 @@ type UpdateQueueNode = { partialState: any, callback: ?Function, + callbackWasCalled: boolean, next: ?UpdateQueueNode, }; @@ -26,6 +27,7 @@ exports.createUpdateQueue = function(partialState : mixed) : UpdateQueue { const queue = { partialState, callback: null, + callbackWasCalled: false, next: null, tail: (null : any), }; @@ -37,6 +39,7 @@ exports.addToQueue = function(queue : UpdateQueue, partialState : mixed) : Updat const node = { partialState, callback: null, + callbackWasCalled: false, next: null, }; queue.tail.next = node; @@ -44,12 +47,21 @@ exports.addToQueue = function(queue : UpdateQueue, partialState : mixed) : Updat return queue; }; -exports.callCallbacks = function(queue : UpdateQueue, partialState : mixed) { +exports.addCallbackToQueue = function(queue : UpdateQueue, callback: Function) : UpdateQueue { + if (queue.tail.callback) { + // If the tail already as a callback, add an empty node to queue + exports.addToQueue(queue, null); + } + queue.tail.callback = callback; + return queue; +}; + +exports.callCallbacks = function(queue : UpdateQueue, context : any) { let node : ?UpdateQueueNode = queue; while (node) { - if (node.callback) { - const { callback } = node; - callback(); + if (node.callback && !node.callbackWasCalled) { + node.callbackWasCalled = true; + node.callback.call(context); } node = node.next; } diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js index 2fd25691567..5813d058f2d 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js @@ -354,4 +354,34 @@ describe('ReactIncrementalSideEffects', function() { // moves to "current" without flushing due to having lower priority. Does this // even happen? Maybe a child doesn't get processed because it is lower prio? + it('calls callback after update is flushed', () => { + let instance; + class Foo extends React.Component { + constructor() { + super(); + instance = this; + this.state = { text: 'foo' }; + } + render() { + return ; + } + } + + ReactNoop.render(); + ReactNoop.flush(); + expect(ReactNoop.root.children).toEqual([ + span('foo'), + ]); + let called = false; + instance.setState({ text: 'bar' }, () => { + expect(ReactNoop.root.children).toEqual([ + span('bar'), + ]); + called = true; + }); + ReactNoop.flush(); + expect(called).toBe(true); + }); + + // TODO: Test that callbacks are not lost if an update is preempted. }); From b4fbdfa3673520b061981764487a9ab5e66214f0 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Thu, 4 Aug 2016 18:55:57 -0700 Subject: [PATCH 17/21] replaceState Adds a field to UpdateQueue that indicates whether an update should replace the previous state completely. --- .../shared/fiber/ReactFiberBeginWork.js | 6 ++++ .../shared/fiber/ReactFiberUpdateQueue.js | 6 ++-- .../fiber/__tests__/ReactIncremental-test.js | 29 +++++++++++++++++++ 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 4e4128e44d6..1a04c64cbaa 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -157,6 +157,12 @@ module.exports = function(config : HostConfig, getSchedu createUpdateQueue(partialState); scheduleUpdate(fiber, updateQueue, LowPriority); }, + enqueueReplaceState(instance, state) { + const fiber = ReactInstanceMap.get(instance); + const updateQueue = createUpdateQueue(state); + updateQueue.isReplace = true; + scheduleUpdate(fiber, updateQueue, LowPriority); + }, enqueueCallback(instance, callback) { const fiber = ReactInstanceMap.get(instance); let updateQueue = fiber.updateQueue ? diff --git a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js index 159b0ceb095..97039994b17 100644 --- a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js +++ b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js @@ -20,6 +20,7 @@ type UpdateQueueNode = { }; export type UpdateQueue = UpdateQueueNode & { + isReplace: boolean, tail: UpdateQueueNode }; @@ -29,6 +30,7 @@ exports.createUpdateQueue = function(partialState : mixed) : UpdateQueue { callback: null, callbackWasCalled: false, next: null, + isReplace: false, tail: (null : any), }; queue.tail = queue; @@ -69,7 +71,7 @@ exports.callCallbacks = function(queue : UpdateQueue, context : any) { exports.mergeUpdateQueue = function(queue : UpdateQueue, prevState : any, props : any) : any { let node : ?UpdateQueueNode = queue; - let state = Object.assign({}, prevState); + let state = queue.isReplace ? null : Object.assign({}, prevState); while (node) { let partialState; if (typeof node.partialState === 'function') { @@ -78,7 +80,7 @@ exports.mergeUpdateQueue = function(queue : UpdateQueue, prevState : any, props } else { partialState = node.partialState; } - state = Object.assign(state, partialState); + state = Object.assign(state || {}, partialState); node = node.next; } return state; diff --git a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js index d16fe5a2ee3..298dcedf262 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js @@ -653,4 +653,33 @@ describe('ReactIncremental', function() { ReactNoop.flush(); expect(instance.state.num).toEqual(6); }); + + it('can replaceState', () => { + let instance; + const Bar = React.createClass({ + getInitialState() { + instance = this; + return { a: 'a' }; + }, + render() { + return
{this.props.children}
; + }, + }); + + function Foo() { + return ( +
+ +
+ ); + } + + ReactNoop.render(); + ReactNoop.flush(); + instance.setState({ b: 'b' }); + instance.setState({ c: 'c' }); + instance.replaceState({ d: 'd' }); + ReactNoop.flush(); + expect(instance.state).toEqual({ d: 'd' }); + }); }); From 75c64caece563ec123355e9b7b4b69ebbcedeecc Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Thu, 4 Aug 2016 19:16:29 -0700 Subject: [PATCH 18/21] forceUpdate Adds a field to the update queue that causes shouldComponentUpdate to be skipped. --- .../shared/fiber/ReactFiberBeginWork.js | 9 ++++- .../shared/fiber/ReactFiberUpdateQueue.js | 2 + .../fiber/__tests__/ReactIncremental-test.js | 40 +++++++++++++++++++ 3 files changed, 50 insertions(+), 1 deletion(-) diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 1a04c64cbaa..64f0b29db55 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -163,6 +163,12 @@ module.exports = function(config : HostConfig, getSchedu updateQueue.isReplace = true; scheduleUpdate(fiber, updateQueue, LowPriority); }, + enqueueForceUpdate(instance) { + const fiber = ReactInstanceMap.get(instance); + const updateQueue = fiber.updateQueue || createUpdateQueue(null); + updateQueue.isForced = true; + scheduleUpdate(fiber, updateQueue, LowPriority); + }, enqueueCallback(instance, callback) { const fiber = ReactInstanceMap.get(instance); let updateQueue = fiber.updateQueue ? @@ -204,7 +210,8 @@ module.exports = function(config : HostConfig, getSchedu // The instance needs access to the fiber so that it can schedule updates ReactInstanceMap.set(instance, workInProgress); instance.updater = updater; - } else if (typeof instance.shouldComponentUpdate === 'function') { + } else if (typeof instance.shouldComponentUpdate === 'function' && + !(updateQueue && updateQueue.isForced)) { if (workInProgress.memoizedProps !== null) { // Reset the props, in case this is a ping-pong case rather than a // completed update case. For the completed update case, the instance diff --git a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js index 97039994b17..aab252b6558 100644 --- a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js +++ b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js @@ -21,6 +21,7 @@ type UpdateQueueNode = { export type UpdateQueue = UpdateQueueNode & { isReplace: boolean, + isForced: boolean, tail: UpdateQueueNode }; @@ -31,6 +32,7 @@ exports.createUpdateQueue = function(partialState : mixed) : UpdateQueue { callbackWasCalled: false, next: null, isReplace: false, + isForced: false, tail: (null : any), }; queue.tail = queue; diff --git a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js index 298dcedf262..ed53fe132f2 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js @@ -682,4 +682,44 @@ describe('ReactIncremental', function() { ReactNoop.flush(); expect(instance.state).toEqual({ d: 'd' }); }); + + it('can forceUpdate', () => { + const ops = []; + + function Baz() { + ops.push('Baz'); + return
; + } + + let instance; + class Bar extends React.Component { + constructor() { + super(); + instance = this; + } + shouldComponentUpdate() { + return false; + } + render() { + ops.push('Bar'); + return ; + } + } + + function Foo() { + ops.push('Foo'); + return ( +
+ +
+ ); + } + + ReactNoop.render(); + ReactNoop.flush(); + expect(ops).toEqual(['Foo', 'Bar', 'Baz']); + instance.forceUpdate(); + ReactNoop.flush(); + expect(ops).toEqual(['Foo', 'Bar', 'Baz', 'Bar', 'Baz']); + }); }); From 410c448320a99b82af6fc058da318e48249ccf31 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Sat, 6 Aug 2016 16:47:48 -0700 Subject: [PATCH 19/21] Don't mutate current tree before work is committed. We should be able to abort an update without any side-effects to the current tree. This fixes a few cases where that was broken. The callback list should only ever be set on the workInProgress. There's no reason to add it to the current tree because they're not needed after they are called during the commit phase. Also found a bug where the memoizedProps were set to null in the case of an update, because the pendingProps were null. Fixed by transfering the props from the instance, like we were already doing with state. Added a test to ensure that setState can be called inside a callback. --- .../shared/fiber/ReactFiberCommitWork.js | 10 +++-- .../shared/fiber/ReactFiberCompleteWork.js | 8 ++-- .../shared/fiber/ReactFiberScheduler.js | 3 -- .../fiber/__tests__/ReactIncremental-test.js | 38 +++++++++++++++++++ 4 files changed, 49 insertions(+), 10 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberCommitWork.js b/src/renderers/shared/fiber/ReactFiberCommitWork.js index b0de664837b..c654a0065c2 100644 --- a/src/renderers/shared/fiber/ReactFiberCommitWork.js +++ b/src/renderers/shared/fiber/ReactFiberCommitWork.js @@ -32,12 +32,16 @@ module.exports = function(config : HostConfig) { function commitWork(finishedWork : Fiber) : void { switch (finishedWork.tag) { case ClassComponent: { + // 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. + if (finishedWork.alternate) { + finishedWork.alternate.updateQueue = null; + } if (finishedWork.callbackList) { const { callbackList } = finishedWork; finishedWork.callbackList = null; - if (finishedWork.alternate) { - finishedWork.alternate.callbackList = null; - } callCallbacks(callbackList, finishedWork.stateNode); } // TODO: Fire componentDidMount/componentDidUpdate, update refs diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index 6d6f9598c41..d4687f45745 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -132,14 +132,14 @@ module.exports = function(config : HostConfig) { transferOutput(workInProgress.child, workInProgress); // Don't use the state queue to compute the memoized state. We already // merged it and assigned it to the instance. Transfer it from there. - const state = workInProgress.stateNode.state; + // Also need to transfer the props, because pendingProps will be null + // in the case of an update + const { state, props } = workInProgress.stateNode; 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) { - current.callbackList = workInProgress.callbackList; - } markForPostEffect(workInProgress); return null; case HostContainer: diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 7b4d5edc97a..76369f0614a 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -143,9 +143,6 @@ module.exports = function(config : HostConfig) { // to the system not to redo any work here. workInProgress.pendingProps = null; workInProgress.updateQueue = null; - if (current) { - current.updateQueue = null; - } const returnFiber = workInProgress.return; diff --git a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js index ed53fe132f2..e0a0a3a5c6b 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js @@ -654,6 +654,44 @@ describe('ReactIncremental', function() { expect(instance.state.num).toEqual(6); }); + it('can call setState inside update callback', () => { + let instance; + class Bar extends React.Component { + constructor() { + super(); + this.state = { num: 1 }; + instance = this; + } + render() { + return
{this.props.children}
; + } + } + + function Foo({ multiplier }) { + return ( +
+ +
+ ); + } + + function updater(state, props) { + return { num: state.num * props.multiplier }; + } + + function callback() { + this.setState({ called: true }); + } + + ReactNoop.render(); + ReactNoop.flush(); + instance.setState(updater); + instance.setState(updater, callback); + ReactNoop.flush(); + expect(instance.state.num).toEqual(4); + expect(instance.state.called).toEqual(true); + }); + it('can replaceState', () => { let instance; const Bar = React.createClass({ From 995a02db85957847228a1e56d0b6ded4fb03de58 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 2 Sep 2016 17:23:09 -0700 Subject: [PATCH 20/21] Check for truthiness of alternate This is unfortunate since we agreed on using the `null | Fiber` convention instead of `?Fiber` but haven't upgraded yet and this is the pattern I've been using everywhere else so far. --- src/renderers/shared/fiber/ReactFiberBeginWork.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 64f0b29db55..264c670d3ca 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -121,7 +121,7 @@ module.exports = function(config : HostConfig, getSchedu fiber.updateQueue = updateQueue; // Schedule update on the alternate as well, since we don't know which tree // is current. - if (fiber.alternate !== null) { + if (fiber.alternate) { fiber.alternate.updateQueue = updateQueue; } while (true) { @@ -129,7 +129,7 @@ module.exports = function(config : HostConfig, getSchedu fiber.pendingWorkPriority >= priorityLevel) { fiber.pendingWorkPriority = priorityLevel; } - if (fiber.alternate !== null) { + if (fiber.alternate) { if (fiber.alternate.pendingWorkPriority === NoWork || fiber.alternate.pendingWorkPriority >= priorityLevel) { fiber.alternate.pendingWorkPriority = priorityLevel; From c948425ec990bbf3faac4c63faac67709d9358d9 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Tue, 9 Aug 2016 18:13:00 -0700 Subject: [PATCH 21/21] Separate priority field for pending updates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently there is a single priority field `pendingWorkPriority` that represents the priority of the entire subtree. The scheduler does a breadth-first search to find the next unit of work with a matching priority level. If a subtree's priority does not match, that entire subtree is skipped. That means when an update is scheduled deep in the tree (`setState`), its priority must be bubbled to the top of the tree. The problem is that this causes the all the parent priorities to be overridden — there's no way to schedule a high priority update inside a low priority tree without increasing the priority of the entire tree. To address this, this PR introduces a separate priority field called `pendingUpdatePriority` (not 100% sold on the name). The new field represents the priority of the pending props and state, which may be lower than the priority of the entire tree. Now, when the scheduler is searching for work to perform, it only matches if the ` pendingUpdatePriority` matches. It continues to use the `pendingWorkPriority` as a way to ignore subtrees if they do not match. (To elaborate further: because the work priority is the highest priority of any node in the entire tree, if it does not match, the tree can be skipped.) --- src/renderers/shared/fiber/ReactChildFiber.js | 17 ++++++- src/renderers/shared/fiber/ReactFiber.js | 38 ++++++++++++++-- .../shared/fiber/ReactFiberBeginWork.js | 25 ++++++++--- .../shared/fiber/ReactFiberCompleteWork.js | 2 +- .../shared/fiber/ReactFiberReconciler.js | 3 ++ .../shared/fiber/ReactFiberScheduler.js | 5 ++- .../fiber/__tests__/ReactIncremental-test.js | 45 +++++++++++++++++++ 7 files changed, 122 insertions(+), 13 deletions(-) diff --git a/src/renderers/shared/fiber/ReactChildFiber.js b/src/renderers/shared/fiber/ReactChildFiber.js index 699b0e78830..26ee1dac2fb 100644 --- a/src/renderers/shared/fiber/ReactChildFiber.js +++ b/src/renderers/shared/fiber/ReactChildFiber.js @@ -26,6 +26,7 @@ var { var ReactFiber = require('ReactFiber'); var ReactReifiedYield = require('ReactReifiedYield'); +var ReactPriorityLevel = require('ReactPriorityLevel'); const { cloneFiber, @@ -38,6 +39,10 @@ const { createReifiedYield, } = ReactReifiedYield; +const { + NoWork, +} = ReactPriorityLevel; + const isArray = Array.isArray; function ChildReconciler(shouldClone) { @@ -64,7 +69,11 @@ function ChildReconciler(shouldClone) { const clone = shouldClone ? cloneFiber(existingChild, priority) : existingChild; if (!shouldClone) { // TODO: This might be lowering the priority of nested unfinished work. - clone.pendingWorkPriority = priority; + clone.pendingUpdatePriority = priority; + if (clone.pendingWorkPriority === NoWork || + clone.pendingWorkPriority > priority) { + clone.pendingWorkPriority = priority; + } } clone.pendingProps = element.props; // clone.child = existingChild.child; @@ -136,7 +145,11 @@ function ChildReconciler(shouldClone) { const clone = shouldClone ? cloneFiber(existingChild, priority) : existingChild; if (!shouldClone) { // TODO: This might be lowering the priority of nested unfinished work. - clone.pendingWorkPriority = priority; + clone.pendingUpdatePriority = priority; + if (clone.pendingWorkPriority === NoWork || + clone.pendingWorkPriority > priority) { + clone.pendingWorkPriority = priority; + } } clone.pendingProps = element.props; // clone.child = existingChild.child; diff --git a/src/renderers/shared/fiber/ReactFiber.js b/src/renderers/shared/fiber/ReactFiber.js index 7dadfbe47a9..0e083345069 100644 --- a/src/renderers/shared/fiber/ReactFiber.js +++ b/src/renderers/shared/fiber/ReactFiber.js @@ -96,7 +96,12 @@ export type Fiber = Instance & { firstEffect: ?Fiber, lastEffect: ?Fiber, - // This will be used to quickly determine if a subtree has no pending changes. + // The update priority is the priority of a fiber's pending props and state. + // It may be lower than the priority of the entire subtree. + pendingUpdatePriority: PriorityLevel, + + // The work priority is the priority of the entire subtree. It will be used to + // quickly determine if a subtree has no pending changes. pendingWorkPriority: PriorityLevel, // This value represents the priority level that was last used to process this @@ -167,6 +172,7 @@ var createFiber = function(tag : TypeOfWork, key : null | string) : Fiber { firstEffect: null, lastEffect: null, + pendingUpdatePriority: NoWork, pendingWorkPriority: NoWork, progressedPriority: NoWork, progressedChild: null, @@ -191,6 +197,28 @@ exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fi // fiber.progressedPriority = NoWork; // fiber.progressedChild = null; + // Don't deprioritize when cloning. Unlike other priority comparisons (e.g. + // in the scheduler), this one must check that priorityLevel is not equal to + // NoWork, otherwise work will be dropped. For complete correctness, the other + // priority comparisons should also perform this check, even though it's not + // an issue in practice. I didn't catch this at first and it created a subtle + // bug, which suggests we may need to extract the logic into a + // utility function (shouldOverridePriority). + let updatePriority; + let workPriority; + if (priorityLevel !== NoWork && + (priorityLevel < fiber.pendingUpdatePriority || fiber.pendingUpdatePriority === NoWork)) { + updatePriority = priorityLevel; + } else { + updatePriority = fiber.pendingUpdatePriority; + } + if (updatePriority !== NoWork && + (updatePriority < fiber.pendingWorkPriority || fiber.pendingWorkPriority === NoWork)) { + workPriority = updatePriority; + } else { + workPriority = fiber.pendingWorkPriority; + } + // We use a double buffering pooling technique because we know that we'll only // ever need at most two versions of a tree. We pool the "other" unused node // that we're free to reuse. This is lazily created to avoid allocating extra @@ -204,7 +232,8 @@ exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fi alt.pendingProps = fiber.pendingProps; // TODO: Pass as argument. alt.updateQueue = fiber.updateQueue; alt.callbackList = fiber.callbackList; - alt.pendingWorkPriority = priorityLevel; + alt.pendingUpdatePriority = updatePriority; + alt.pendingWorkPriority = workPriority; alt.child = fiber.child; alt.memoizedProps = fiber.memoizedProps; @@ -231,7 +260,8 @@ exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fi alt.pendingProps = fiber.pendingProps; alt.updateQueue = fiber.updateQueue; alt.callbackList = fiber.callbackList; - alt.pendingWorkPriority = priorityLevel; + alt.pendingUpdatePriority = updatePriority; + alt.pendingWorkPriority = workPriority; alt.memoizedProps = fiber.memoizedProps; alt.output = fiber.output; @@ -253,6 +283,7 @@ exports.createFiberFromElement = function(element : ReactElement<*>, priorityLev // $FlowFixMe: ReactElement.key is currently defined as ?string but should be defined as null | string in Flow. const fiber = createFiberFromElementType(element.type, element.key); fiber.pendingProps = element.props; + fiber.pendingUpdatePriority = priorityLevel; fiber.pendingWorkPriority = priorityLevel; return fiber; }; @@ -282,6 +313,7 @@ exports.createFiberFromCoroutine = function(coroutine : ReactCoroutine, priority const fiber = createFiber(CoroutineComponent, coroutine.key); fiber.type = coroutine.handler; fiber.pendingProps = coroutine; + fiber.pendingUpdatePriority = priorityLevel; fiber.pendingWorkPriority = priorityLevel; return fiber; }; diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 264c670d3ca..03b73f81e71 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -64,7 +64,7 @@ module.exports = function(config : HostConfig, getSchedu } function reconcileChildren(current, workInProgress, nextChildren) { - const priorityLevel = workInProgress.pendingWorkPriority; + const priorityLevel = workInProgress.pendingUpdatePriority; reconcileChildrenAtPriority(current, workInProgress, nextChildren, priorityLevel); } @@ -124,14 +124,29 @@ module.exports = function(config : HostConfig, getSchedu if (fiber.alternate) { fiber.alternate.updateQueue = updateQueue; } + + // Set the update priority of the fiber and its alternate + if (fiber.pendingUpdatePriority === NoWork || + fiber.pendingUpdatePriority > priorityLevel) { + fiber.pendingUpdatePriority = priorityLevel; + } + if (fiber.alternate) { + if (fiber.alternate.pendingUpdatePriority === NoWork || + fiber.alternate.pendingUpdatePriority > priorityLevel) { + fiber.alternate.pendingUpdatePriority = priorityLevel; + } + } + + // For this fiber and all its ancestors and their alternates, set the + // work (subtree) priority while (true) { if (fiber.pendingWorkPriority === NoWork || - fiber.pendingWorkPriority >= priorityLevel) { + fiber.pendingWorkPriority > priorityLevel) { fiber.pendingWorkPriority = priorityLevel; } if (fiber.alternate) { if (fiber.alternate.pendingWorkPriority === NoWork || - fiber.alternate.pendingWorkPriority >= priorityLevel) { + fiber.alternate.pendingWorkPriority > priorityLevel) { fiber.alternate.pendingWorkPriority = priorityLevel; } } @@ -235,7 +250,7 @@ module.exports = function(config : HostConfig, getSchedu function updateHostComponent(current, workInProgress) { const nextChildren = workInProgress.pendingProps.children; if (workInProgress.pendingProps.hidden && - workInProgress.pendingWorkPriority !== OffscreenPriority) { + workInProgress.pendingUpdatePriority !== OffscreenPriority) { // If this host component is hidden, we can bail out on the children. // We'll rerender the children later at the lower priority. @@ -311,7 +326,7 @@ module.exports = function(config : HostConfig, getSchedu */ function bailoutOnAlreadyFinishedWork(current, workInProgress : Fiber) : ?Fiber { - const priorityLevel = workInProgress.pendingWorkPriority; + const priorityLevel = workInProgress.pendingUpdatePriority; // TODO: We should ideally be able to bail out early if the children have no // more work to do. However, since we don't have a separation of this diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index d4687f45745..38c257b05be 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -113,7 +113,7 @@ module.exports = function(config : HostConfig) { var currentFirstChild = current ? current.stateNode : null; // Inherit the priority of the returnFiber. - const priority = workInProgress.pendingWorkPriority; + const priority = workInProgress.pendingUpdatePriority; workInProgress.stateNode = reconcileChildFibers( workInProgress, currentFirstChild, diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js index aa39d1b48d3..623f6c98b31 100644 --- a/src/renderers/shared/fiber/ReactFiberReconciler.js +++ b/src/renderers/shared/fiber/ReactFiberReconciler.js @@ -73,6 +73,7 @@ module.exports = function(config : HostConfig) : Reconci // TODO: This should not override the pendingWorkPriority if there is // higher priority work in the subtree. container.pendingProps = element; + container.pendingUpdatePriority = LowPriority; container.pendingWorkPriority = LowPriority; scheduleLowPriWork(root, LowPriority); @@ -88,6 +89,7 @@ module.exports = function(config : HostConfig) : Reconci const root : FiberRoot = (container.stateNode : any); // TODO: Use pending work/state instead of props. root.current.pendingProps = element; + root.current.pendingUpdatePriority = LowPriority; root.current.pendingWorkPriority = LowPriority; scheduleLowPriWork(root, LowPriority); @@ -98,6 +100,7 @@ module.exports = function(config : HostConfig) : Reconci const root : FiberRoot = (container.stateNode : any); // TODO: Use pending work/state instead of props. root.current.pendingProps = []; + root.current.pendingUpdatePriority = LowPriority; root.current.pendingWorkPriority = LowPriority; scheduleLowPriWork(root, LowPriority); diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 76369f0614a..54e8708951a 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -75,8 +75,8 @@ module.exports = function(config : HostConfig) { let highestPriorityLevel = NoWork; while (root) { if (highestPriorityLevel === NoWork || - highestPriorityLevel > root.current.pendingWorkPriority) { - highestPriorityLevel = root.current.pendingWorkPriority; + highestPriorityLevel > root.current.pendingUpdatePriority) { + highestPriorityLevel = root.current.pendingUpdatePriority; highestPriorityRoot = root; } // We didn't find anything to do in this root, so let's try the next one. @@ -137,6 +137,7 @@ module.exports = function(config : HostConfig) { const current = workInProgress.alternate; const next = completeWork(current, workInProgress); + workInProgress.pendingUpdatePriority = NoWork; resetWorkPriority(workInProgress); // The work is now done. We don't need this anymore. This flags diff --git a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js index e0a0a3a5c6b..42cd8b09720 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js @@ -692,6 +692,51 @@ describe('ReactIncremental', function() { expect(instance.state.called).toEqual(true); }); + it('can setState without overriding its parents\' priority', () => { + let ops = []; + let instance; + class Baz extends React.Component { + constructor() { + super(); + instance = this; + this.state = { num: 0 }; + } + render() { + ops.push('Baz'); + return ; + } + } + + function Bar({ id }) { + ops.push('Bar' + id); + return
; + } + + function Foo() { + ops.push('Foo'); + return [ + , + , + ]; + } + + ReactNoop.render(); + ReactNoop.flush(); + expect(ops).toEqual(['Foo', 'Bar2', 'Bar1', 'Baz']); + ops = []; + ReactNoop.render(); + // Even though Baz is in a hidden subtree, calling setState gives it a + // higher priority. It should not affect the priority of anything else in + // the subtree. + instance.setState({ num: 1 }); + ReactNoop.flush(); + // Baz should come before Bar1 because it has higher priority + expect(ops).toEqual(['Foo', 'Bar2', 'Baz', 'Bar1']); + }); + it('can replaceState', () => { let instance; const Bar = React.createClass({