Add DEV warning if forwardRef function doesn't use the ref param (#13168

) * Add DEV warning if forwardRef function doesn't use the ref param * Fixed a forwardRef arity warning in another test
facebook · Jul 7, 2018 · 095dd50 · 095dd50
1 parent 5662595
commit 095dd50
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 8 deletions.
diff --git a/packages/react-test-renderer/src/__tests__/ReactTestRendererTraversal-test.js b/packages/react-test-renderer/src/__tests__/ReactTestRendererTraversal-test.js
@@ -201,7 +201,7 @@ describe('ReactTestRendererTraversal', () => {
   });
 
   it('can have special nodes as roots', () => {
-    const FR = React.forwardRef(props => <section {...props} />);
+    const FR = React.forwardRef((props, ref) => <section {...props} />);
     expect(
       ReactTestRenderer.create(
         <FR>

diff --git a/packages/react/src/__tests__/forwardRef-test.js b/packages/react/src/__tests__/forwardRef-test.js
@@ -136,12 +136,12 @@ describe('forwardRef', () => {
   });
 
   it('should warn if the render function provided has propTypes or defaultProps attributes', () => {
-    function renderWithPropTypes() {
+    function renderWithPropTypes(props, ref) {
       return null;
     }
     renderWithPropTypes.propTypes = {};
 
-    function renderWithDefaultProps() {
+    function renderWithDefaultProps(props, ref) {
       return null;
     }
     renderWithDefaultProps.defaultProps = {};
@@ -155,4 +155,22 @@ describe('forwardRef', () => {
         'Did you accidentally pass a React component?',
     );
   });
+
+  it('should warn if the render function provided does not use the forwarded ref parameter', () => {
+    function arityOfZero() {
+      return null;
+    }
+
+    const arityOfOne = props => null;
+
+    expect(() => React.forwardRef(arityOfZero)).toWarnDev(
+      'forwardRef render functions accept two parameters: props and ref. ' +
+        'Did you forget to use the ref parameter?',
+    );
+
+    expect(() => React.forwardRef(arityOfOne)).toWarnDev(
+      'forwardRef render functions accept two parameters: props and ref. ' +
+        'Did you forget to use the ref parameter?',
+    );
+  });
 });
diff --git a/packages/react/src/forwardRef.js b/packages/react/src/forwardRef.js
@@ -13,11 +13,19 @@ export default function forwardRef<Props, ElementType: React$ElementType>(
   render: (props: Props, ref: React$ElementRef<ElementType>) => React$Node,
 ) {
   if (__DEV__) {
-    warning(
-      typeof render === 'function',
-      'forwardRef requires a render function but was given %s.',
-      render === null ? 'null' : typeof render,
-    );
+    if (typeof render !== 'function') {
+      warning(
+        false,
+        'forwardRef requires a render function but was given %s.',
+        render === null ? 'null' : typeof render,
+      );
+    } else {
+      warning(
+        render.length === 2,
+        'forwardRef render functions accept two parameters: props and ref. ' +
+          'Did you forget to use the ref parameter?',
+      );
+    }
 
     if (render != null) {
       warning(