Warn for callback refs on functional components (Stack + Fiber)

scripts/fiber/tests-failing.txt

@@ -66,7 +66,6 @@ src/renderers/shared/shared/__tests__/ReactMultiChildText-test.js
 
 src/renderers/shared/shared/__tests__/ReactStatelessComponent-test.js
 * should warn when stateless component returns array
-* should throw on string refs in pure functions
 * should warn when using non-React functions in JSX
 
 src/renderers/shared/shared/__tests__/ReactUpdates-test.js

scripts/fiber/tests-passing-except-dev.txt

@@ -116,4 +116,3 @@ src/renderers/shared/shared/__tests__/ReactMultiChild-test.js
 
 src/renderers/shared/shared/__tests__/ReactStatelessComponent-test.js
 * should warn for childContextTypes on a functional component
-* should warn when given a ref

scripts/fiber/tests-passing.txt

@@ -1535,6 +1535,9 @@ src/renderers/shared/shared/__tests__/ReactStatelessComponent-test.js
 * should update stateless component
 * should unmount stateless component
 * should pass context thru stateless component
+* should throw on string refs in pure functions
+* should warn when given a string ref
+* should warn when given a function ref
 * should provide a null ref
 * should use correct name in key warning
 * should support default props and prop types

src/renderers/shared/fiber/ReactChildFiber.js

