Skip to content

Commit

Permalink
Enhance dev warnings for forwardRef render function (facebook#13627) (f…
Browse files Browse the repository at this point in the history
…acebook#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
  • Loading branch information
andresroberto authored and jetoneza committed Jan 23, 2019
1 parent 425788c commit 130b839
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 13 deletions.
30 changes: 20 additions & 10 deletions packages/react/src/__tests__/forwardRef-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = () => <div ref={arguments[1]} />;
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 => <div {...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) => <div {...props} ref={ref} />;
React.forwardRef(arityOfTwo);
});

it('should warn if the render function provided expects to use more than two parameters', () => {
const arityOfThree = (props, ref, x) => <div {...props} ref={ref} x={x} />;

expect(() => React.forwardRef(arityOfThree)).toWarnDev(
'forwardRef render functions accept exactly two parameters: props and ref. ' +
'Any additional parameter will be undefined.',
{withoutStack: true},
);
});
Expand Down
9 changes: 6 additions & 3 deletions packages/react/src/forwardRef.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,12 @@ export default function forwardRef<Props, ElementType: React$ElementType>(
);
} 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.',
);
}

Expand Down

0 comments on commit 130b839

Please sign in to comment.