diff --git a/src/renderers/shared/fiber/ReactFiberCommitWork.js b/src/renderers/shared/fiber/ReactFiberCommitWork.js index a2411f8eb0..c12c95629d 100644 --- a/src/renderers/shared/fiber/ReactFiberCommitWork.js +++ b/src/renderers/shared/fiber/ReactFiberCommitWork.js @@ -182,18 +182,11 @@ module.exports = function(config : HostConfig) { } } - function commitDeletion(current : Fiber) : void { - // Recursively delete all host nodes from the parent. - // TODO: Error handling. - const parent = getHostParent(current); - + function unmountHostComponents(parent, current) { // We only have the top Fiber that was inserted but we need recurse down its // children to find all the terminal nodes. - // TODO: Call componentWillUnmount on all classes as needed. Recurse down - // removed HostComponents but don't call removeChild on already removed - // children. let node : Fiber = current; - outer: while (true) { + while (true) { if (node.tag === HostComponent || node.tag === HostText) { commitNestedUnmounts(node); // After all the children have unmounted, it is now safe to remove the @@ -210,16 +203,25 @@ module.exports = function(config : HostConfig) { } } if (node === current) { - break outer; + return; } while (!node.sibling) { if (!node.return || node.return === current) { - break outer; + return; } node = node.return; } node = node.sibling; } + } + + function commitDeletion(current : Fiber) : void { + // Recursively delete all host nodes from the parent. + // TODO: Error handling. + const parent = getHostParent(current); + + unmountHostComponents(parent, current); + // Cut off the return pointers to disconnect it from the tree. Ideally, we // should clear the child pointer of the parent alternate to let this // get GC:ed but we don't know which for sure which parent is the current diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index fb2f043e24..5677ffafcb 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -119,12 +119,18 @@ module.exports = function(config : HostConfig) { switch (effectfulFiber.effectTag) { case Placement: { commitInsertion(effectfulFiber); + // Clear the effect tag so that we know that this is inserted, before + // any life-cycles like componentDidMount gets called. + effectfulFiber.effectTag = NoWork; break; } case PlacementAndUpdate: { commitInsertion(effectfulFiber); const current = effectfulFiber.alternate; commitWork(current, effectfulFiber); + // Clear the effect tag so that we know that this is inserted, before + // any life-cycles like componentDidMount gets called. + effectfulFiber.effectTag = Update; break; } case Update: { @@ -158,7 +164,6 @@ module.exports = function(config : HostConfig) { effectfulFiber.nextEffect = null; // Ensure that we reset the effectTag here so that we can rely on effect // tags to reason about the current life-cycle. - effectfulFiber.effectTag = NoWork; effectfulFiber = next; } diff --git a/src/renderers/shared/fiber/ReactFiberTreeReflection.js b/src/renderers/shared/fiber/ReactFiberTreeReflection.js index 45e2bebb5b..836b6609d3 100644 --- a/src/renderers/shared/fiber/ReactFiberTreeReflection.js +++ b/src/renderers/shared/fiber/ReactFiberTreeReflection.js @@ -23,15 +23,41 @@ var { HostText, } = require('ReactTypeOfWork'); +var { + NoEffect, + Placement, +} = require('ReactTypeOfSideEffect'); + exports.isMounted = function(component : ReactComponent) : boolean { var parent : ?Fiber = ReactInstanceMap.get(component); if (!parent) { return false; } - // TODO: This doesn't deal with the case where it has completed but not yet - // committed. It also doesn't deal with unmounts since they currently don't - // clean up the item in the ReactInstanceMap. - return true; + let node = parent; + if (!parent.alternate) { + // If there is no alternate, this might be a new tree that isn't inserted + // yet. If it is, then it will have a pending insertion effect on it. + if ((node.effectTag & Placement) !== NoEffect) { + return false; + } + while (node.return) { + node = node.return; + if ((node.effectTag & Placement) !== NoEffect) { + return false; + } + } + } else { + while (node.return) { + node = node.return; + } + } + if (node.tag === HostContainer) { + // TODO: Check if this was a nested HostContainer when used with + // renderContainerIntoSubtree. + return true; + } + // If we didn't hit the root, that means that we're in an disconnected tree. + return false; }; exports.findCurrentHostFiber = function(component : ReactComponent) : Fiber | null { diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalReflection-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalReflection-test.js new file mode 100644 index 0000000000..acec3df6f4 --- /dev/null +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalReflection-test.js @@ -0,0 +1,127 @@ +/** + * 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. + * + * @emails react-core + */ + +'use strict'; + +var React; +var ReactNoop; + +describe('ReactIncrementalReflection', () => { + beforeEach(() => { + React = require('React'); + ReactNoop = require('ReactNoop'); + }); + + it('handles isMounted even when the initial render is deferred', () => { + + let ops = []; + + const instances = []; + + const Component = React.createClass({ + componentWillMount() { + instances.push(this); + ops.push('componentWillMount', this.isMounted()); + }, + componentDidMount() { + ops.push('componentDidMount', this.isMounted()); + }, + render() { + return ; + }, + }); + + function Foo() { + return ; + } + + ReactNoop.render(); + + // Render part way through but don't yet commit the updates. + ReactNoop.flushDeferredPri(20); + + expect(ops).toEqual([ + 'componentWillMount', false, + ]); + + expect(instances[0].isMounted()).toBe(false); + + ops = []; + + // Render the rest and commit the updates. + ReactNoop.flush(); + + expect(ops).toEqual([ + 'componentDidMount', true, + ]); + + expect(instances[0].isMounted()).toBe(true); + + }); + + it('handles isMounted when an unmount is deferred', () => { + + let ops = []; + + const instances = []; + + const Component = React.createClass({ + componentWillMount() { + instances.push(this); + }, + componentWillUnmount() { + ops.push('componentWillUnmount', this.isMounted()); + }, + render() { + ops.push('Component'); + return ; + }, + }); + + function Other() { + ops.push('Other'); + return ; + } + + function Foo(props) { + return props.mount ? : ; + } + + ReactNoop.render(); + ReactNoop.flush(); + + expect(ops).toEqual(['Component']); + ops = []; + + expect(instances[0].isMounted()).toBe(true); + + ReactNoop.render(); + // Render part way through but don't yet commit the updates so it is not + // fully unmounted yet. + ReactNoop.flushDeferredPri(20); + + expect(ops).toEqual(['Other']); + ops = []; + + expect(instances[0].isMounted()).toBe(true); + + // Finish flushing the unmount. + ReactNoop.flush(); + + expect(ops).toEqual([ + 'componentWillUnmount', true, + ]); + + expect(instances[0].isMounted()).toBe(false); + + }); + +});