@samitouri / QOS-React-2 / commits / ecbf7af40b

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

Andres Rojas committed Sep 13, 2018 at 17:12 UTC ecbf7af40b72b90c007416132eb5e941542d4cca
2 files changed +26 -13
packages/react/src/__tests__/forwardRef-test.js
+20 -10
@@ -162,22 +162,32 @@ describe('forwardRef', () => {
162 );
163 });
164
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 + });
169
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} />;
172
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. ' +
175 'Did you forget to use the ref parameter?',
176 {withoutStack: true},
177 );
178 + });
179
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.',
191 {withoutStack: true},
192 );
193 });
packages/react/src/forwardRef.js
+6 -3
@@ -21,9 +21,12 @@ export default function forwardRef<Props, ElementType: React$ElementType>(
21 );
22 } else {
23 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.',
30 );
31 }
32