From d87c4d5e5ca3e82ea143ce1db5b2ee448482a12d Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Sat, 4 Mar 2017 04:23:09 +0000 Subject: [PATCH] [Fiber] Don't schedule class fibers without relevant lifecycles for commit (#9105) * Don't schedule class fibers without relevant lifecycles for commit * Separate Update and Ref effects * Simplify the exit condition * Add missing annotation * Write conditions differently to avoid an extra check * Inline markUpdateIfNecessary() * Inline markUpdateIfAlreadyInProgress() --- .../shared/fiber/ReactFiberClassComponent.js | 46 ++++++++++--------- .../shared/fiber/ReactFiberCommitWork.js | 39 ++++++---------- .../shared/fiber/ReactFiberCompleteWork.js | 11 ++++- .../shared/fiber/ReactFiberScheduler.js | 28 +++++++---- 4 files changed, 67 insertions(+), 57 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberClassComponent.js b/src/renderers/shared/fiber/ReactFiberClassComponent.js index 5af2249d3b..ae35f8db6d 100644 --- a/src/renderers/shared/fiber/ReactFiberClassComponent.js +++ b/src/renderers/shared/fiber/ReactFiberClassComponent.js @@ -227,22 +227,6 @@ module.exports = function( } } - - function markUpdate(workInProgress) { - workInProgress.effectTag |= Update; - } - - function markUpdateIfAlreadyInProgress(current: Fiber | null, workInProgress : Fiber) { - // If an update was already in progress, we should schedule an Update - // effect even though we're bailing out, so that cWU/cDU are called. - if (current !== null) { - if (workInProgress.memoizedProps !== current.memoizedProps || - workInProgress.memoizedState !== current.memoizedState) { - markUpdate(workInProgress); - } - } - } - function resetInputPointers(workInProgress : Fiber, instance : any) { instance.props = workInProgress.memoizedProps; instance.state = workInProgress.memoizedState; @@ -276,7 +260,6 @@ module.exports = function( // Invokes the mount life-cycles on a previously never rendered instance. function mountClassInstance(workInProgress : Fiber, priorityLevel : PriorityLevel) : void { - markUpdate(workInProgress); const instance = workInProgress.stateNode; const state = instance.state || null; @@ -310,12 +293,14 @@ module.exports = function( ); } } + if (typeof instance.componentDidMount === 'function') { + workInProgress.effectTag |= Update; + } } // Called on a preexisting class instance. Returns false if a resumed render // could be reused. function resumeMountClassInstance(workInProgress : Fiber, priorityLevel : PriorityLevel) : boolean { - markUpdate(workInProgress); const instance = workInProgress.stateNode; resetInputPointers(workInProgress, instance); @@ -378,6 +363,9 @@ module.exports = function( priorityLevel ); } + if (typeof instance.componentDidMount === 'function') { + workInProgress.effectTag |= Update; + } return true; } @@ -447,7 +435,14 @@ module.exports = function( oldState === newState && !hasContextChanged() && !(updateQueue !== null && updateQueue.hasForceUpdate)) { - markUpdateIfAlreadyInProgress(current, workInProgress); + // If an update was already in progress, we should schedule an Update + // effect even though we're bailing out, so that cWU/cDU are called. + if (typeof instance.componentDidUpdate === 'function') { + if (oldProps !== current.memoizedProps || + oldState !== current.memoizedState) { + workInProgress.effectTag |= Update; + } + } return false; } @@ -461,12 +456,21 @@ module.exports = function( ); if (shouldUpdate) { - markUpdate(workInProgress); if (typeof instance.componentWillUpdate === 'function') { instance.componentWillUpdate(newProps, newState, newContext); } + if (typeof instance.componentDidUpdate === 'function') { + workInProgress.effectTag |= Update; + } } else { - markUpdateIfAlreadyInProgress(current, workInProgress); + // If an update was already in progress, we should schedule an Update + // effect even though we're bailing out, so that cWU/cDU are called. + if (typeof instance.componentDidUpdate === 'function') { + if (oldProps !== current.memoizedProps || + oldState !== current.memoizedState) { + workInProgress.effectTag |= Update; + } + } // If shouldComponentUpdate returned false, we should still update the // memoized props/state to indicate that this work can be reused. diff --git a/src/renderers/shared/fiber/ReactFiberCommitWork.js b/src/renderers/shared/fiber/ReactFiberCommitWork.js index e9676b106a..32cd4fb52f 100644 --- a/src/renderers/shared/fiber/ReactFiberCommitWork.js +++ b/src/renderers/shared/fiber/ReactFiberCommitWork.js @@ -87,16 +87,6 @@ module.exports = function( } } - // Only called during update. It's ok to throw. - function detachRefIfNeeded(current : Fiber | null, finishedWork : Fiber) { - if (current) { - const currentRef = current.ref; - if (currentRef !== null && currentRef !== finishedWork.ref) { - currentRef(null); - } - } - } - function getHostParent(fiber : Fiber) : I | C { let parent = fiber.return; while (parent !== null) { @@ -381,7 +371,6 @@ module.exports = function( function commitWork(current : Fiber | null, finishedWork : Fiber) : void { switch (finishedWork.tag) { case ClassComponent: { - detachRefIfNeeded(current, finishedWork); return; } case HostComponent: { @@ -398,7 +387,6 @@ module.exports = function( commitUpdate(instance, updatePayload, type, oldProps, newProps, finishedWork); } } - detachRefIfNeeded(current, finishedWork); return; } case HostText: { @@ -435,15 +423,11 @@ module.exports = function( const instance = finishedWork.stateNode; if (finishedWork.effectTag & Update) { if (current === null) { - if (typeof instance.componentDidMount === 'function') { - instance.componentDidMount(); - } + instance.componentDidMount(); } else { - if (typeof instance.componentDidUpdate === 'function') { - const prevProps = current.memoizedProps; - const prevState = current.memoizedState; - instance.componentDidUpdate(prevProps, prevState); - } + const prevProps = current.memoizedProps; + const prevState = current.memoizedState; + instance.componentDidUpdate(prevProps, prevState); } } if ((finishedWork.effectTag & Callback) && finishedWork.updateQueue !== null) { @@ -495,10 +479,7 @@ module.exports = function( } } - function commitRef(finishedWork : Fiber) { - if (finishedWork.tag !== ClassComponent && finishedWork.tag !== HostComponent) { - return; - } + function commitAttachRef(finishedWork : Fiber) { const ref = finishedWork.ref; if (ref !== null) { const instance = getPublicInstance(finishedWork.stateNode); @@ -506,12 +487,20 @@ module.exports = function( } } + function commitDetachRef(current : Fiber) { + const currentRef = current.ref; + if (currentRef !== null) { + currentRef(null); + } + } + return { commitPlacement, commitDeletion, commitWork, commitLifeCycles, - commitRef, + commitAttachRef, + commitDetachRef, }; }; diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index 8a04936183..aef565e1d2 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -85,6 +85,10 @@ module.exports = function( workInProgress.effectTag |= Update; } + function markRef(workInProgress : Fiber) { + workInProgress.effectTag |= Ref; + } + function appendAllYields(yields : Array, workInProgress : Fiber) { let node = workInProgress.stateNode; if (node) { @@ -231,9 +235,12 @@ module.exports = function( workInProgress.updateQueue = (updatePayload : any); // If the update payload indicates that there is a change or if there // is a new ref we mark this as an update. - if (updatePayload || current.ref !== workInProgress.ref) { + if (updatePayload) { markUpdate(workInProgress); } + if (current.ref !== workInProgress.ref) { + markRef(workInProgress); + } } else { if (!newProps) { invariant( @@ -270,7 +277,7 @@ module.exports = function( workInProgress.stateNode = instance; if (workInProgress.ref !== null) { // If there is a ref on a host node we need to schedule a callback - workInProgress.effectTag |= Ref; + markRef(workInProgress); } } return null; diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index cacaacdf83..861dd4a621 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -146,7 +146,8 @@ module.exports = function(config : HostConfig(config : HostConfig(config : HostConfig