diff --git a/src/renderers/shared/fiber/ReactFiberCommitWork.js b/src/renderers/shared/fiber/ReactFiberCommitWork.js index 2591361128..837f12d00c 100644 --- a/src/renderers/shared/fiber/ReactFiberCommitWork.js +++ b/src/renderers/shared/fiber/ReactFiberCommitWork.js @@ -47,18 +47,18 @@ module.exports = function(config : HostConfig) { } } - function attachRef(current : ?Fiber, finishedWork : Fiber, instance : any) { - const ref = finishedWork.ref; + function detachRefIfNeeded(current : ?Fiber, finishedWork : Fiber) { if (current) { const currentRef = current.ref; - if (currentRef && currentRef !== ref) { - // TODO: This needs to be done in a separate pass before any other refs - // gets resolved. Otherwise we might invoke them in the wrong order - // when the same ref switches between two components. + if (currentRef && currentRef !== finishedWork.ref) { currentRef(null); } } - if (ref) { + } + + function attachRef(current : ?Fiber, finishedWork : Fiber, instance : any) { + const ref = finishedWork.ref; + if (ref && (!current || current.ref !== ref)) { ref(instance); } } @@ -239,6 +239,46 @@ module.exports = function(config : HostConfig) { } function commitWork(current : ?Fiber, finishedWork : Fiber) : void { + switch (finishedWork.tag) { + case ClassComponent: { + detachRefIfNeeded(current, finishedWork); + return; + } + case HostContainer: { + // TODO: Attach children to root container. + const children = finishedWork.output; + const root : FiberRoot = finishedWork.stateNode; + const containerInfo : C = root.containerInfo; + updateContainer(containerInfo, children); + return; + } + case HostComponent: { + const instance : I = finishedWork.stateNode; + if (instance != null && current) { + // Commit the work prepared earlier. + const newProps = finishedWork.memoizedProps; + const oldProps = current.memoizedProps; + commitUpdate(instance, oldProps, newProps); + } + detachRefIfNeeded(current, finishedWork); + return; + } + case HostText: { + if (finishedWork.stateNode == null || !current) { + throw new Error('This should only be done during updates.'); + } + const textInstance : TI = finishedWork.stateNode; + const newText : string = finishedWork.memoizedProps; + const oldText : string = current.memoizedProps; + commitTextUpdate(textInstance, oldText, newText); + return; + } + default: + throw new Error('This unit of work tag should not have side-effects.'); + } + } + + function commitLifeCycles(current : ?Fiber, finishedWork : Fiber) : void { switch (finishedWork.tag) { case ClassComponent: { const instance = finishedWork.stateNode; @@ -270,33 +310,13 @@ module.exports = function(config : HostConfig) { attachRef(current, finishedWork, instance); return; } - case HostContainer: { - // TODO: Attach children to root container. - const children = finishedWork.output; - const root : FiberRoot = finishedWork.stateNode; - const containerInfo : C = root.containerInfo; - updateContainer(containerInfo, children); - return; - } case HostComponent: { const instance : I = finishedWork.stateNode; - if (instance != null && current) { - // Commit the work prepared earlier. - const newProps = finishedWork.memoizedProps; - const oldProps = current.memoizedProps; - commitUpdate(instance, oldProps, newProps); - } attachRef(current, finishedWork, instance); return; } case HostText: { - if (finishedWork.stateNode == null || !current) { - throw new Error('This should only be done during updates.'); - } - const textInstance : TI = finishedWork.stateNode; - const newText : string = finishedWork.memoizedProps; - const oldText : string = current.memoizedProps; - commitTextUpdate(textInstance, oldText, newText); + // We have no life-cycles associated with text. return; } default: @@ -308,6 +328,7 @@ module.exports = function(config : HostConfig) { commitInsertion, commitDeletion, commitWork, + commitLifeCycles, }; }; diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 1348e6037a..a1e0382ef7 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -54,7 +54,8 @@ module.exports = function(config : HostConfig) { const { beginWork } = ReactFiberBeginWork(config, getScheduler); const { completeWork } = ReactFiberCompleteWork(config); - const { commitInsertion, commitDeletion, commitWork } = ReactFiberCommitWork(config); + const { commitInsertion, commitDeletion, commitWork, commitLifeCycles } = + ReactFiberCommitWork(config); const scheduleAnimationCallback = config.scheduleAnimationCallback; const scheduleDeferredCallback = config.scheduleDeferredCallback; @@ -112,31 +113,43 @@ module.exports = function(config : HostConfig) { // Commit all the side-effects within a tree. // TODO: Error handling. - // First, we'll perform all the host insertion and deletion effects. + // First, we'll perform all the host insertions, updates, deletions and + // ref unmounts. let effectfulFiber = finishedWork.firstEffect; while (effectfulFiber) { switch (effectfulFiber.effectTag) { - // TODO: Should we commit host updates here? Otherwise update to parent - // host components are not visible in life-cycles. Such as when you read - // layout information. - case Placement: - case PlacementAndUpdate: + case Placement: { commitInsertion(effectfulFiber); break; - case Deletion: + } + case PlacementAndUpdate: { + commitInsertion(effectfulFiber); + const current = effectfulFiber.alternate; + commitWork(current, effectfulFiber); + break; + } + case Update: { + const current = effectfulFiber.alternate; + commitWork(current, effectfulFiber); + break; + } + case Deletion: { commitDeletion(effectfulFiber); break; + } } effectfulFiber = effectfulFiber.nextEffect; } - // Next, we'll perform all other effects. + // Next, we'll perform all life-cycles and ref callbacks. Life-cycles + // happens as a separate pass so that all effects in the entire tree have + // already been invoked. effectfulFiber = finishedWork.firstEffect; while (effectfulFiber) { - const current = effectfulFiber.alternate; if (effectfulFiber.effectTag === Update || effectfulFiber.effectTag === PlacementAndUpdate) { - commitWork(current, effectfulFiber); + const current = effectfulFiber.alternate; + commitLifeCycles(current, effectfulFiber); } const next = effectfulFiber.nextEffect; // Ensure that we clean these up so that we don't accidentally keep them. diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js index 382f947fc5..3347b7212f 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js @@ -680,14 +680,11 @@ describe('ReactIncrementalSideEffects', () => { ReactNoop.render(); ReactNoop.flush(); expect(ops).toEqual([ - // TODO: All detach should happen first. Currently they're interleaved. - // detach + // detach all refs that switched handlers first. null, - // reattach + null, + // reattach as a separate phase classInstance, - // detach - null, - // reattach div(), ]);