From 7da8d37ac19b0501991bf54d01490a3d7f3e35d6 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Sat, 7 Jul 2018 07:44:13 -0700 Subject: [PATCH 1/2] Add DEV warning if forwardRef function doesn't use the ref param --- .../react/src/__tests__/forwardRef-test.js | 22 +++++++++++++++++-- packages/react/src/forwardRef.js | 18 ++++++++++----- 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/packages/react/src/__tests__/forwardRef-test.js b/packages/react/src/__tests__/forwardRef-test.js index aa1cd160d5d1..b14408bd289e 100644 --- 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 index 419af668a304..0f4cf3ef1fc9 100644 --- a/packages/react/src/forwardRef.js +++ b/packages/react/src/forwardRef.js @@ -13,11 +13,19 @@ export default function forwardRef( render: (props: Props, ref: React$ElementRef) => 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( From af43dfca048fd2dbec3b62625387fc19a51b5b64 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Sat, 7 Jul 2018 07:58:11 -0700 Subject: [PATCH 2/2] Fixed a forwardRef arity warning in another test --- .../src/__tests__/ReactTestRendererTraversal-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-test-renderer/src/__tests__/ReactTestRendererTraversal-test.js b/packages/react-test-renderer/src/__tests__/ReactTestRendererTraversal-test.js index 3ea71880eaf7..a3c88f2bf941 100644 --- 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 =>
); + const FR = React.forwardRef((props, ref) =>
); expect( ReactTestRenderer.create(