From 40d940433f34e5ea0f83d488dbb3f2c42a2d3d90 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Tue, 10 Jan 2017 00:20:27 -0800 Subject: [PATCH 01/14] Change pendingWorkPriority to represent the priority of the child Implications: - When we clone from current, set the pendingWorkPriority of the parent, not the child. - The check to bail out on low priority must occur earlier, before we beginWork on it. It turns out this only happens when the parent bails out on already finished work. So we do the check there. --- scripts/fiber/tests-failing.txt | 3 + scripts/fiber/tests-passing.txt | 1 - src/renderers/shared/fiber/ReactChildFiber.js | 188 ++++++++---------- src/renderers/shared/fiber/ReactFiber.js | 24 +-- .../shared/fiber/ReactFiberBeginWork.js | 137 +++++-------- .../shared/fiber/ReactFiberCompleteWork.js | 51 ++++- .../shared/fiber/ReactFiberScheduler.js | 127 ++++++------ .../fiber/__tests__/ReactIncremental-test.js | 4 +- 8 files changed, 256 insertions(+), 279 deletions(-) diff --git a/scripts/fiber/tests-failing.txt b/scripts/fiber/tests-failing.txt index fdde0f21ec2..291ebe9525e 100644 --- a/scripts/fiber/tests-failing.txt +++ b/scripts/fiber/tests-failing.txt @@ -49,6 +49,9 @@ src/renderers/shared/__tests__/ReactPerf-test.js * should not count time in a portal towards lifecycle method * should work when measurement starts during reconciliation +src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js +* can defer side-effects and reuse them later - complex + src/renderers/shared/hooks/__tests__/ReactComponentTreeHook-test.js * can be retrieved by ID diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 21c98427165..e39919fc239 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -1244,7 +1244,6 @@ src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js * can update a completed tree before it has a chance to commit * updates a child even though the old props is empty * can defer side-effects and resume them later on -* can defer side-effects and reuse them later - complex * deprioritizes setStates that happens within a deprioritized tree * calls callback after update is flushed * calls setState callback even if component bails out diff --git a/src/renderers/shared/fiber/ReactChildFiber.js b/src/renderers/shared/fiber/ReactChildFiber.js index c741d94bddf..5fb78c98b3b 100644 --- a/src/renderers/shared/fiber/ReactChildFiber.js +++ b/src/renderers/shared/fiber/ReactChildFiber.js @@ -192,19 +192,15 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { return existingChildren; } - function useFiber(fiber : Fiber, priority : PriorityLevel) : Fiber { + function useFiber(fiber : Fiber) : Fiber { // We currently set sibling to null and index to 0 here because it is easy // to forget to do before returning it. E.g. for the single child case. if (shouldClone) { - const clone = cloneFiber(fiber, priority); + const clone = cloneFiber(fiber); clone.index = 0; clone.sibling = null; return clone; } else { - // We override the pending priority even if it is higher, because if - // we're reconciling at a lower priority that means that this was - // down-prioritized. - fiber.pendingWorkPriority = priority; fiber.effectTag = NoEffect; fiber.index = 0; fiber.sibling = null; @@ -248,17 +244,16 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { function updateTextNode( returnFiber : Fiber, current : ?Fiber, - textContent : string, - priority : PriorityLevel + textContent : string ) { if (current == null || current.tag !== HostText) { // Insert - const created = createFiberFromText(textContent, priority); + const created = createFiberFromText(textContent); created.return = returnFiber; return created; } else { // Update - const existing = useFiber(current, priority); + const existing = useFiber(current); existing.pendingProps = textContent; existing.return = returnFiber; return existing; @@ -268,18 +263,17 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { function updateElement( returnFiber : Fiber, current : ?Fiber, - element : ReactElement, - priority : PriorityLevel + element : ReactElement ) : Fiber { if (current == null || current.type !== element.type) { // Insert - const created = createFiberFromElement(element, priority); + const created = createFiberFromElement(element); created.ref = coerceRef(current, element); created.return = returnFiber; return created; } else { // Move based on index - const existing = useFiber(current, priority); + const existing = useFiber(current); existing.ref = coerceRef(current, element); existing.pendingProps = element.props; existing.return = returnFiber; @@ -294,18 +288,17 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { function updateCoroutine( returnFiber : Fiber, current : ?Fiber, - coroutine : ReactCoroutine, - priority : PriorityLevel + coroutine : ReactCoroutine ) : Fiber { // TODO: Should this also compare handler to determine whether to reuse? if (current == null || current.tag !== CoroutineComponent) { // Insert - const created = createFiberFromCoroutine(coroutine, priority); + const created = createFiberFromCoroutine(coroutine); created.return = returnFiber; return created; } else { // Move based on index - const existing = useFiber(current, priority); + const existing = useFiber(current); existing.pendingProps = coroutine; existing.return = returnFiber; return existing; @@ -315,20 +308,19 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { function updateYield( returnFiber : Fiber, current : ?Fiber, - yieldNode : ReactYield, - priority : PriorityLevel + yieldNode : ReactYield ) : Fiber { // TODO: Should this also compare continuation to determine whether to reuse? if (current == null || current.tag !== YieldComponent) { // Insert const reifiedYield = createReifiedYield(yieldNode); - const created = createFiberFromYield(yieldNode, priority); + const created = createFiberFromYield(yieldNode); created.type = reifiedYield; created.return = returnFiber; return created; } else { // Move based on index - const existing = useFiber(current, priority); + const existing = useFiber(current); existing.type = createUpdatedReifiedYield( current.type, yieldNode @@ -341,8 +333,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { function updatePortal( returnFiber : Fiber, current : ?Fiber, - portal : ReactPortal, - priority : PriorityLevel + portal : ReactPortal ) : Fiber { if ( current == null || @@ -351,12 +342,12 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { current.stateNode.implementation !== portal.implementation ) { // Insert - const created = createFiberFromPortal(portal, priority); + const created = createFiberFromPortal(portal); created.return = returnFiber; return created; } else { // Update - const existing = useFiber(current, priority); + const existing = useFiber(current); existing.pendingProps = portal.children || []; existing.return = returnFiber; return existing; @@ -366,17 +357,16 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { function updateFragment( returnFiber : Fiber, current : ?Fiber, - fragment : Iterable<*>, - priority : PriorityLevel + fragment : Iterable<*> ) : Fiber { if (current == null || current.tag !== Fragment) { // Insert - const created = createFiberFromFragment(fragment, priority); + const created = createFiberFromFragment(fragment); created.return = returnFiber; return created; } else { // Update - const existing = useFiber(current, priority); + const existing = useFiber(current); existing.pendingProps = fragment; existing.return = returnFiber; return existing; @@ -385,14 +375,13 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { function createChild( returnFiber : Fiber, - newChild : any, - priority : PriorityLevel + newChild : any ) : ?Fiber { if (typeof newChild === 'string' || typeof newChild === 'number') { // Text nodes doesn't have keys. If the previous node is implicitly keyed // we can continue to replace it without aborting even if it is not a text // node. - const created = createFiberFromText('' + newChild, priority); + const created = createFiberFromText('' + newChild); created.return = returnFiber; return created; } @@ -400,35 +389,35 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { if (typeof newChild === 'object' && newChild !== null) { switch (newChild.$$typeof) { case REACT_ELEMENT_TYPE: { - const created = createFiberFromElement(newChild, priority); + const created = createFiberFromElement(newChild); created.ref = coerceRef(null, newChild); created.return = returnFiber; return created; } case REACT_COROUTINE_TYPE: { - const created = createFiberFromCoroutine(newChild, priority); + const created = createFiberFromCoroutine(newChild); created.return = returnFiber; return created; } case REACT_YIELD_TYPE: { const reifiedYield = createReifiedYield(newChild); - const created = createFiberFromYield(newChild, priority); + const created = createFiberFromYield(newChild); created.type = reifiedYield; created.return = returnFiber; return created; } case REACT_PORTAL_TYPE: { - const created = createFiberFromPortal(newChild, priority); + const created = createFiberFromPortal(newChild); created.return = returnFiber; return created; } } if (isArray(newChild) || getIteratorFn(newChild)) { - const created = createFiberFromFragment(newChild, priority); + const created = createFiberFromFragment(newChild); created.return = returnFiber; return created; } @@ -440,8 +429,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { function updateSlot( returnFiber : Fiber, oldFiber : ?Fiber, - newChild : any, - priority : PriorityLevel + newChild : any ) : ?Fiber { // Update the fiber if the keys match, otherwise return null. @@ -454,14 +442,14 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { if (key !== null) { return null; } - return updateTextNode(returnFiber, oldFiber, '' + newChild, priority); + return updateTextNode(returnFiber, oldFiber, '' + newChild); } if (typeof newChild === 'object' && newChild !== null) { switch (newChild.$$typeof) { case REACT_ELEMENT_TYPE: { if (newChild.key === key) { - return updateElement(returnFiber, oldFiber, newChild, priority); + return updateElement(returnFiber, oldFiber, newChild); } else { return null; } @@ -469,7 +457,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { case REACT_COROUTINE_TYPE: { if (newChild.key === key) { - return updateCoroutine(returnFiber, oldFiber, newChild, priority); + return updateCoroutine(returnFiber, oldFiber, newChild); } else { return null; } @@ -477,7 +465,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { case REACT_YIELD_TYPE: { if (newChild.key === key) { - return updateYield(returnFiber, oldFiber, newChild, priority); + return updateYield(returnFiber, oldFiber, newChild); } else { return null; } @@ -490,7 +478,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { if (key !== null) { return null; } - return updateFragment(returnFiber, oldFiber, newChild, priority); + return updateFragment(returnFiber, oldFiber, newChild); } } @@ -501,15 +489,14 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { existingChildren : Map, returnFiber : Fiber, newIdx : number, - newChild : any, - priority : PriorityLevel + newChild : any ) : ?Fiber { if (typeof newChild === 'string' || typeof newChild === 'number') { // Text nodes doesn't have keys, so we neither have to check the old nor // new node for the key. If both are text nodes, they match. const matchedFiber = existingChildren.get(newIdx) || null; - return updateTextNode(returnFiber, matchedFiber, '' + newChild, priority); + return updateTextNode(returnFiber, matchedFiber, '' + newChild); } if (typeof newChild === 'object' && newChild !== null) { @@ -518,34 +505,34 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { const matchedFiber = existingChildren.get( newChild.key === null ? newIdx : newChild.key ) || null; - return updateElement(returnFiber, matchedFiber, newChild, priority); + return updateElement(returnFiber, matchedFiber, newChild); } case REACT_COROUTINE_TYPE: { const matchedFiber = existingChildren.get( newChild.key === null ? newIdx : newChild.key ) || null; - return updateCoroutine(returnFiber, matchedFiber, newChild, priority); + return updateCoroutine(returnFiber, matchedFiber, newChild); } case REACT_YIELD_TYPE: { const matchedFiber = existingChildren.get( newChild.key === null ? newIdx : newChild.key ) || null; - return updateYield(returnFiber, matchedFiber, newChild, priority); + return updateYield(returnFiber, matchedFiber, newChild); } case REACT_PORTAL_TYPE: { const matchedFiber = existingChildren.get( newChild.key === null ? newIdx : newChild.key ) || null; - return updatePortal(returnFiber, matchedFiber, newChild, priority); + return updatePortal(returnFiber, matchedFiber, newChild); } } if (isArray(newChild) || getIteratorFn(newChild)) { const matchedFiber = existingChildren.get(newIdx) || null; - return updateFragment(returnFiber, matchedFiber, newChild, priority); + return updateFragment(returnFiber, matchedFiber, newChild); } } @@ -597,8 +584,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { function reconcileChildrenArray( returnFiber : Fiber, currentFirstChild : ?Fiber, - newChildren : Array<*>, - priority : PriorityLevel) : ?Fiber { + newChildren : Array<*>) : ?Fiber { // This algorithm can't optimize by searching from boths ends since we // don't have backpointers on fibers. I'm trying to see how far we can get @@ -647,8 +633,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { const newFiber = updateSlot( returnFiber, oldFiber, - newChildren[newIdx], - priority + newChildren[newIdx] ); if (!newFiber) { // TODO: This breaks on empty slots like null children. That's @@ -694,8 +679,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { for (; newIdx < newChildren.length; newIdx++) { const newFiber = createChild( returnFiber, - newChildren[newIdx], - priority + newChildren[newIdx] ); if (!newFiber) { continue; @@ -721,8 +705,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { existingChildren, returnFiber, newIdx, - newChildren[newIdx], - priority + newChildren[newIdx] ); if (newFiber) { if (shouldTrackSideEffects) { @@ -758,8 +741,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { function reconcileChildrenIterator( returnFiber : Fiber, currentFirstChild : ?Fiber, - newChildrenIterable : Iterable<*>, - priority : PriorityLevel) : ?Fiber { + newChildrenIterable : Iterable<*>) : ?Fiber { // This is the same implementation as reconcileChildrenArray(), // but using the iterator instead. @@ -810,8 +792,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { const newFiber = updateSlot( returnFiber, oldFiber, - step.value, - priority + step.value ); if (!newFiber) { // TODO: This breaks on empty slots like null children. That's @@ -857,8 +838,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { for (; !step.done; newIdx++, step = newChildren.next()) { const newFiber = createChild( returnFiber, - step.value, - priority + step.value ); if (!newFiber) { continue; @@ -884,8 +864,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { existingChildren, returnFiber, newIdx, - step.value, - priority + step.value ); if (newFiber) { if (shouldTrackSideEffects) { @@ -921,8 +900,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { function reconcileSingleTextNode( returnFiber : Fiber, currentFirstChild : ?Fiber, - textContent : string, - priority : PriorityLevel + textContent : string ) : Fiber { // There's no need to check for keys on text nodes since we don't have a // way to define them. @@ -930,7 +908,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { // We already have an existing node so let's just update it and delete // the rest. deleteRemainingChildren(returnFiber, currentFirstChild.sibling); - const existing = useFiber(currentFirstChild, priority); + const existing = useFiber(currentFirstChild); existing.pendingProps = textContent; existing.return = returnFiber; return existing; @@ -938,7 +916,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { // The existing first child is not a text node so we need to create one // and delete the existing ones. deleteRemainingChildren(returnFiber, currentFirstChild); - const created = createFiberFromText(textContent, priority); + const created = createFiberFromText(textContent); created.return = returnFiber; return created; } @@ -946,8 +924,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { function reconcileSingleElement( returnFiber : Fiber, currentFirstChild : ?Fiber, - element : ReactElement, - priority : PriorityLevel + element : ReactElement ) : Fiber { const key = element.key; let child = currentFirstChild; @@ -957,7 +934,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { if (child.key === key) { if (child.type === element.type) { deleteRemainingChildren(returnFiber, child.sibling); - const existing = useFiber(child, priority); + const existing = useFiber(child); existing.ref = coerceRef(child, element); existing.pendingProps = element.props; existing.return = returnFiber; @@ -976,7 +953,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { child = child.sibling; } - const created = createFiberFromElement(element, priority); + const created = createFiberFromElement(element); created.ref = coerceRef(currentFirstChild, element); created.return = returnFiber; return created; @@ -985,8 +962,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { function reconcileSingleCoroutine( returnFiber : Fiber, currentFirstChild : ?Fiber, - coroutine : ReactCoroutine, - priority : PriorityLevel + coroutine : ReactCoroutine ) : Fiber { const key = coroutine.key; let child = currentFirstChild; @@ -996,7 +972,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { if (child.key === key) { if (child.tag === CoroutineComponent) { deleteRemainingChildren(returnFiber, child.sibling); - const existing = useFiber(child, priority); + const existing = useFiber(child); existing.pendingProps = coroutine; existing.return = returnFiber; return existing; @@ -1010,7 +986,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { child = child.sibling; } - const created = createFiberFromCoroutine(coroutine, priority); + const created = createFiberFromCoroutine(coroutine); created.return = returnFiber; return created; } @@ -1018,8 +994,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { function reconcileSingleYield( returnFiber : Fiber, currentFirstChild : ?Fiber, - yieldNode : ReactYield, - priority : PriorityLevel + yieldNode : ReactYield ) : Fiber { const key = yieldNode.key; let child = currentFirstChild; @@ -1029,7 +1004,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { if (child.key === key) { if (child.tag === YieldComponent) { deleteRemainingChildren(returnFiber, child.sibling); - const existing = useFiber(child, priority); + const existing = useFiber(child); existing.type = createUpdatedReifiedYield( child.type, yieldNode @@ -1047,7 +1022,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { } const reifiedYield = createReifiedYield(yieldNode); - const created = createFiberFromYield(yieldNode, priority); + const created = createFiberFromYield(yieldNode); created.type = reifiedYield; created.return = returnFiber; return created; @@ -1056,8 +1031,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { function reconcileSinglePortal( returnFiber : Fiber, currentFirstChild : ?Fiber, - portal : ReactPortal, - priority : PriorityLevel + portal : ReactPortal ) : Fiber { const key = portal.key; let child = currentFirstChild; @@ -1071,7 +1045,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { child.stateNode.implementation === portal.implementation ) { deleteRemainingChildren(returnFiber, child.sibling); - const existing = useFiber(child, priority); + const existing = useFiber(child); existing.pendingProps = portal.children || []; existing.return = returnFiber; return existing; @@ -1085,7 +1059,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { child = child.sibling; } - const created = createFiberFromPortal(portal, priority); + const created = createFiberFromPortal(portal); created.return = returnFiber; return created; } @@ -1097,8 +1071,10 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { returnFiber : Fiber, currentFirstChild : ?Fiber, newChild : any, - priority : PriorityLevel + priorityLevel : PriorityLevel ) : ?Fiber { + returnFiber.pendingWorkPriority = priorityLevel; + // This function is not recursive. // If the top level item is an array, we treat it as a set of children, // not as a fragment. Nested arrays on the other hand will be treated as @@ -1170,8 +1146,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { return placeSingleChild(reconcileSingleTextNode( returnFiber, currentFirstChild, - '' + newChild, - priority + '' + newChild )); } @@ -1181,32 +1156,28 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { return placeSingleChild(reconcileSingleElement( returnFiber, currentFirstChild, - newChild, - priority + newChild )); case REACT_COROUTINE_TYPE: return placeSingleChild(reconcileSingleCoroutine( returnFiber, currentFirstChild, - newChild, - priority + newChild )); case REACT_YIELD_TYPE: return placeSingleChild(reconcileSingleYield( returnFiber, currentFirstChild, - newChild, - priority + newChild )); case REACT_PORTAL_TYPE: return placeSingleChild(reconcileSinglePortal( returnFiber, currentFirstChild, - newChild, - priority + newChild )); } @@ -1214,8 +1185,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { return reconcileChildrenArray( returnFiber, currentFirstChild, - newChild, - priority + newChild ); } @@ -1223,8 +1193,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { return reconcileChildrenIterator( returnFiber, currentFirstChild, - newChild, - priority + newChild ); } } @@ -1276,6 +1245,10 @@ exports.cloneChildFibers = function(current : ?Fiber, workInProgress : Fiber) : if (!workInProgress.child) { return; } + + // cloneChildFibers is only called when bailing out on already finished work. + // So the pendingWorkPriority should remain at its existing value. + if (current && workInProgress.child === current.child) { // We use workInProgress.child since that lets Flow know that it can't be // null since we validated that already. However, as the line above suggests @@ -1288,16 +1261,13 @@ exports.cloneChildFibers = function(current : ?Fiber, workInProgress : Fiber) : // observable when the first sibling has lower priority work remaining // than the next sibling. At that point we should add tests that catches // this. - let newChild = cloneFiber(currentChild, currentChild.pendingWorkPriority); + let newChild = cloneFiber(currentChild); workInProgress.child = newChild; newChild.return = workInProgress; while (currentChild.sibling) { currentChild = currentChild.sibling; - newChild = newChild.sibling = cloneFiber( - currentChild, - currentChild.pendingWorkPriority - ); + newChild = newChild.sibling = cloneFiber(currentChild); newChild.return = workInProgress; } newChild.sibling = null; diff --git a/src/renderers/shared/fiber/ReactFiber.js b/src/renderers/shared/fiber/ReactFiber.js index 3657d087bb8..3824df543d9 100644 --- a/src/renderers/shared/fiber/ReactFiber.js +++ b/src/renderers/shared/fiber/ReactFiber.js @@ -128,6 +128,8 @@ export type Fiber = { lastEffect: ?Fiber, // This will be used to quickly determine if a subtree has no pending changes. + // It represents the priority of the child subtree, not including the + // fiber itself. pendingWorkPriority: PriorityLevel, // This value represents the priority level that was last used to process this @@ -234,7 +236,7 @@ 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 { +exports.cloneFiber = function(fiber : Fiber) : 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 @@ -278,7 +280,7 @@ exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fi // TODO: Pass in the new pendingProps as an argument maybe? alt.pendingProps = fiber.pendingProps; cloneUpdateQueue(fiber, alt); - alt.pendingWorkPriority = priorityLevel; + alt.pendingWorkPriority = fiber.pendingWorkPriority; alt.memoizedProps = fiber.memoizedProps; alt.memoizedState = fiber.memoizedState; @@ -297,15 +299,13 @@ exports.createHostRootFiber = function() : Fiber { return fiber; }; -exports.createFiberFromElement = function(element : ReactElement, priorityLevel : PriorityLevel) : Fiber { +exports.createFiberFromElement = function(element : ReactElement) : Fiber { let owner = null; if (__DEV__) { owner = element._owner; } - const fiber = createFiberFromElementType(element.type, element.key, owner); fiber.pendingProps = element.props; - fiber.pendingWorkPriority = priorityLevel; if (__DEV__) { fiber._debugSource = element._source; @@ -315,19 +315,17 @@ exports.createFiberFromElement = function(element : ReactElement, priorityLevel return fiber; }; -exports.createFiberFromFragment = function(elements : ReactFragment, priorityLevel : PriorityLevel) : Fiber { +exports.createFiberFromFragment = function(elements : ReactFragment) : Fiber { // TODO: Consider supporting keyed fragments. Technically, we accidentally // support that in the existing React. const fiber = createFiber(Fragment, null); fiber.pendingProps = elements; - fiber.pendingWorkPriority = priorityLevel; return fiber; }; -exports.createFiberFromText = function(content : string, priorityLevel : PriorityLevel) : Fiber { +exports.createFiberFromText = function(content : string) : Fiber { const fiber = createFiber(HostText, null); fiber.pendingProps = content; - fiber.pendingWorkPriority = priorityLevel; return fiber; }; @@ -384,24 +382,22 @@ function createFiberFromElementType(type : mixed, key : null | string, debugOwne exports.createFiberFromElementType = createFiberFromElementType; -exports.createFiberFromCoroutine = function(coroutine : ReactCoroutine, priorityLevel : PriorityLevel) : Fiber { +exports.createFiberFromCoroutine = function(coroutine : ReactCoroutine) : Fiber { const fiber = createFiber(CoroutineComponent, coroutine.key); fiber.type = coroutine.handler; fiber.pendingProps = coroutine; - fiber.pendingWorkPriority = priorityLevel; return fiber; }; -exports.createFiberFromYield = function(yieldNode : ReactYield, priorityLevel : PriorityLevel) : Fiber { +exports.createFiberFromYield = function(yieldNode : ReactYield) : Fiber { const fiber = createFiber(YieldComponent, yieldNode.key); fiber.pendingProps = {}; return fiber; }; -exports.createFiberFromPortal = function(portal : ReactPortal, priorityLevel : PriorityLevel) : Fiber { +exports.createFiberFromPortal = function(portal : ReactPortal) : Fiber { const fiber = createFiber(HostPortal, portal.key); fiber.pendingProps = portal.children || []; - fiber.pendingWorkPriority = priorityLevel; fiber.stateNode = { containerInfo: portal.containerInfo, implementation: portal.implementation, diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index dbbb9146695..0ee676556f5 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -121,11 +121,6 @@ module.exports = function( workInProgress.lastEffect = workInProgress.progressedLastDeletion; } - 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. @@ -174,7 +169,7 @@ module.exports = function( markChildAsProgressed(current, workInProgress, priorityLevel); } - function updateFragment(current, workInProgress) { + function updateFragment(current, workInProgress, priorityLevel) { var nextChildren = workInProgress.pendingProps; if (hasContextChanged()) { // Normally we can bail out on props equality but if context has changed @@ -183,9 +178,9 @@ module.exports = function( nextChildren = workInProgress.memoizedProps; } } else if (nextChildren === null || workInProgress.memoizedProps === nextChildren) { - return bailoutOnAlreadyFinishedWork(current, workInProgress); + return bailoutOnAlreadyFinishedWork(current, workInProgress, priorityLevel); } - reconcileChildren(current, workInProgress, nextChildren); + reconcileChildrenAtPriority(current, workInProgress, nextChildren, priorityLevel); memoizeProps(workInProgress, nextChildren); return workInProgress.child; } @@ -198,7 +193,7 @@ module.exports = function( } } - function updateFunctionalComponent(current, workInProgress) { + function updateFunctionalComponent(current, workInProgress, priorityLevel) { var fn = workInProgress.type; var nextProps = workInProgress.pendingProps; @@ -211,7 +206,7 @@ module.exports = function( } } else { if (nextProps == null || memoizedProps === nextProps) { - return bailoutOnAlreadyFinishedWork(current, workInProgress); + return bailoutOnAlreadyFinishedWork(current, workInProgress, priorityLevel); } // 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. @@ -219,7 +214,7 @@ module.exports = function( !fn.shouldComponentUpdate(memoizedProps, nextProps)) { // Memoize props even if shouldComponentUpdate returns false memoizeProps(workInProgress, nextProps); - return bailoutOnAlreadyFinishedWork(current, workInProgress); + return bailoutOnAlreadyFinishedWork(current, workInProgress, priorityLevel); } } @@ -234,7 +229,7 @@ module.exports = function( } else { nextChildren = fn(nextProps, context); } - reconcileChildren(current, workInProgress, nextChildren); + reconcileChildrenAtPriority(current, workInProgress, nextChildren, priorityLevel); memoizeProps(workInProgress, nextProps); return workInProgress.child; } @@ -259,7 +254,7 @@ module.exports = function( } else { shouldUpdate = updateClassInstance(current, workInProgress, priorityLevel); } - return finishClassComponent(current, workInProgress, shouldUpdate, hasContext); + return finishClassComponent(current, workInProgress, shouldUpdate, hasContext, priorityLevel); } function finishClassComponent( @@ -267,12 +262,13 @@ module.exports = function( workInProgress : Fiber, shouldUpdate : boolean, hasContext : boolean, + priorityLevel : PriorityLevel ) { // Refs should update even if shouldComponentUpdate returns false markRef(current, workInProgress); if (!shouldUpdate) { - return bailoutOnAlreadyFinishedWork(current, workInProgress); + return bailoutOnAlreadyFinishedWork(current, workInProgress, priorityLevel); } const instance = workInProgress.stateNode; @@ -280,7 +276,7 @@ module.exports = function( // Rerender ReactCurrentOwner.current = workInProgress; const nextChildren = instance.render(); - reconcileChildren(current, workInProgress, nextChildren); + reconcileChildrenAtPriority(current, workInProgress, nextChildren, priorityLevel); // Memoize props and state using the values we just used to render. // TODO: Restructure so we never read values from the instance. memoizeState(workInProgress, instance.state); @@ -319,18 +315,18 @@ module.exports = function( if (prevState === state) { // If the state is the same as before, that's a bailout because we had // no work matching this priority. - return bailoutOnAlreadyFinishedWork(current, workInProgress); + return bailoutOnAlreadyFinishedWork(current, workInProgress, priorityLevel); } const element = state.element; - reconcileChildren(current, workInProgress, element); + reconcileChildrenAtPriority(current, workInProgress, element, priorityLevel); memoizeState(workInProgress, state); return workInProgress.child; } // If there is no update queue, that's a bailout because the root has no props. - return bailoutOnAlreadyFinishedWork(current, workInProgress); + return bailoutOnAlreadyFinishedWork(current, workInProgress, priorityLevel); } - function updateHostComponent(current, workInProgress) { + function updateHostComponent(current, workInProgress, priorityLevel) { pushHostContext(workInProgress); let nextProps = workInProgress.pendingProps; @@ -346,8 +342,7 @@ module.exports = function( } } } else if (nextProps === null || memoizedProps === nextProps) { - if (memoizedProps.hidden && - workInProgress.pendingWorkPriority !== OffscreenPriority) { + if (memoizedProps.hidden && priorityLevel !== OffscreenPriority) { // This subtree still has work, but it should be deprioritized so we need // to bail out and not do any work yet. // TODO: It would be better if this tree got its correct priority set @@ -355,16 +350,9 @@ module.exports = function( // priority reconciliation first before we can get down here. However, // that is a bit tricky since workInProgress and current can have // different "hidden" settings. - let child = workInProgress.progressedChild; - while (child) { - // To ensure that this subtree gets its priority reset, the children - // need to be reset. - child.pendingWorkPriority = OffscreenPriority; - child = child.sibling; - } return null; } - return bailoutOnAlreadyFinishedWork(current, workInProgress); + return bailoutOnAlreadyFinishedWork(current, workInProgress, priorityLevel); } let nextChildren = nextProps.children; @@ -387,8 +375,7 @@ module.exports = function( markRef(current, workInProgress); - if (nextProps.hidden && - workInProgress.pendingWorkPriority !== OffscreenPriority) { + if (nextProps.hidden && priorityLevel !== OffscreenPriority) { // If this host component is hidden, we can bail out on the children. // We'll rerender the children later at the lower priority. @@ -424,7 +411,7 @@ module.exports = function( // Abort and don't process children yet. return null; } else { - reconcileChildren(current, workInProgress, nextChildren); + reconcileChildrenAtPriority(current, workInProgress, nextChildren, priorityLevel); memoizeProps(workInProgress, nextProps); return workInProgress.child; } @@ -469,7 +456,7 @@ module.exports = function( const hasContext = pushContextProvider(workInProgress); adoptClassInstance(workInProgress, value); mountClassInstance(workInProgress, priorityLevel); - return finishClassComponent(current, workInProgress, true, hasContext); + return finishClassComponent(current, workInProgress, true, hasContext, priorityLevel); } else { // Proceed under the assumption that this is a functional component workInProgress.tag = FunctionalComponent; @@ -498,13 +485,13 @@ module.exports = function( } } } - reconcileChildren(current, workInProgress, value); + reconcileChildrenAtPriority(current, workInProgress, value, priorityLevel); memoizeProps(workInProgress, props); return workInProgress.child; } } - function updateCoroutineComponent(current, workInProgress) { + function updateCoroutineComponent(current, workInProgress, priorityLevel) { var nextCoroutine = (workInProgress.pendingProps : null | ReactCoroutine); if (hasContextChanged()) { // Normally we can bail out on props equality but if context has changed @@ -516,18 +503,17 @@ module.exports = function( } } } else if (nextCoroutine === null || workInProgress.memoizedProps === nextCoroutine) { - return bailoutOnAlreadyFinishedWork(current, workInProgress); + return bailoutOnAlreadyFinishedWork(current, workInProgress, priorityLevel); } - reconcileChildren(current, workInProgress, nextCoroutine.children); + reconcileChildrenAtPriority(current, workInProgress, nextCoroutine.children, priorityLevel); memoizeProps(workInProgress, nextCoroutine); // This doesn't take arbitrary time so we could synchronously just begin // eagerly do the work of workInProgress.child as an optimization. return workInProgress.child; } - function updatePortalComponent(current, workInProgress) { + function updatePortalComponent(current, workInProgress, priorityLevel) { pushHostContainer(workInProgress, workInProgress.stateNode.containerInfo); - const priorityLevel = workInProgress.pendingWorkPriority; let nextChildren = workInProgress.pendingProps; if (hasContextChanged()) { // Normally we can bail out on props equality but if context has changed @@ -539,7 +525,7 @@ module.exports = function( } } } else if (nextChildren === null || workInProgress.memoizedProps === nextChildren) { - return bailoutOnAlreadyFinishedWork(current, workInProgress); + return bailoutOnAlreadyFinishedWork(current, workInProgress, priorityLevel); } if (!current) { @@ -557,7 +543,7 @@ module.exports = function( memoizeProps(workInProgress, nextChildren); markChildAsProgressed(current, workInProgress, priorityLevel); } else { - reconcileChildren(current, workInProgress, nextChildren); + reconcileChildrenAtPriority(current, workInProgress, nextChildren, priorityLevel); memoizeProps(workInProgress, nextChildren); } return workInProgress.child; @@ -582,13 +568,14 @@ module.exports = function( } */ - function bailoutOnAlreadyFinishedWork(current, workInProgress : Fiber) : ?Fiber { - const priorityLevel = workInProgress.pendingWorkPriority; - // 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. + function bailoutOnAlreadyFinishedWork( + current : ?Fiber, + workInProgress : Fiber, + priorityLevel : PriorityLevel + ) : ?Fiber { + // TODO: We should be able to bail out early if the children have no more + // work to do and have already completed. However, we currently don't have + // a way of knowing if the child has completed. // 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. @@ -604,25 +591,17 @@ module.exports = function( clearDeletions(workInProgress); } - cloneChildFibers(current, workInProgress); - markChildAsProgressed(current, workInProgress, priorityLevel); - return workInProgress.child; - } - - function bailoutOnLowPriority(current, workInProgress) { - // TODO: Handle HostComponent tags here as well and call pushHostContext()? - // See PR 8590 discussion for context - switch (workInProgress.tag) { - case ClassComponent: - pushContextProvider(workInProgress); - break; - case HostPortal: - pushHostContainer(workInProgress, workInProgress.stateNode.containerInfo); - break; + if (workInProgress.pendingWorkPriority === NoWork || + workInProgress.pendingWorkPriority > priorityLevel) { + // The work in the child's subtree does not have sufficient priority. + // Bail out. + return null; + } else { + // Keep the existing progressed priority + cloneChildFibers(current, workInProgress); + markChildAsProgressed(current, workInProgress, workInProgress.progressedPriority); + return workInProgress.child; } - // TODO: What if this is currently in progress? - // How can that happen? How is this not being cloned? - return null; } function memoizeProps(workInProgress : Fiber, nextProps : any) { @@ -638,11 +617,6 @@ module.exports = function( } function beginWork(current : ?Fiber, workInProgress : Fiber, priorityLevel : PriorityLevel) : ?Fiber { - if (workInProgress.pendingWorkPriority === NoWork || - workInProgress.pendingWorkPriority > priorityLevel) { - return bailoutOnLowPriority(current, workInProgress); - } - if (__DEV__) { ReactDebugCurrentFiber.current = workInProgress; } @@ -654,7 +628,7 @@ module.exports = function( if (workInProgress.progressedPriority === priorityLevel) { // If we have progressed work on this priority level already, we can - // proceed this that as the child. + // proceed with that as the child. workInProgress.child = workInProgress.progressedChild; } @@ -662,29 +636,29 @@ module.exports = function( case IndeterminateComponent: return mountIndeterminateComponent(current, workInProgress, priorityLevel); case FunctionalComponent: - return updateFunctionalComponent(current, workInProgress); + return updateFunctionalComponent(current, workInProgress, priorityLevel); case ClassComponent: return updateClassComponent(current, workInProgress, priorityLevel); case HostRoot: return updateHostRoot(current, workInProgress, priorityLevel); case HostComponent: - return updateHostComponent(current, workInProgress); + return updateHostComponent(current, workInProgress, priorityLevel); case HostText: - return updateHostText(current, workInProgress); + return updateHostText(current, workInProgress, priorityLevel); case CoroutineHandlerPhase: // This is a restart. Reset the tag to the initial phase. workInProgress.tag = CoroutineComponent; // Intentionally fall through since this is now the same. case CoroutineComponent: - return updateCoroutineComponent(current, workInProgress); + return updateCoroutineComponent(current, workInProgress, priorityLevel); case YieldComponent: // A yield component is just a placeholder, we can just run through the // next one immediately. return null; case HostPortal: - return updatePortalComponent(current, workInProgress); + return updatePortalComponent(current, workInProgress, priorityLevel); case Fragment: - return updateFragment(current, workInProgress); + return updateFragment(current, workInProgress, priorityLevel); default: throw new Error('Unknown unit of work tag'); } @@ -699,11 +673,6 @@ module.exports = function( // Add an error effect so we can handle the error during the commit phase workInProgress.effectTag |= Err; - if (workInProgress.pendingWorkPriority === NoWork || - workInProgress.pendingWorkPriority > priorityLevel) { - return bailoutOnLowPriority(current, workInProgress); - } - // If we don't bail out, we're going be recomputing our children so we need // to drop our effect list. workInProgress.firstEffect = null; @@ -711,7 +680,7 @@ module.exports = function( // Unmount the current children as if the component rendered null const nextChildren = null; - reconcileChildren(current, workInProgress, nextChildren); + reconcileChildrenAtPriority(current, workInProgress, nextChildren, priorityLevel); if (workInProgress.tag === ClassComponent) { const instance = workInProgress.stateNode; diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index 776db30ecc0..40d81c6be20 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -18,6 +18,7 @@ import type { HostContext } from 'ReactFiberHostContext'; import type { FiberRoot } from 'ReactFiberRoot'; import type { HostConfig } from 'ReactFiberReconciler'; import type { ReifiedYield } from 'ReactReifiedYield'; +import type { PriorityLevel } from 'ReactPriorityLevel'; var { reconcileChildFibers } = require('ReactChildFiber'); var { @@ -25,6 +26,7 @@ var { } = require('ReactFiberContext'); var ReactTypeOfWork = require('ReactTypeOfWork'); var ReactTypeOfSideEffect = require('ReactTypeOfSideEffect'); +var ReactPriorityLevel = require('ReactPriorityLevel'); var { IndeterminateComponent, FunctionalComponent, @@ -42,6 +44,10 @@ var { Ref, Update, } = ReactTypeOfSideEffect; +var { + NoWork, + OffscreenPriority, +} = ReactPriorityLevel; if (__DEV__) { var ReactDebugCurrentFiber = require('ReactDebugCurrentFiber'); @@ -50,6 +56,7 @@ if (__DEV__) { module.exports = function( config : HostConfig, hostContext : HostContext, + getUpdateAndChildPriority : (fiber: Fiber) => PriorityLevel, ) { const { createInstance, @@ -66,6 +73,25 @@ module.exports = function( popHostContainer, } = hostContext; + function resetWorkPriority(workInProgress : Fiber) { + let newPriority = NoWork; + // 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) { + const childPriority = getUpdateAndChildPriority(child); + // Ensure that remaining work priority bubbles up. + if (childPriority !== NoWork && + (newPriority === NoWork || + newPriority > childPriority)) { + newPriority = childPriority; + } + child = child.sibling; + } + workInProgress.pendingWorkPriority = newPriority; + } + function markUpdate(workInProgress : Fiber) { // Tag the fiber with an update effect. This turns a Placement into // an UpdateAndPlacement. @@ -164,17 +190,19 @@ module.exports = function( } } - function completeWork(current : ?Fiber, workInProgress : Fiber) : ?Fiber { + function completeWork(current : ?Fiber, workInProgress : Fiber, priorityLevel : PriorityLevel) : ?Fiber { if (__DEV__) { ReactDebugCurrentFiber.current = workInProgress; } switch (workInProgress.tag) { case FunctionalComponent: + resetWorkPriority(workInProgress); return null; case ClassComponent: { // We are leaving this subtree, so pop context if any. popContextProvider(workInProgress); + resetWorkPriority(workInProgress); return null; } case HostRoot: { @@ -184,6 +212,7 @@ module.exports = function( fiberRoot.context = fiberRoot.pendingContext; fiberRoot.pendingContext = null; } + resetWorkPriority(workInProgress); return null; } case HostComponent: { @@ -247,6 +276,15 @@ module.exports = function( workInProgress.effectTag |= Ref; } } + if (newProps.hidden && + priorityLevel !== NoWork && + priorityLevel < OffscreenPriority) { + // If this node is hidden, and we're reconciling at higher than + // offscreen priority, there's remaining work in the subtree. + workInProgress.pendingWorkPriority = OffscreenPriority; + } else { + resetWorkPriority(workInProgress); + } return null; } case HostText: { @@ -272,23 +310,30 @@ module.exports = function( const textInstance = createTextInstance(newText, rootContainerInstance, currentHostContext, workInProgress); workInProgress.stateNode = textInstance; } + resetWorkPriority(workInProgress); return null; } - case CoroutineComponent: - return moveCoroutineToHandlerPhase(current, workInProgress); + case CoroutineComponent: { + const next = moveCoroutineToHandlerPhase(current, workInProgress); + resetWorkPriority(workInProgress); + return next; + } case CoroutineHandlerPhase: // Reset the tag to now be a first phase coroutine. workInProgress.tag = CoroutineComponent; + resetWorkPriority(workInProgress); return null; case YieldComponent: // Does nothing. return null; case Fragment: + resetWorkPriority(workInProgress); return null; case HostPortal: // TODO: Only mark this as an update if we have any pending callbacks. markUpdate(workInProgress); popHostContainer(workInProgress); + resetWorkPriority(workInProgress); return null; // Error cases diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 773ac5c83a2..2531e7d9210 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -75,6 +75,7 @@ var { var { getPendingPriority, + addTopLevelUpdate, } = require('ReactFiberUpdateQueue'); var { @@ -97,7 +98,11 @@ module.exports = function(config : HostConfig(config : HostConfig updatePriority) && updatePriority !== NoWork) { + return updatePriority; + } else { + return childPriority; + } + } + // findNextUnitOfWork mutates the current priority context. It is reset after // after the workLoop exits, so never call findNextUnitOfWork from outside // the work loop. function findNextUnitOfWork() { // Clear out roots with no more work on them, or if they have uncaught errors - while (nextScheduledRoot && nextScheduledRoot.current.pendingWorkPriority === NoWork) { + while (nextScheduledRoot && getUpdateAndChildPriority(nextScheduledRoot.current) === NoWork) { // Unschedule this root. nextScheduledRoot.isScheduled = false; // Read the next pointer now. @@ -212,10 +230,11 @@ module.exports = function(config : HostConfig root.current.pendingWorkPriority)) { - highestPriorityLevel = root.current.pendingWorkPriority; + highestPriorityLevel > rootPriority)) { + highestPriorityLevel = rootPriority; highestPriorityRoot = root; } // We didn't find anything to do in this root, so let's try the next one. @@ -232,10 +251,7 @@ module.exports = function(config : HostConfig(config : HostConfig 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. @@ -457,13 +447,11 @@ module.exports = function(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig { // We're now rendering an update that will bail out on updating middle. ReactNoop.render(); - ReactNoop.flushDeferredPri(45 + 5); + ReactNoop.flushDeferredPri(45 + 10); expect(ops).toEqual(['Foo', 'Bar', 'Bar']); @@ -428,7 +428,7 @@ describe('ReactIncremental', () => { // Let us try this again without fully finishing the first time. This will // create a hanging subtree that is reconciling at the normal priority. ReactNoop.render(); - ReactNoop.flushDeferredPri(40); + ReactNoop.flushDeferredPri(35); expect(ops).toEqual(['Foo', 'Bar']); From aabcaa6d8ebaaeb53f338c4183a8756e898f5819 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Tue, 10 Jan 2017 16:57:31 -0800 Subject: [PATCH 02/14] Ensure effects are reused when bailing out on the child --- scripts/fiber/tests-failing.txt | 3 --- scripts/fiber/tests-passing.txt | 1 + .../shared/fiber/ReactFiberScheduler.js | 23 +++++++++++++++++++ .../ReactIncrementalSideEffects-test.js | 2 +- 4 files changed, 25 insertions(+), 4 deletions(-) diff --git a/scripts/fiber/tests-failing.txt b/scripts/fiber/tests-failing.txt index 291ebe9525e..fdde0f21ec2 100644 --- a/scripts/fiber/tests-failing.txt +++ b/scripts/fiber/tests-failing.txt @@ -49,9 +49,6 @@ src/renderers/shared/__tests__/ReactPerf-test.js * should not count time in a portal towards lifecycle method * should work when measurement starts during reconciliation -src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js -* can defer side-effects and reuse them later - complex - src/renderers/shared/hooks/__tests__/ReactComponentTreeHook-test.js * can be retrieved by ID diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index e39919fc239..21c98427165 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -1244,6 +1244,7 @@ src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js * can update a completed tree before it has a chance to commit * updates a child even though the old props is empty * can defer side-effects and resume them later on +* can defer side-effects and reuse them later - complex * deprioritizes setStates that happens within a deprioritized tree * calls callback after update is flushed * calls setState callback even if component bails out diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 2531e7d9210..5cd6754cb04 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -518,6 +518,23 @@ module.exports = function(config : HostConfig(config : HostConfig { // Updated. span(1), div( + span(1), // Still not updated. span(0), - span(0), span(0) ) ), From 02a32fc735ed99201cf8a5d2c600691798f64f48 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 13 Jan 2017 13:50:46 -0800 Subject: [PATCH 03/14] When a boundary fails, set pendingWorkPriority to TaskPriority Workaround for the fact that host roots (which act as an error boundary for uncaught errors) do not have a parent. Because the child will be set to null on the next commit, anyway, I think this is fine. --- src/renderers/shared/fiber/ReactChildFiber.js | 2 -- src/renderers/shared/fiber/ReactFiberScheduler.js | 11 ++++++----- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/renderers/shared/fiber/ReactChildFiber.js b/src/renderers/shared/fiber/ReactChildFiber.js index 5fb78c98b3b..f7f9c363327 100644 --- a/src/renderers/shared/fiber/ReactChildFiber.js +++ b/src/renderers/shared/fiber/ReactChildFiber.js @@ -1090,7 +1090,6 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { returnFiber, currentFirstChild, newChild, - priority )); case REACT_PORTAL_TYPE: @@ -1098,7 +1097,6 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { returnFiber, currentFirstChild, newChild, - priority )); } } diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 5cd6754cb04..92323751d6d 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -75,7 +75,6 @@ var { var { getPendingPriority, - addTopLevelUpdate, } = require('ReactFiberUpdateQueue'); var { @@ -1120,10 +1119,12 @@ module.exports = function(config : HostConfig Date: Tue, 17 Jan 2017 16:01:45 -0800 Subject: [PATCH 04/14] Ensure that priority is not dropped from a progressed subtree Must bubble up priority from a progressed subtree that was preempted by a high priority update. We know this is the case if the progressed priority is lower than the priority at which we're reconciling. --- scripts/fiber/tests-passing.txt | 1 + .../shared/fiber/ReactFiberBeginWork.js | 1 + .../shared/fiber/ReactFiberCompleteWork.js | 36 +++++--- .../ReactIncrementalSideEffects-test.js | 88 +++++++++++++++++++ 4 files changed, 115 insertions(+), 11 deletions(-) diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 21c98427165..e932c965db9 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -1245,6 +1245,7 @@ src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js * updates a child even though the old props is empty * can defer side-effects and resume them later on * can defer side-effects and reuse them later - complex +* does not drop priority from a progressed subtree * deprioritizes setStates that happens within a deprioritized tree * calls callback after update is flushed * calls setState callback even if component bails out diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 0ee676556f5..ea774006fe9 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -629,6 +629,7 @@ module.exports = function( if (workInProgress.progressedPriority === priorityLevel) { // If we have progressed work on this priority level already, we can // proceed with that as the child. + workInProgress.pendingWorkPriority = priorityLevel; workInProgress.child = workInProgress.progressedChild; } diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index 40d81c6be20..edfc849e795 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -73,8 +73,22 @@ module.exports = function( popHostContainer, } = hostContext; - function resetWorkPriority(workInProgress : Fiber) { - let newPriority = NoWork; + function resetWorkPriority(workInProgress : Fiber, priorityLevel : PriorityLevel) { + // priorityLevel is the level we're currently reconciling at. It's called + // nextPriorityLevel in the scheduler. Can't think of a name that's + // not confusing. + + // If the progressedPriority is less than the priority we're currently + // reconciling at, this was a bailout. Set the work priority to the + // progressed priority, otherwise reset it to NoWork. + let newPriority; + if (workInProgress.progressedPriority === NoWork || + (workInProgress.progressedPriority > priorityLevel) && priorityLevel !== NoWork) { + newPriority = workInProgress.progressedPriority; + } else { + newPriority = NoWork; + } + // 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. @@ -197,12 +211,12 @@ module.exports = function( switch (workInProgress.tag) { case FunctionalComponent: - resetWorkPriority(workInProgress); + resetWorkPriority(workInProgress, priorityLevel); return null; case ClassComponent: { // We are leaving this subtree, so pop context if any. popContextProvider(workInProgress); - resetWorkPriority(workInProgress); + resetWorkPriority(workInProgress, priorityLevel); return null; } case HostRoot: { @@ -212,7 +226,7 @@ module.exports = function( fiberRoot.context = fiberRoot.pendingContext; fiberRoot.pendingContext = null; } - resetWorkPriority(workInProgress); + resetWorkPriority(workInProgress, priorityLevel); return null; } case HostComponent: { @@ -283,7 +297,7 @@ module.exports = function( // offscreen priority, there's remaining work in the subtree. workInProgress.pendingWorkPriority = OffscreenPriority; } else { - resetWorkPriority(workInProgress); + resetWorkPriority(workInProgress, priorityLevel); } return null; } @@ -310,30 +324,30 @@ module.exports = function( const textInstance = createTextInstance(newText, rootContainerInstance, currentHostContext, workInProgress); workInProgress.stateNode = textInstance; } - resetWorkPriority(workInProgress); + resetWorkPriority(workInProgress, priorityLevel); return null; } case CoroutineComponent: { const next = moveCoroutineToHandlerPhase(current, workInProgress); - resetWorkPriority(workInProgress); + resetWorkPriority(workInProgress, priorityLevel); return next; } case CoroutineHandlerPhase: // Reset the tag to now be a first phase coroutine. workInProgress.tag = CoroutineComponent; - resetWorkPriority(workInProgress); + resetWorkPriority(workInProgress, priorityLevel); return null; case YieldComponent: // Does nothing. return null; case Fragment: - resetWorkPriority(workInProgress); + resetWorkPriority(workInProgress, priorityLevel); return null; case HostPortal: // TODO: Only mark this as an update if we have any pending callbacks. markUpdate(workInProgress); popHostContainer(workInProgress); - resetWorkPriority(workInProgress); + resetWorkPriority(workInProgress, priorityLevel); return null; // Error cases diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js index b76d1a5d2de..826af52b804 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js @@ -756,6 +756,94 @@ describe('ReactIncrementalSideEffects', () => { expect(ops).toEqual(['Bar', 'Baz', 'Bar', 'Bar']); }); + it('does not drop priority from a progressed subtree', () => { + let ops = []; + let lowPri; + let highPri; + + function LowPriDidComplete() { + ops.push('LowPriDidComplete'); + // Because this is terminal, beginning work on LowPriDidComplete implies + // that LowPri will be completed before the scheduler yields. + return null; + } + + class LowPri extends React.Component { + state = { step: 0 }; + render() { + ops.push('LowPri'); + lowPri = this; + return [ + , + , + ]; + } + } + + function LowPriSibling() { + ops.push('LowPriSibling'); + return null; + } + + class HighPri extends React.Component { + state = { step: 0 }; + render() { + ops.push('HighPri'); + highPri = this; + return ; + } + } + + function App() { + ops.push('App'); + return [ +
+ + +
, +
, + ]; + } + + ReactNoop.render(); + ReactNoop.flush(); + expect(ReactNoop.getChildren()).toEqual([ + div(span(0)), + div(span(0)), + ]); + ops = []; + + lowPri.setState({ step: 1 }); + // Do just enough work to begin LowPri + ReactNoop.flushDeferredPri(30); + expect(ops).toEqual(['LowPri']); + // Now we'll do one more tick of work to complete LowPri. Because LowPri + // has a sibling, the parent div of LowPri has not yet completed. + ReactNoop.flushDeferredPri(10); + expect(ops).toEqual(['LowPri', 'LowPriDidComplete']); + expect(ReactNoop.getChildren()).toEqual([ + div(span(0)), // Complete, but not yet updated + div(span(0)), + ]); + ops = []; + + // Interrupt with high pri update + ReactNoop.performAnimationWork(() => highPri.setState({ step: 1 })); + ReactNoop.flushAnimationPri(); + expect(ops).toEqual(['HighPri']); + expect(ReactNoop.getChildren()).toEqual([ + div(span(0)), // Completed, but not yet updated + div(span(1)), + ]); + ops = []; + + ReactNoop.flush(); + expect(ReactNoop.getChildren()).toEqual([ + div(span(1)), + div(span(1)), + ]); + }); + it('deprioritizes setStates that happens within a deprioritized tree', () => { var ops = []; From d6a9c050fe54b04dc402f8387e50bd730b2d71ce Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Tue, 17 Jan 2017 16:29:51 -0800 Subject: [PATCH 05/14] Fix mistake in sCU test Without the change, the second update does not create higher priority work than the first one. --- .../shared/fiber/__tests__/ReactIncremental-test.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js index 7f7eea8bc34..fe3eafa5ccc 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js @@ -2225,7 +2225,7 @@ describe('ReactIncremental', () => { function Root(props) { return ( -