diff --git a/scripts/fiber/tests-passing-except-dev.txt b/scripts/fiber/tests-passing-except-dev.txt index 5590ed90a21..0b604fcf6f5 100644 --- a/scripts/fiber/tests-passing-except-dev.txt +++ b/scripts/fiber/tests-passing-except-dev.txt @@ -177,3 +177,7 @@ src/renderers/shared/shared/__tests__/ReactStatelessComponent-test.js * should warn for childContextTypes on a functional component * should warn when given a ref * should use correct name in key warning + +src/renderers/shared/shared/__tests__/refs-test.js +* attaches, detaches from fiber component with stack layer +* attaches, detaches from stack component with fiber layer diff --git a/src/renderers/shared/fiber/ReactChildFiber.js b/src/renderers/shared/fiber/ReactChildFiber.js index e07fdf7646c..0dc661d6273 100644 --- a/src/renderers/shared/fiber/ReactChildFiber.js +++ b/src/renderers/shared/fiber/ReactChildFiber.js @@ -15,6 +15,7 @@ import type { ReactCoroutine, ReactYield } from 'ReactCoroutine'; import type { ReactPortal } from 'ReactPortal'; import type { Fiber } from 'ReactFiber'; +import type { ReactInstance } from 'ReactInstanceType'; import type { PriorityLevel } from 'ReactPriorityLevel'; var REACT_ELEMENT_TYPE = require('ReactElementSymbol'); @@ -33,6 +34,7 @@ var ReactTypeOfWork = require('ReactTypeOfWork'); var emptyObject = require('emptyObject'); var getIteratorFn = require('getIteratorFn'); +var invariant = require('invariant'); const { cloneFiber, @@ -70,21 +72,32 @@ function coerceRef(current: ?Fiber, element: ReactElement) { let mixedRef = element.ref; if (mixedRef != null && typeof mixedRef !== 'function') { if (element._owner) { - const ownerFiber : ?Fiber = (element._owner : any); - if (ownerFiber && ownerFiber.tag === ClassComponent) { - const stringRef = String(mixedRef); - // Check if previous string ref matches new string ref - if (current && current.ref && current.ref._stringRef === stringRef) { - return current.ref; + const ownerFiber : ?(Fiber | ReactInstance) = (element._owner : any); + let inst; + if (ownerFiber) { + if ((ownerFiber : any).tag === ClassComponent) { + inst = (ownerFiber : any).stateNode; + } else { + // Stack + inst = (ownerFiber : any).getPublicInstance(); } - const inst = ownerFiber.stateNode; - const ref = function(value) { - const refs = inst.refs === emptyObject ? (inst.refs = {}) : inst.refs; - refs[stringRef] = value; - }; - ref._stringRef = stringRef; - return ref; } + invariant(inst, 'Missing owner for string ref %s', mixedRef); + const stringRef = String(mixedRef); + // Check if previous string ref matches new string ref + if (current && current.ref && current.ref._stringRef === stringRef) { + return current.ref; + } + const ref = function(value) { + const refs = inst.refs === emptyObject ? (inst.refs = {}) : inst.refs; + if (value === null) { + delete refs[stringRef]; + } else { + refs[stringRef] = value; + } + }; + ref._stringRef = stringRef; + return ref; } } return mixedRef; diff --git a/src/renderers/shared/shared/__tests__/refs-test.js b/src/renderers/shared/shared/__tests__/refs-test.js index d5e13b8e223..f18b5870e16 100644 --- a/src/renderers/shared/shared/__tests__/refs-test.js +++ b/src/renderers/shared/shared/__tests__/refs-test.js @@ -111,6 +111,8 @@ var expectClickLogsLengthToBe = function(instance, length) { describe('reactiverefs', () => { beforeEach(() => { jest.resetModuleRegistry(); + React = require('React'); + ReactTestUtils = require('ReactTestUtils'); }); /** @@ -152,43 +154,46 @@ describe('reactiverefs', () => { * Tests that when a ref hops around children, we can track that correctly. */ describe('ref swapping', () => { + let RefHopsAround; beforeEach(() => { jest.resetModuleRegistry(); - }); + React = require('React'); + ReactTestUtils = require('ReactTestUtils'); - class RefHopsAround extends React.Component { - state = {count: 0}; + RefHopsAround = class extends React.Component { + state = {count: 0}; - moveRef = () => { - this.setState({count: this.state.count + 1}); - }; + moveRef = () => { + this.setState({count: this.state.count + 1}); + }; - render() { - var count = this.state.count; - /** - * What we have here, is three divs with refs (div1/2/3), but a single - * moving cursor ref `hopRef` that "hops" around the three. We'll call the - * `moveRef()` function several times and make sure that the hop ref - * points to the correct divs. - */ - return ( -
-
-
-
-
- ); - } - } + render() { + var count = this.state.count; + /** + * What we have here, is three divs with refs (div1/2/3), but a single + * moving cursor ref `hopRef` that "hops" around the three. We'll call the + * `moveRef()` function several times and make sure that the hop ref + * points to the correct divs. + */ + return ( +
+
+
+
+
+ ); + } + }; + }); it('Allow refs to hop around children correctly', () => { var refHopsAround = ReactTestUtils.renderIntoDocument(); @@ -284,3 +289,101 @@ describe('ref swapping', () => { expect(a.refs[1].nodeName).toBe('DIV'); }); }); + +describe('string refs between fiber and stack', () => { + beforeEach(() => { + jest.resetModuleRegistry(); + React = require('React'); + ReactTestUtils = require('ReactTestUtils'); + }); + + it('attaches, detaches from fiber component with stack layer', () => { + spyOn(console, 'error'); + const ReactCurrentOwner = require('ReactCurrentOwner'); + const ReactDOM = require('ReactDOM'); + const ReactDOMFiber = require('ReactDOMFiber'); + const ReactInstanceMap = require('ReactInstanceMap'); + let layerMounted = false; + class A extends React.Component { + render() { + return
; + } + componentDidMount() { + // ReactLayeredComponentMixin sets ReactCurrentOwner manually + ReactCurrentOwner.current = ReactInstanceMap.get(this); + const span = ; + ReactCurrentOwner.current = null; + + ReactDOM.unstable_renderSubtreeIntoContainer( + this, + span, + this._container = document.createElement('div'), + () => { + expect(this.refs.span.nodeName).toBe('SPAN'); + layerMounted = true; + } + ); + } + componentWillUnmount() { + ReactDOM.unmountComponentAtNode(this._container); + } + } + const container = document.createElement('div'); + const a = ReactDOMFiber.render(, container); + expect(a.refs.span).toBeTruthy(); + ReactDOMFiber.unmountComponentAtNode(container); + expect(a.refs.span).toBe(undefined); + expect(layerMounted).toBe(true); + expectDev(console.error.calls.count()).toBe(1); + expectDev(console.error.calls.argsFor(0)[0]).toBe( + 'Warning: You are using React DOM Fiber which is an experimental ' + + 'renderer. It is likely to have bugs, breaking changes and is ' + + 'unsupported.' + ); + }); + + it('attaches, detaches from stack component with fiber layer', () => { + spyOn(console, 'error'); + const ReactCurrentOwner = require('ReactCurrentOwner'); + const ReactDOM = require('ReactDOM'); + const ReactDOMFiber = require('ReactDOMFiber'); + const ReactInstanceMap = require('ReactInstanceMap'); + let layerMounted = false; + class A extends React.Component { + render() { + return
; + } + componentDidMount() { + // ReactLayeredComponentMixin sets ReactCurrentOwner manually + ReactCurrentOwner.current = ReactInstanceMap.get(this); + const span = ; + ReactCurrentOwner.current = null; + + ReactDOMFiber.unstable_renderSubtreeIntoContainer( + this, + span, + this._container = document.createElement('div'), + () => { + expect(this.refs.span.nodeName).toBe('SPAN'); + layerMounted = true; + } + ); + } + componentWillUnmount() { + ReactDOMFiber.unmountComponentAtNode(this._container); + } + } + const container = document.createElement('div'); + const a = ReactDOM.render(, container); + expect(a.refs.span).toBeTruthy(); + ReactDOM.unmountComponentAtNode(container); + expect(a.refs.span).toBe(undefined); + expect(layerMounted).toBe(true); + expectDev(console.error.calls.count()).toBe(1); + expectDev(console.error.calls.argsFor(0)[0]).toBe( + 'Warning: You are using React DOM Fiber which is an experimental ' + + 'renderer. It is likely to have bugs, breaking changes and is ' + + 'unsupported.' + ); + }); +}); diff --git a/src/renderers/shared/stack/reconciler/ReactOwner.js b/src/renderers/shared/stack/reconciler/ReactOwner.js index c634dfb3844..b6c67fffc76 100644 --- a/src/renderers/shared/stack/reconciler/ReactOwner.js +++ b/src/renderers/shared/stack/reconciler/ReactOwner.js @@ -12,8 +12,12 @@ 'use strict'; +var { ClassComponent } = require('ReactTypeOfWork'); + +var emptyObject = require('emptyObject'); var invariant = require('invariant'); +import type { Fiber } from 'ReactFiber'; import type { ReactInstance } from 'ReactInstanceType'; /** @@ -72,16 +76,22 @@ var ReactOwner = { addComponentAsRefTo: function( component: ReactInstance, ref: string, - owner: ReactInstance, + owner: ReactInstance | Fiber, ): void { - invariant( - isValidOwner(owner), - 'addComponentAsRefTo(...): Only a ReactOwner can have refs. You might ' + - 'be adding a ref to a component that was not created inside a component\'s ' + - '`render` method, or you have multiple copies of React loaded ' + - '(details: https://fb.me/react-refs-must-have-owner).' - ); - owner.attachRef(ref, component); + if (owner && (owner : any).tag === ClassComponent) { + const inst = (owner : any).stateNode; + const refs = inst.refs === emptyObject ? (inst.refs = {}) : inst.refs; + refs[ref] = component.getPublicInstance(); + } else { + invariant( + isValidOwner(owner), + 'addComponentAsRefTo(...): Only a ReactOwner can have refs. You might ' + + 'be adding a ref to a component that was not created inside a component\'s ' + + '`render` method, or you have multiple copies of React loaded ' + + '(details: https://fb.me/react-refs-must-have-owner).' + ); + (owner : any).attachRef(ref, component); + } }, /** @@ -96,20 +106,27 @@ var ReactOwner = { removeComponentAsRefFrom: function( component: ReactInstance, ref: string, - owner: ReactInstance, + owner: ReactInstance | Fiber, ): void { - invariant( - isValidOwner(owner), - 'removeComponentAsRefFrom(...): Only a ReactOwner can have refs. You might ' + - 'be removing a ref to a component that was not created inside a component\'s ' + - '`render` method, or you have multiple copies of React loaded ' + - '(details: https://fb.me/react-refs-must-have-owner).' - ); - var ownerPublicInstance = owner.getPublicInstance(); - // Check that `component`'s owner is still alive and that `component` is still the current ref - // because we do not want to detach the ref if another component stole it. - if (ownerPublicInstance && ownerPublicInstance.refs[ref] === component.getPublicInstance()) { - owner.detachRef(ref); + if (owner && (owner : any).tag === ClassComponent) { + const inst = (owner : any).stateNode; + if (inst && inst.refs[ref] === component.getPublicInstance()) { + delete inst.refs[ref]; + } + } else { + invariant( + isValidOwner(owner), + 'removeComponentAsRefFrom(...): Only a ReactOwner can have refs. You might ' + + 'be removing a ref to a component that was not created inside a component\'s ' + + '`render` method, or you have multiple copies of React loaded ' + + '(details: https://fb.me/react-refs-must-have-owner).' + ); + var ownerPublicInstance = (owner : any).getPublicInstance(); + // Check that `component`'s owner is still alive and that `component` is still the current ref + // because we do not want to detach the ref if another component stole it. + if (ownerPublicInstance && ownerPublicInstance.refs[ref] === component.getPublicInstance()) { + (owner : any).detachRef(ref); + } } },