diff --git a/scripts/fiber/tests-failing.txt b/scripts/fiber/tests-failing.txt index 77059f7db5..d816054f91 100644 --- a/scripts/fiber/tests-failing.txt +++ b/scripts/fiber/tests-failing.txt @@ -108,10 +108,6 @@ src/renderers/dom/stack/client/__tests__/ReactRenderDocument-test.js * should throw on full document render w/ no markup * supports findDOMNode on full-page components -src/renderers/dom/stack/client/__tests__/findDOMNode-test.js -* findDOMNode should reject random objects -* findDOMNode should reject unmounted objects with render func - src/renderers/dom/stack/server/__tests__/ReactServerRendering-test.js * should generate simple markup * should generate simple markup for self-closing tags diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 1ceb9e0be3..069436ff51 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -908,6 +908,8 @@ src/renderers/dom/stack/client/__tests__/ReactMountDestruction-test.js src/renderers/dom/stack/client/__tests__/findDOMNode-test.js * findDOMNode should return null if passed null * findDOMNode should find dom element +* findDOMNode should reject random objects +* findDOMNode should reject unmounted objects with render func * findDOMNode should not throw an error when called within a component that is not mounted src/renderers/dom/stack/server/__tests__/ReactServerRendering-test.js diff --git a/src/renderers/dom/fiber/ReactDOMFiber.js b/src/renderers/dom/fiber/ReactDOMFiber.js index 1e1516aeec..91f25fe46a 100644 --- a/src/renderers/dom/fiber/ReactDOMFiber.js +++ b/src/renderers/dom/fiber/ReactDOMFiber.js @@ -12,6 +12,7 @@ 'use strict'; +import type { Fiber } from 'ReactFiber'; import type { HostChildren } from 'ReactFiberReconciler'; var ReactControlledComponent = require('ReactControlledComponent'); @@ -21,11 +22,7 @@ var ReactDOMFeatureFlags = require('ReactDOMFeatureFlags'); var ReactDOMFiberComponent = require('ReactDOMFiberComponent'); var ReactDOMInjection = require('ReactDOMInjection'); -ReactDOMInjection.inject(); -ReactControlledComponent.injection.injectFiberControlledHostComponent( - ReactDOMFiberComponent -); - +var findDOMNode = require('findDOMNode'); var warning = require('warning'); var { @@ -35,6 +32,14 @@ var { } = ReactDOMFiberComponent; var { precacheFiberNode } = ReactDOMComponentTree; +ReactDOMInjection.inject(); +ReactControlledComponent.injection.injectFiberControlledHostComponent( + ReactDOMFiberComponent +); +findDOMNode._injectFiber(function(fiber: Fiber) { + return DOMRenderer.findHostInstance(fiber); +}); + type DOMContainerElement = Element & { _reactRootContainer: ?Object }; type Container = Element; @@ -167,17 +172,7 @@ var ReactDOM = { } }, - findDOMNode(componentOrElement : Element | ?ReactComponent) : null | Element | Text { - if (componentOrElement == null) { - return null; - } - // Unsound duck typing. - const component = (componentOrElement : any); - if (component.nodeType === 1) { - return component; - } - return DOMRenderer.findHostInstance(component); - }, + findDOMNode: findDOMNode, unstable_batchedUpdates(fn : () => A) : A { return DOMRenderer.batchedUpdates(fn); diff --git a/src/renderers/dom/stack/client/findDOMNode.js b/src/renderers/dom/shared/findDOMNode.js similarity index 58% rename from src/renderers/dom/stack/client/findDOMNode.js rename to src/renderers/dom/shared/findDOMNode.js index 4445705b32..b4d1bf21be 100644 --- a/src/renderers/dom/stack/client/findDOMNode.js +++ b/src/renderers/dom/shared/findDOMNode.js @@ -7,59 +7,59 @@ * of patent rights can be found in the PATENTS file in the same directory. * * @providesModule findDOMNode + * @flow */ -'use strict'; - var ReactCurrentOwner = require('ReactCurrentOwner'); -var ReactDOMComponentTree = require('ReactDOMComponentTree'); var ReactInstanceMap = require('ReactInstanceMap'); -var getHostComponentFromComposite = require('getHostComponentFromComposite'); +var getComponentName = require('getComponentName'); var invariant = require('invariant'); var warning = require('warning'); -/** - * Returns the DOM node rendered by this element. - * - * See https://facebook.github.io/react/docs/react-dom.html#finddomnode - * - * @param {ReactComponent|DOMElement} componentOrElement - * @return {?DOMElement} The root node of this element. - */ -function findDOMNode(componentOrElement) { +let findFiber = function(arg) { + invariant(false, 'Missing injection for fiber findDOMNode'); +}; +let findStack = function(arg) { + invariant(false, 'Missing injection for stack findDOMNode'); +}; + +const findDOMNode = function(componentOrElement : Element | ?ReactComponent) : null | Element | Text { if (__DEV__) { var owner = ReactCurrentOwner.current; - if (owner !== null) { + if (owner !== null && '_warnedAboutRefsInRender' in owner) { warning( - owner._warnedAboutRefsInRender, + (owner: any)._warnedAboutRefsInRender, '%s is accessing findDOMNode inside its render(). ' + 'render() should be a pure function of props and state. It should ' + 'never access something that requires stale data from the previous ' + 'render, such as refs. Move this logic to componentDidMount and ' + 'componentDidUpdate instead.', - owner.getName() || 'A component' + getComponentName(owner) || 'A component' ); - owner._warnedAboutRefsInRender = true; + (owner: any)._warnedAboutRefsInRender = true; } } if (componentOrElement == null) { return null; } - if (componentOrElement.nodeType === 1) { - return componentOrElement; + if ((componentOrElement: any).nodeType === 1) { + return (componentOrElement: any); } var inst = ReactInstanceMap.get(componentOrElement); if (inst) { - inst = getHostComponentFromComposite(inst); - return inst ? ReactDOMComponentTree.getNodeFromInstance(inst) : null; + if (typeof inst.tag === 'number') { + return findFiber(inst); + } else { + return findStack(inst); + } } if (typeof componentOrElement.render === 'function') { invariant( false, - 'findDOMNode was called on an unmounted component.' + 'Unable to find node on an unmounted component.' ); } else { invariant( @@ -68,6 +68,13 @@ function findDOMNode(componentOrElement) { Object.keys(componentOrElement) ); } -} +}; + +findDOMNode._injectFiber = function(fn) { + findFiber = fn; +}; +findDOMNode._injectStack = function(fn) { + findStack = fn; +}; module.exports = findDOMNode; diff --git a/src/renderers/dom/stack/client/ReactDOMStackInjection.js b/src/renderers/dom/stack/client/ReactDOMStackInjection.js index 6bf56262ac..379628873c 100644 --- a/src/renderers/dom/stack/client/ReactDOMStackInjection.js +++ b/src/renderers/dom/stack/client/ReactDOMStackInjection.js @@ -15,6 +15,7 @@ var ReactComponentEnvironment = require('ReactComponentEnvironment'); var ReactComponentBrowserEnvironment = require('ReactComponentBrowserEnvironment'); var ReactDOMComponent = require('ReactDOMComponent'); +var ReactDOMComponentTree = require('ReactDOMComponentTree'); var ReactDOMEmptyComponent = require('ReactDOMEmptyComponent'); var ReactDOMTextComponent = require('ReactDOMTextComponent'); var ReactDefaultBatchingStrategy = require('ReactDefaultBatchingStrategy'); @@ -24,6 +25,9 @@ var ReactHostComponent = require('ReactHostComponent'); var ReactReconcileTransaction = require('ReactReconcileTransaction'); var ReactUpdates = require('ReactUpdates'); +var findDOMNode = require('findDOMNode'); +var getHostComponentFromComposite = require('getHostComponentFromComposite'); + var alreadyInjected = false; function inject() { @@ -61,6 +65,11 @@ function inject() { ); ReactComponentEnvironment.injection.injectEnvironment(ReactComponentBrowserEnvironment); + + findDOMNode._injectStack(function(inst) { + inst = getHostComponentFromComposite(inst); + return inst ? ReactDOMComponentTree.getNodeFromInstance(inst) : null; + }); } module.exports = { diff --git a/src/renderers/dom/stack/client/__tests__/findDOMNode-test.js b/src/renderers/dom/stack/client/__tests__/findDOMNode-test.js index 05b76d4fba..ea1e72e58c 100644 --- a/src/renderers/dom/stack/client/__tests__/findDOMNode-test.js +++ b/src/renderers/dom/stack/client/__tests__/findDOMNode-test.js @@ -54,7 +54,7 @@ describe('findDOMNode', () => { ReactDOM.unmountComponentAtNode(container); expect(() => ReactDOM.findDOMNode(inst)).toThrowError( - 'findDOMNode was called on an unmounted component.' + 'Unable to find node on an unmounted component.' ); }); diff --git a/src/renderers/noop/ReactNoop.js b/src/renderers/noop/ReactNoop.js index 4ff3f5b61c..f6c91e60a7 100644 --- a/src/renderers/noop/ReactNoop.js +++ b/src/renderers/noop/ReactNoop.js @@ -24,6 +24,7 @@ import type { UpdateQueue } from 'ReactFiberUpdateQueue'; import type { HostChildren } from 'ReactFiberReconciler'; var ReactFiberReconciler = require('ReactFiberReconciler'); +var ReactInstanceMap = require('ReactInstanceMap'); var { AnimationPriority, } = require('ReactPriorityLevel'); @@ -209,7 +210,8 @@ var ReactNoop = { if (component.tag === TERMINAL_TAG || component.tag === TEXT_TAG) { return component; } - return NoopRenderer.findHostInstance(component); + const inst = ReactInstanceMap.get(component); + return inst ? NoopRenderer.findHostInstance(inst) : null; }, flushAnimationPri() { diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js index 820e745149..2f780f096b 100644 --- a/src/renderers/shared/fiber/ReactFiberReconciler.js +++ b/src/renderers/shared/fiber/ReactFiberReconciler.js @@ -78,7 +78,7 @@ export type Reconciler = { getPublicRootInstance(container : OpaqueNode) : (ReactComponent | TI | I | null), // Use for findDOMNode/findHostNode. Legacy API. - findHostInstance(component : ReactComponent) : I | TI | null, + findHostInstance(component : Fiber) : I | TI | null, }; module.exports = function(config : HostConfig) : Reconciler { @@ -165,12 +165,12 @@ module.exports = function(config : HostConfig) : return containerFiber.child.stateNode; }, - findHostInstance(component : ReactComponent) : I | TI | null { - const fiber = findCurrentHostFiber(component); - if (!fiber) { + findHostInstance(fiber : Fiber) : I | TI | null { + const hostFiber = findCurrentHostFiber(fiber); + if (!hostFiber) { return null; } - return fiber.stateNode; + return hostFiber.stateNode; }, }; diff --git a/src/renderers/shared/fiber/ReactFiberTreeReflection.js b/src/renderers/shared/fiber/ReactFiberTreeReflection.js index 23485cd4ac..0f66e094bb 100644 --- a/src/renderers/shared/fiber/ReactFiberTreeReflection.js +++ b/src/renderers/shared/fiber/ReactFiberTreeReflection.js @@ -16,6 +16,8 @@ import type { Fiber } from 'ReactFiber'; var ReactInstanceMap = require('ReactInstanceMap'); +var invariant = require('invariant'); + var { HostContainer, HostComponent, @@ -27,18 +29,22 @@ var { Placement, } = require('ReactTypeOfSideEffect'); -function isFiberMounted(fiber : Fiber) : boolean { +var MOUNTING = 1; +var MOUNTED = 2; +var UNMOUNTED = 3; + +function isFiberMounted(fiber : Fiber) : number { 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) { - return false; + return MOUNTING; } while (node.return) { node = node.return; if ((node.effectTag & Placement) !== NoEffect) { - return false; + return MOUNTING; } } } else { @@ -49,10 +55,11 @@ function isFiberMounted(fiber : Fiber) : boolean { if (node.tag === HostContainer) { // TODO: Check if this was a nested HostContainer when used with // renderContainerIntoSubtree. - return true; + return MOUNTED; } - // If we didn't hit the root, that means that we're in an disconnected tree. - return false; + // If we didn't hit the root, that means that we're in an disconnected tree + // that has been unmounted. + return UNMOUNTED; } exports.isMounted = function(component : ReactComponent) : boolean { @@ -60,17 +67,18 @@ exports.isMounted = function(component : ReactComponent) : boolea if (!fiber) { return false; } - return isFiberMounted(fiber); + return isFiberMounted(fiber) === MOUNTED; }; -exports.findCurrentHostFiber = function(component : ReactComponent) : Fiber | null { - let parent = ReactInstanceMap.get(component); - if (!parent) { - return null; - } - - if (!isFiberMounted(parent)) { - // First check if this node itself is mounted. +exports.findCurrentHostFiber = function(parent : Fiber) : Fiber | null { + // First check if this node itself is mounted. + const state = isFiberMounted(parent, true); + if (state === UNMOUNTED) { + invariant( + false, + 'Unable to find node on an unmounted component.' + ); + } else if (state === MOUNTING) { return null; }