Skip to content

Commit fb636ab

Browse files
andresrobertoSimek
authored andcommitted
Enhance dev warnings for forwardRef render function (facebook#13627) (facebook#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
1 parent a34e520 commit fb636ab

File tree

2 files changed

+26
-13
lines changed

2 files changed

+26
-13
lines changed

packages/react/src/__tests__/forwardRef-test.js

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -162,22 +162,32 @@ describe('forwardRef', () => {
162162
);
163163
});
164164

165-
it('should warn if the render function provided does not use the forwarded ref parameter', () => {
166-
function arityOfZero() {
167-
return null;
168-
}
165+
it('should not warn if the render function provided does not use any parameter', () => {
166+
const arityOfZero = () => <div ref={arguments[1]} />;
167+
React.forwardRef(arityOfZero);
168+
});
169169

170-
const arityOfOne = props => null;
170+
it('should warn if the render function provided does not use the forwarded ref parameter', () => {
171+
const arityOfOne = props => <div {...props} />;
171172

172-
expect(() => React.forwardRef(arityOfZero)).toWarnDev(
173-
'forwardRef render functions accept two parameters: props and ref. ' +
173+
expect(() => React.forwardRef(arityOfOne)).toWarnDev(
174+
'forwardRef render functions accept exactly two parameters: props and ref. ' +
174175
'Did you forget to use the ref parameter?',
175176
{withoutStack: true},
176177
);
178+
});
177179

178-
expect(() => React.forwardRef(arityOfOne)).toWarnDev(
179-
'forwardRef render functions accept two parameters: props and ref. ' +
180-
'Did you forget to use the ref parameter?',
180+
it('should not warn if the render function provided use exactly two parameters', () => {
181+
const arityOfTwo = (props, ref) => <div {...props} ref={ref} />;
182+
React.forwardRef(arityOfTwo);
183+
});
184+
185+
it('should warn if the render function provided expects to use more than two parameters', () => {
186+
const arityOfThree = (props, ref, x) => <div {...props} ref={ref} x={x} />;
187+
188+
expect(() => React.forwardRef(arityOfThree)).toWarnDev(
189+
'forwardRef render functions accept exactly two parameters: props and ref. ' +
190+
'Any additional parameter will be undefined.',
181191
{withoutStack: true},
182192
);
183193
});

packages/react/src/forwardRef.js

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,12 @@ export default function forwardRef<Props, ElementType: React$ElementType>(
2121
);
2222
} else {
2323
warningWithoutStack(
24-
render.length === 2,
25-
'forwardRef render functions accept two parameters: props and ref. ' +
26-
'Did you forget to use the ref parameter?',
24+
// Do not warn for 0 arguments because it could be due to usage of the 'arguments' object
25+
render.length === 0 || render.length === 2,
26+
'forwardRef render functions accept exactly two parameters: props and ref. %s',
27+
render.length === 1
28+
? 'Did you forget to use the ref parameter?'
29+
: 'Any additional parameter will be undefined.',
2730
);
2831
}
2932

0 commit comments

Comments
 (0)