From 130b839d2a6152b74fd425c7c1f0a621e06b3c67 Mon Sep 17 00:00:00 2001 From: Andres Rojas Date: Thu, 13 Sep 2018 17:12:34 +0200 Subject: [PATCH] Enhance dev warnings for forwardRef render function (#13627) (#13636) * Enhance dev warnings for forwardRef render function - For 0 parameters: Do not warn because it may be due to usage of the arguments object. - For 1 parameter: Warn about missing the 'ref' parameter. - For 2 parameters: This is the ideal. Do not warn. - For more than 2 parameters: Warn about undefined parameters. * Make test cases for forwardRef warnings more realistic * Add period to warning sentence --- .../react/src/__tests__/forwardRef-test.js | 30 ++++++++++++------- packages/react/src/forwardRef.js | 9 ++++-- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/packages/react/src/__tests__/forwardRef-test.js b/packages/react/src/__tests__/forwardRef-test.js index 82e096e57341d..08977347b6267 100644 --- a/packages/react/src/__tests__/forwardRef-test.js +++ b/packages/react/src/__tests__/forwardRef-test.js @@ -162,22 +162,32 @@ describe('forwardRef', () => { ); }); - it('should warn if the render function provided does not use the forwarded ref parameter', () => { - function arityOfZero() { - return null; - } + it('should not warn if the render function provided does not use any parameter', () => { + const arityOfZero = () =>
; + React.forwardRef(arityOfZero); + }); - const arityOfOne = props => null; + it('should warn if the render function provided does not use the forwarded ref parameter', () => { + const arityOfOne = props =>
; - expect(() => React.forwardRef(arityOfZero)).toWarnDev( - 'forwardRef render functions accept two parameters: props and ref. ' + + expect(() => React.forwardRef(arityOfOne)).toWarnDev( + 'forwardRef render functions accept exactly two parameters: props and ref. ' + 'Did you forget to use the ref parameter?', {withoutStack: true}, ); + }); - expect(() => React.forwardRef(arityOfOne)).toWarnDev( - 'forwardRef render functions accept two parameters: props and ref. ' + - 'Did you forget to use the ref parameter?', + it('should not warn if the render function provided use exactly two parameters', () => { + const arityOfTwo = (props, ref) =>
; + React.forwardRef(arityOfTwo); + }); + + it('should warn if the render function provided expects to use more than two parameters', () => { + const arityOfThree = (props, ref, x) =>
; + + expect(() => React.forwardRef(arityOfThree)).toWarnDev( + 'forwardRef render functions accept exactly two parameters: props and ref. ' + + 'Any additional parameter will be undefined.', {withoutStack: true}, ); }); diff --git a/packages/react/src/forwardRef.js b/packages/react/src/forwardRef.js index 3bfa9f62d4b7f..1018375e8c92a 100644 --- a/packages/react/src/forwardRef.js +++ b/packages/react/src/forwardRef.js @@ -21,9 +21,12 @@ export default function forwardRef( ); } else { warningWithoutStack( - render.length === 2, - 'forwardRef render functions accept two parameters: props and ref. ' + - 'Did you forget to use the ref parameter?', + // Do not warn for 0 arguments because it could be due to usage of the 'arguments' object + render.length === 0 || render.length === 2, + 'forwardRef render functions accept exactly two parameters: props and ref. %s', + render.length === 1 + ? 'Did you forget to use the ref parameter?' + : 'Any additional parameter will be undefined.', ); }