Warn on mount when deps are not an array (#15018)
* Warn on mount when deps are not an array * Check other Hooks * I can't figure out how to fix error/warning nesting lint But it doesn't really matter much because we test other cases in the other test.
Dan Abramov committed
Mar 5, 2019 at 17:41 UTC
fd557d453d37eab29eca18f0507750ab2093669d
2 files changed
+92
packages/react-reconciler/src/ReactFiberHooks.js
+21
@@ -209,6 +209,22 @@ function updateHookTypesDev() {
209
}
210
}
211
212
+function checkDepsAreArrayDev(deps: mixed) {
213
+ if (__DEV__) {
214
+ if (deps !== undefined && deps !== null && !Array.isArray(deps)) {
215
+ // Verify deps, but only on mount to avoid extra checks.
216
+ // It's unlikely their type would change as usually you define them inline.
217
+ warning(
218
+ false,
219
+ '%s received a final argument that is not an array (instead, received `%s`). When ' +
220
+ 'specified, the final argument must be an array.',
221
+ currentHookNameInDev,
222
+ typeof deps,
223
+ );
224
+ }
225
+ }
226
+}
227
+
228
function warnOnHookMismatchInDev(currentHookName: HookType) {
229
if (__DEV__) {
230
const componentName = getComponentName(
@@ -1249,6 +1265,7 @@ if (__DEV__) {
1265
useCallback<T>(callback: T, deps: Array<mixed> | void | null): T {
1266
currentHookNameInDev = 'useCallback';
1267
mountHookTypesDev();
1268
+ checkDepsAreArrayDev(deps);
1269
return mountCallback(callback, deps);
1270
},
1271
useContext<T>(
@@ -1265,6 +1282,7 @@ if (__DEV__) {
1282
): void {
1283
currentHookNameInDev = 'useEffect';
1284
mountHookTypesDev();
1285
+ checkDepsAreArrayDev(deps);
1286
return mountEffect(create, deps);
1287
},
1288
useImperativeHandle<T>(
@@ -1274,6 +1292,7 @@ if (__DEV__) {
1292
): void {
1293
currentHookNameInDev = 'useImperativeHandle';
1294
mountHookTypesDev();
1295
+ checkDepsAreArrayDev(deps);
1296
return mountImperativeHandle(ref, create, deps);
1297
},
1298
useLayoutEffect(
@@ -1282,11 +1301,13 @@ if (__DEV__) {
1301
): void {
1302
currentHookNameInDev = 'useLayoutEffect';
1303
mountHookTypesDev();
1304
+ checkDepsAreArrayDev(deps);
1305
return mountLayoutEffect(create, deps);
1306
},
1307
useMemo<T>(create: () => T, deps: Array<mixed> | void | null): T {
1308
currentHookNameInDev = 'useMemo';
1309
mountHookTypesDev();
1310
+ checkDepsAreArrayDev(deps);
1311
const prevDispatcher = ReactCurrentDispatcher.current;
1312
ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnMountInDEV;
1313
try {
packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js
+71
@@ -632,6 +632,77 @@ describe('ReactHooks', () => {
632
]);
633
});
634
635
+ it('warns if deps is not an array', () => {
636
+ const {useEffect, useLayoutEffect, useMemo, useCallback} = React;
637
+
638
+ function App(props) {
639
+ useEffect(() => {}, props.deps);
640
+ useLayoutEffect(() => {}, props.deps);
641
+ useMemo(() => {}, props.deps);
642
+ useCallback(() => {}, props.deps);
643
+ return null;
644
+ }
645
+
646
+ expect(() => {
647
+ ReactTestRenderer.create(<App deps={'hello'} />);
648
+ }).toWarnDev([
649
+ 'Warning: useEffect received a final argument that is not an array (instead, received `string`). ' +
650
+ 'When specified, the final argument must be an array.',
651
+ 'Warning: useLayoutEffect received a final argument that is not an array (instead, received `string`). ' +
652
+ 'When specified, the final argument must be an array.',
653
+ 'Warning: useMemo received a final argument that is not an array (instead, received `string`). ' +
654
+ 'When specified, the final argument must be an array.',
655
+ 'Warning: useCallback received a final argument that is not an array (instead, received `string`). ' +
656
+ 'When specified, the final argument must be an array.',
657
+ ]);
658
+ expect(() => {
659
+ ReactTestRenderer.create(<App deps={100500} />);
660
+ }).toWarnDev([
661
+ 'Warning: useEffect received a final argument that is not an array (instead, received `number`). ' +
662
+ 'When specified, the final argument must be an array.',
663
+ 'Warning: useLayoutEffect received a final argument that is not an array (instead, received `number`). ' +
664
+ 'When specified, the final argument must be an array.',
665
+ 'Warning: useMemo received a final argument that is not an array (instead, received `number`). ' +
666
+ 'When specified, the final argument must be an array.',
667
+ 'Warning: useCallback received a final argument that is not an array (instead, received `number`). ' +
668
+ 'When specified, the final argument must be an array.',
669
+ ]);
670
+ expect(() => {
671
+ ReactTestRenderer.create(<App deps={{}} />);
672
+ }).toWarnDev([
673
+ 'Warning: useEffect received a final argument that is not an array (instead, received `object`). ' +
674
+ 'When specified, the final argument must be an array.',
675
+ 'Warning: useLayoutEffect received a final argument that is not an array (instead, received `object`). ' +
676
+ 'When specified, the final argument must be an array.',
677
+ 'Warning: useMemo received a final argument that is not an array (instead, received `object`). ' +
678
+ 'When specified, the final argument must be an array.',
679
+ 'Warning: useCallback received a final argument that is not an array (instead, received `object`). ' +
680
+ 'When specified, the final argument must be an array.',
681
+ ]);
682
+ ReactTestRenderer.create(<App deps={[]} />);
683
+ ReactTestRenderer.create(<App deps={null} />);
684
+ ReactTestRenderer.create(<App deps={undefined} />);
685
+ });
686
+
687
+ it('warns if deps is not an array for useImperativeHandle', () => {
688
+ const {useImperativeHandle} = React;
689
+
690
+ const App = React.forwardRef((props, ref) => {
691
+ useImperativeHandle(ref, () => {}, props.deps);
692
+ return null;
693
+ });
694
+
695
+ expect(() => {
696
+ ReactTestRenderer.create(<App deps={'hello'} />);
697
+ }).toWarnDev([
698
+ 'Warning: useImperativeHandle received a final argument that is not an array (instead, received `string`). ' +
699
+ 'When specified, the final argument must be an array.',
700
+ ]);
701
+ ReactTestRenderer.create(<App deps={[]} />);
702
+ ReactTestRenderer.create(<App deps={null} />);
703
+ ReactTestRenderer.create(<App deps={undefined} />);
704
+ });
705
+
706
it('assumes useEffect clean-up function is either a function or undefined', () => {
707
const {useLayoutEffect} = React;
708