From d5752aca0dc40f4ab3ec480940d045dc1fddf889 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 24 Oct 2016 16:02:22 -0700 Subject: [PATCH] findDOMNode when a component is not yet mounted or unmounted First we need to check if a component subtree is mounted at all. If it is, we need to search down the fiber for the first host node. However, we might be searching the "work in progress" instead of current. One realization is that it doesn't matter if we search work in progress or current if they're the same. They will generally be the same unless there is an insertion pending or something in the alternate tree was already deleted. So if we find one of those cases, we switch to look in the alternate tree instead. --- src/renderers/noop/ReactNoop.js | 12 ++ .../shared/fiber/ReactFiberTreeReflection.js | 45 ++++-- .../ReactIncrementalReflection-test.js | 148 ++++++++++++++++++ 3 files changed, 195 insertions(+), 10 deletions(-) diff --git a/src/renderers/noop/ReactNoop.js b/src/renderers/noop/ReactNoop.js index 0ac23ac0c3..9b9c13673f 100644 --- a/src/renderers/noop/ReactNoop.js +++ b/src/renderers/noop/ReactNoop.js @@ -154,6 +154,18 @@ var ReactNoop = { } }, + findInstance(componentOrElement : Element | ?ReactComponent) : null | Instance | TextInstance { + if (componentOrElement == null) { + return null; + } + // Unsound duck typing. + const component = (componentOrElement : any); + if (component.tag === TERMINAL_TAG || component.tag === TEXT_TAG) { + return component; + } + return NoopRenderer.findHostInstance(component); + }, + flushAnimationPri() { var cb = scheduledAnimationCallback; if (cb === null) { diff --git a/src/renderers/shared/fiber/ReactFiberTreeReflection.js b/src/renderers/shared/fiber/ReactFiberTreeReflection.js index 836b6609d3..d806992e74 100644 --- a/src/renderers/shared/fiber/ReactFiberTreeReflection.js +++ b/src/renderers/shared/fiber/ReactFiberTreeReflection.js @@ -17,7 +17,6 @@ import type { Fiber } from 'ReactFiber'; var ReactInstanceMap = require('ReactInstanceMap'); var { - ClassComponent, HostContainer, HostComponent, HostText, @@ -28,13 +27,9 @@ var { Placement, } = require('ReactTypeOfSideEffect'); -exports.isMounted = function(component : ReactComponent) : boolean { - var parent : ?Fiber = ReactInstanceMap.get(component); - if (!parent) { - return false; - } - let node = parent; - if (!parent.alternate) { +function isFiberMounted(fiber : Fiber) : boolean { + let node = fiber; + if (!fiber.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) { @@ -58,17 +53,47 @@ exports.isMounted = function(component : ReactComponent) : boolea } // If we didn't hit the root, that means that we're in an disconnected tree. return false; +} + +exports.isMounted = function(component : ReactComponent) : boolean { + var fiber : ?Fiber = ReactInstanceMap.get(component); + if (!fiber) { + return false; + } + return isFiberMounted(fiber); }; exports.findCurrentHostFiber = function(component : ReactComponent) : Fiber | null { - var parent : ?Fiber = ReactInstanceMap.get(component); + let parent = ReactInstanceMap.get(component); if (!parent) { return null; } - // TODO: This search is incomplete because this could be one of two possible fibers. + if (!isFiberMounted(parent)) { + // First check if this node itself is mounted. + return null; + } + + let didTryOtherTree = false; + + // Next we'll drill down this component to find the first HostComponent/Text. let node : Fiber = parent; while (true) { + if ((node.effectTag & Placement) !== NoEffect || !node.return) { + // If any node along the way was deleted, or is an insertion, that means + // that we're actually in a work in progress to update this component with + // a different component. We need to look in the "current" fiber instead. + if (!parent.alternate) { + return null; + } + if (didTryOtherTree) { + // Safety, to avoid an infinite loop if something goes wrong. + throw new Error('This should never hit this infinite loop.'); + } + didTryOtherTree = true; + node = parent = parent.alternate; + continue; + } if (node.tag === HostComponent || node.tag === HostText) { return node; } else if (node.child) { diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalReflection-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalReflection-test.js index acec3df6f4..6eb02db48c 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncrementalReflection-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalReflection-test.js @@ -124,4 +124,152 @@ describe('ReactIncrementalReflection', () => { }); + it('finds no node before insertion and correct node before deletion', () => { + + let ops = []; + + let classInstance = null; + + class Component extends React.Component { + componentWillMount() { + classInstance = this; + ops.push('componentWillMount', ReactNoop.findInstance(this)); + } + componentDidMount() { + ops.push('componentDidMount', ReactNoop.findInstance(this)); + } + componentWillUpdate() { + ops.push('componentWillUpdate', ReactNoop.findInstance(this)); + } + componentDidUpdate() { + ops.push('componentDidUpdate', ReactNoop.findInstance(this)); + } + render() { + ops.push('render'); + return this.props.step < 2 ? + this.span = ref} /> : + this.props.step === 2 ? +
this.div = ref} /> : + null; + } + } + + function Sibling() { + // Sibling is used to assert that we've rendered past the first component. + ops.push('render sibling'); + return ; + } + + function Foo(props) { + return [ + , + , + ]; + } + + ReactNoop.render(); + // Flush past Component but don't complete rendering everything yet. + ReactNoop.flushDeferredPri(30); + + expect(ops).toEqual([ + 'componentWillMount', null, + 'render', + 'render sibling', + ]); + + ops = []; + + expect(classInstance).toBeDefined(); + // The instance has been complete but is still not committed so it should + // not find any host nodes in it. + expect(ReactNoop.findInstance(classInstance)).toBe(null); + + ReactNoop.flush(); + + const hostSpan = classInstance.span; + expect(hostSpan).toBeDefined(); + + expect(ReactNoop.findInstance(classInstance)).toBe(hostSpan); + + expect(ops).toEqual([ + 'componentDidMount', hostSpan, + ]); + + ops = []; + + // Flush next step which will cause an update but not yet render a new host + // node. + ReactNoop.render(); + ReactNoop.flush(); + + expect(ops).toEqual([ + 'componentWillUpdate', hostSpan, + 'render', + 'render sibling', + 'componentDidUpdate', hostSpan, + ]); + + expect(ReactNoop.findInstance(classInstance)).toBe(hostSpan); + + ops = []; + + // The next step will render a new host node but won't get committed yet. + // We expect this to mutate the original Fiber. + ReactNoop.render(); + ReactNoop.flushDeferredPri(30); + + expect(ops).toEqual([ + 'componentWillUpdate', hostSpan, + 'render', + 'render sibling', + ]); + + ops = []; + + // This should still be the host span. + expect(ReactNoop.findInstance(classInstance)).toBe(hostSpan); + + // When we finally flush the tree it will get committed. + ReactNoop.flush(); + + const hostDiv = classInstance.div; + + expect(hostDiv).toBeDefined(); + expect(hostSpan).not.toBe(hostDiv); + + expect(ops).toEqual([ + 'componentDidUpdate', hostDiv, + ]); + + ops = []; + + // We should now find the new host node. + expect(ReactNoop.findInstance(classInstance)).toBe(hostDiv); + + // Finally we will render to null but not yet commit it. + ReactNoop.render(); + ReactNoop.flushDeferredPri(25); + + expect(ops).toEqual([ + 'componentWillUpdate', hostDiv, + 'render', + 'render sibling', + ]); + + ops = []; + + // This should still be the host div since the deletion is not committed. + expect(ReactNoop.findInstance(classInstance)).toBe(hostDiv); + + ReactNoop.flush(); + + expect(ops).toEqual([ + 'componentDidUpdate', null, + ]); + + // This should still be the host div since the deletion is not committed. + expect(ReactNoop.findInstance(classInstance)).toBe(null); + }); + + });