Refs between fiber and stack (#8458)

sophiebits · web-flow · commit 705c9bcfd274 · 2016-11-30T12:17:03.000-08:00
I feel gross. Better ideas welcome.
diff --git 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
@@ -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<any>) {
   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
@@ -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 (
-        <div>
-          <div
-            className="first"
-            ref={count % 3 === 0 ? 'hopRef' : 'divOneRef'}
-          />
-          <div
-            className="second"
-            ref={count % 3 === 1 ? 'hopRef' : 'divTwoRef'}
-          />
-          <div
-            className="third"
-            ref={count % 3 === 2 ? 'hopRef' : 'divThreeRef'}
-          />
-        </div>
-      );
-    }
-  }
+      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 (
+          <div>
+            <div
+              className="first"
+              ref={count % 3 === 0 ? 'hopRef' : 'divOneRef'}
+            />
+            <div
+              className="second"
+              ref={count % 3 === 1 ? 'hopRef' : 'divTwoRef'}
+            />
+            <div
+              className="third"
+              ref={count % 3 === 2 ? 'hopRef' : 'divThreeRef'}
+            />
+          </div>
+        );
+      }
+    };
+  });
 
   it('Allow refs to hop around children correctly', () => {
     var refHopsAround = ReactTestUtils.renderIntoDocument(<RefHopsAround />);
@@ -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 <div />;
+      }
+      componentDidMount() {
+        // ReactLayeredComponentMixin sets ReactCurrentOwner manually
+        ReactCurrentOwner.current = ReactInstanceMap.get(this);
+        const span = <span ref="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(<A />, 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 <div />;
+      }
+      componentDidMount() {
+        // ReactLayeredComponentMixin sets ReactCurrentOwner manually
+        ReactCurrentOwner.current = ReactInstanceMap.get(this);
+        const span = <span ref="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(<A />, 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
@@ -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);
+      }
     }
   },
 