@@ -78,14 +78,16 @@ function coerceRef(current: ?Fiber, element: ReactElement) {
   let mixedRef = element.ref;
   if (mixedRef != null && typeof mixedRef !== 'function') {
     if (element._owner) {
-      const ownerFiber : ?(Fiber | ReactInstance) = (element._owner : any);
+      const owner : ?(Fiber | ReactInstance) = (element._owner : any);
       let inst;
-      if (ownerFiber) {
-        if ((ownerFiber : any).tag === ClassComponent) {
-          inst = (ownerFiber : any).stateNode;
+      if (owner) {
+        if (typeof owner.tag === 'number') {
+          const ownerFiber = ((owner : any) : Fiber);
+          invariant(ownerFiber.tag === ClassComponent, 'Stateless function components cannot have refs.');
+          inst = ownerFiber.stateNode;
         } else {
           // Stack
-          inst = (ownerFiber : any).getPublicInstance();
+          inst = (owner : any).getPublicInstance();
         }
       }
       invariant(inst, 'Missing owner for string ref %s', mixedRef);

src/renderers/shared/fiber/ReactDebugCurrentFiber.js

@@ -15,12 +15,6 @@
 import type { Fiber } from 'ReactFiber';
 
 if (__DEV__) {
-  var {
-    IndeterminateComponent,
-    FunctionalComponent,
-    ClassComponent,
-    HostComponent,
-  } = require('ReactTypeOfWork');
   var getComponentName = require('getComponentName');
   var { getStackAddendumByWorkInProgressFiber } = require('ReactComponentTreeHook');
 }
@@ -31,18 +25,8 @@ function getCurrentFiberOwnerName() : string | null {
     if (fiber == null) {
       return null;
     }
-    switch (fiber.tag) {
-      case IndeterminateComponent:
-      case FunctionalComponent:
-      case ClassComponent:
-        return getComponentName(fiber);
-      case HostComponent:
-        if (fiber._debugOwner != null) {
-          return getComponentName(fiber._debugOwner);
-        }
-        return null;
-      default:
-        return null;
+    if (fiber._debugOwner != null) {
+      return getComponentName(fiber._debugOwner);
     }
   }
   return null;

src/renderers/shared/fiber/ReactFiber.js

@@ -50,7 +50,7 @@ var {
 var invariant = require('invariant');
 
 if (__DEV__) {
-  var { getCurrentFiberOwnerName } = require('ReactDebugCurrentFiber');
+  var getComponentName = require('getComponentName');
 }
 
 // A Fiber is work on a Component that needs to be done or was done. There can
@@ -300,7 +300,12 @@ exports.createHostRootFiber = function() : Fiber {
 };
 
 exports.createFiberFromElement = function(element : ReactElement, priorityLevel : PriorityLevel) : Fiber {
-  const fiber = createFiberFromElementType(element.type, element.key);
+  let owner = null;
+  if (__DEV__) {
+    owner = element._owner;
+  }
+
+  const fiber = createFiberFromElementType(element.type, element.key, owner);
   fiber.pendingProps = element.props;
   fiber.pendingWorkPriority = priorityLevel;
 
@@ -328,7 +333,7 @@ exports.createFiberFromText = function(content : string, priorityLevel : Priorit
   return fiber;
 };
 
-function createFiberFromElementType(type : mixed, key : null | string) : Fiber {
+function createFiberFromElementType(type : mixed, key : null | string, debugOwner : null | Fiber | ReactInstance) : Fiber {
   let fiber;
   if (typeof type === 'function') {
     fiber = shouldConstruct(type) ?
@@ -363,7 +368,7 @@ function createFiberFromElementType(type : mixed, key : null | string) : Fiber {
           ' You likely forgot to export your component from the file ' +
           'it\'s defined in.';
       }
-      const ownerName = getCurrentFiberOwnerName();
+      const ownerName = debugOwner ? getComponentName(debugOwner) : null;
       if (ownerName) {
         info += ' Check the render method of `' + ownerName + '`.';
       }

src/renderers/shared/fiber/ReactFiberBeginWork.js

@@ -63,6 +63,7 @@ var {
 } = require('ReactTypeOfSideEffect');
 var ReactCurrentOwner = require('ReactCurrentOwner');
 var ReactFiberClassComponent = require('ReactFiberClassComponent');
+var warning = require('warning');
 
 if (__DEV__) {
   var ReactDebugCurrentFiber = require('ReactDebugCurrentFiber');
@@ -455,6 +456,23 @@ module.exports = function<T, P, I, TI, C, CX, CI>(
     } else {
       // Proceed under the assumption that this is a functional component
       workInProgress.tag = FunctionalComponent;
+      if (__DEV__) {
+        if (workInProgress.ref != null) {
+          let info = '';
+          const ownerName = ReactDebugCurrentFiber.getCurrentFiberOwnerName();
+          if (ownerName) {
+            info += ' Check the render method of `' + ownerName + '`.';
+          }
+
+          warning(
+            false,
+            'Stateless function components cannot be given refs. ' +
+            'Attempts to access this ref will fail.%s%s',
+            info,
+            ReactDebugCurrentFiber.getCurrentFiberStackAddendum()
+          );
+        }
+      }
       reconcileChildren(current, workInProgress, value);
       return workInProgress.child;
     }

src/renderers/shared/fiber/ReactReifiedYield.js

@@ -22,7 +22,8 @@ export type ReifiedYield = { continuation: Fiber, props: Object };
 exports.createReifiedYield = function(yieldNode : ReactYield) : ReifiedYield {
   var fiber = createFiberFromElementType(
     yieldNode.continuation,
-    yieldNode.key
+    yieldNode.key,
+    null // debugOwner
   );
   return {
     continuation: fiber,
@@ -35,7 +36,8 @@ exports.createUpdatedReifiedYield = function(previousYield : ReifiedYield, yield
   if (fiber.type !== yieldNode.continuation) {
     fiber = createFiberFromElementType(
       yieldNode.continuation,
-      yieldNode.key
+      yieldNode.key,
+      null // debugOwner
     );
   }
   return {

src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js

@@ -739,9 +739,8 @@ describe('ReactIncrementalErrorHandling', () => {
       }
     }
     const InvalidType = undefined;
-    const brokenElement = <InvalidType />;
     function BrokenRender(props) {
-      return brokenElement;
+      return <InvalidType />;
     }
 
     ReactNoop.render(
@@ -776,9 +775,8 @@ describe('ReactIncrementalErrorHandling', () => {
     }
 
     const InvalidType = undefined;
-    const brokenElement = <InvalidType />;
     function BrokenRender(props) {
-      return props.fail ? brokenElement : <span />;
+      return props.fail ? <InvalidType /> : <span />;
     }
 
     ReactNoop.render(

src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js

@@ -21,6 +21,10 @@ describe('ReactIncrementalSideEffects', () => {
     ReactNoop = require('ReactNoop');
   });
 
+  function normalizeCodeLocInfo(str) {
+    return str && str.replace(/\(at .+?:\d+\)/g, '(at **)');
+  }
+
   function div(...children) {
     children = children.map(c => typeof c === 'string' ? { text: c } : c);
     return { type: 'div', children, prop: undefined };
@@ -1069,7 +1073,7 @@ describe('ReactIncrementalSideEffects', () => {
   });
 
   it('invokes ref callbacks after insertion/update/unmount', () => {
-
+    spyOn(console, 'error');
     var classInstance = null;
 
     var ops = [];
@@ -1129,6 +1133,14 @@ describe('ReactIncrementalSideEffects', () => {
       null,
     ]);
 
+    expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe(
+      'Warning: Stateless function components cannot be given refs. ' +
+      'Attempts to access this ref will fail. Check the render method ' +
+      'of `Foo`.\n' +
+      '    in FunctionalComponent (at **)\n' +
+      '    in div (at **)\n' +
+      '    in Foo (at **)'
+    );
   });
 
   // TODO: Test that mounts, updates, refs, unmounts and deletions happen in the

src/renderers/shared/shared/__tests__/ReactComponent-test.js

@@ -352,16 +352,25 @@ describe('ReactComponent', () => {
   it('includes owner name in the error about badly-typed elements', () => {
     spyOn(console, 'error');
 
+    var X = undefined;
+
+    function Indirection(props) {
+      return <div>{props.children}</div>;
+    }
+
+    function Bar() {
+      return <Indirection><X /></Indirection>;
+    }
+
     function Foo() {
-      var X = undefined;
-      return <X />;
+      return <Bar />;
     }
 
     expect(() => ReactTestUtils.renderIntoDocument(<Foo />)).toThrowError(
       'Element type is invalid: expected a string (for built-in components) ' +
       'or a class/function (for composite components) but got: undefined. ' +
       'You likely forgot to export your component from the file it\'s ' +
-      'defined in. Check the render method of `Foo`.'
+      'defined in. Check the render method of `Bar`.'
     );
 
     // One warning for each element creation

src/renderers/shared/shared/__tests__/ReactStatelessComponent-test.js

@@ -20,6 +20,9 @@ function StatelessComponent(props) {
 }
 
 describe('ReactStatelessComponent', () => {
+  function normalizeCodeLocInfo(str) {
+    return str && str.replace(/\(at .+?:\d+\)/g, '(at **)');
+  }
 
   beforeEach(() => {
     React = require('React');
@@ -146,24 +149,60 @@ describe('ReactStatelessComponent', () => {
     );
   });
 
-  it('should warn when given a ref', () => {
+  it('should warn when given a string ref', () => {
     spyOn(console, 'error');
 
+    function Indirection(props) {
+      return <div>{props.children}</div>;
+    }
+
     class Parent extends React.Component {
-      static displayName = 'Parent';
+      render() {
+        return <Indirection><StatelessComponent name="A" ref="stateless"/></Indirection>;
+      }
+    }
 
+    ReactTestUtils.renderIntoDocument(<Parent/>);
+
+    expectDev(console.error.calls.count()).toBe(1);
+    expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe(
+      'Warning: Stateless function components cannot be given refs. ' +
+      'Attempts to access this ref will fail. Check the render method ' +
+      'of `Parent`.\n' +
+      '    in StatelessComponent (at **)\n' +
+      '    in div (at **)\n' +
+      '    in Indirection (at **)\n' +
+      '    in Parent (at **)'
+    );
+  });
+
+  it('should warn when given a function ref', () => {
+    spyOn(console, 'error');
+    var ref = jasmine.createSpy().and.callFake((arg) => {
+      expect(arg).toBe(null);
+    });
+
+    function Indirection(props) {
+      return <div>{props.children}</div>;
+    }
+
+    class Parent extends React.Component {
       render() {
-        return <StatelessComponent name="A" ref="stateless"/>;
+        return <Indirection><StatelessComponent name="A" ref={ref} /></Indirection>;
       }
     }
 
     ReactTestUtils.renderIntoDocument(<Parent/>);
 
     expectDev(console.error.calls.count()).toBe(1);
-    expectDev(console.error.calls.argsFor(0)[0]).toContain(
-      'Stateless function components cannot be given refs ' +
-      '(See ref "stateless" in StatelessComponent created by Parent). ' +
-      'Attempts to access this ref will fail.'
+    expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe(
+      'Warning: Stateless function components cannot be given refs. ' +
+      'Attempts to access this ref will fail. Check the render method ' +
+      'of `Parent`.\n' +
+      '    in StatelessComponent (at **)\n' +
+      '    in div (at **)\n' +
+      '    in Indirection (at **)\n' +
+      '    in Parent (at **)'
     );
   });