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.
facebook · Mar 5, 2019 · fd557d4 · fd557d4
1 parent ff596e3
commit fd557d4
Show file tree

Hide file tree

Showing 2 changed files with 92 additions and 0 deletions.
diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js
@@ -209,6 +209,22 @@ function updateHookTypesDev() {
   }
 }
 
+function checkDepsAreArrayDev(deps: mixed) {
+  if (__DEV__) {
+    if (deps !== undefined && deps !== null && !Array.isArray(deps)) {
+      // Verify deps, but only on mount to avoid extra checks.
+      // It's unlikely their type would change as usually you define them inline.
+      warning(
+        false,
+        '%s received a final argument that is not an array (instead, received `%s`). When ' +
+          'specified, the final argument must be an array.',
+        currentHookNameInDev,
+        typeof deps,
+      );
+    }
+  }
+}
+
 function warnOnHookMismatchInDev(currentHookName: HookType) {
   if (__DEV__) {
     const componentName = getComponentName(
@@ -1249,6 +1265,7 @@ if (__DEV__) {
     useCallback<T>(callback: T, deps: Array<mixed> | void | null): T {
       currentHookNameInDev = 'useCallback';
       mountHookTypesDev();
+      checkDepsAreArrayDev(deps);
       return mountCallback(callback, deps);
     },
     useContext<T>(
@@ -1265,6 +1282,7 @@ if (__DEV__) {
     ): void {
       currentHookNameInDev = 'useEffect';
       mountHookTypesDev();
+      checkDepsAreArrayDev(deps);
       return mountEffect(create, deps);
     },
     useImperativeHandle<T>(
@@ -1274,6 +1292,7 @@ if (__DEV__) {
     ): void {
       currentHookNameInDev = 'useImperativeHandle';
       mountHookTypesDev();
+      checkDepsAreArrayDev(deps);
       return mountImperativeHandle(ref, create, deps);
     },
     useLayoutEffect(
@@ -1282,11 +1301,13 @@ if (__DEV__) {
     ): void {
       currentHookNameInDev = 'useLayoutEffect';
       mountHookTypesDev();
+      checkDepsAreArrayDev(deps);
       return mountLayoutEffect(create, deps);
     },
     useMemo<T>(create: () => T, deps: Array<mixed> | void | null): T {
       currentHookNameInDev = 'useMemo';
       mountHookTypesDev();
+      checkDepsAreArrayDev(deps);
       const prevDispatcher = ReactCurrentDispatcher.current;
       ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnMountInDEV;
       try {

diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js
@@ -632,6 +632,77 @@ describe('ReactHooks', () => {
     ]);
   });
 
+  it('warns if deps is not an array', () => {
+    const {useEffect, useLayoutEffect, useMemo, useCallback} = React;
+
+    function App(props) {
+      useEffect(() => {}, props.deps);
+      useLayoutEffect(() => {}, props.deps);
+      useMemo(() => {}, props.deps);
+      useCallback(() => {}, props.deps);
+      return null;
+    }
+
+    expect(() => {
+      ReactTestRenderer.create(<App deps={'hello'} />);
+    }).toWarnDev([
+      'Warning: useEffect received a final argument that is not an array (instead, received `string`). ' +
+        'When specified, the final argument must be an array.',
+      'Warning: useLayoutEffect received a final argument that is not an array (instead, received `string`). ' +
+        'When specified, the final argument must be an array.',
+      'Warning: useMemo received a final argument that is not an array (instead, received `string`). ' +
+        'When specified, the final argument must be an array.',
+      'Warning: useCallback received a final argument that is not an array (instead, received `string`). ' +
+        'When specified, the final argument must be an array.',
+    ]);
+    expect(() => {
+      ReactTestRenderer.create(<App deps={100500} />);
+    }).toWarnDev([
+      'Warning: useEffect received a final argument that is not an array (instead, received `number`). ' +
+        'When specified, the final argument must be an array.',
+      'Warning: useLayoutEffect received a final argument that is not an array (instead, received `number`). ' +
+        'When specified, the final argument must be an array.',
+      'Warning: useMemo received a final argument that is not an array (instead, received `number`). ' +
+        'When specified, the final argument must be an array.',
+      'Warning: useCallback received a final argument that is not an array (instead, received `number`). ' +
+        'When specified, the final argument must be an array.',
+    ]);
+    expect(() => {
+      ReactTestRenderer.create(<App deps={{}} />);
+    }).toWarnDev([
+      'Warning: useEffect received a final argument that is not an array (instead, received `object`). ' +
+        'When specified, the final argument must be an array.',
+      'Warning: useLayoutEffect received a final argument that is not an array (instead, received `object`). ' +
+        'When specified, the final argument must be an array.',
+      'Warning: useMemo received a final argument that is not an array (instead, received `object`). ' +
+        'When specified, the final argument must be an array.',
+      'Warning: useCallback received a final argument that is not an array (instead, received `object`). ' +
+        'When specified, the final argument must be an array.',
+    ]);
+    ReactTestRenderer.create(<App deps={[]} />);
+    ReactTestRenderer.create(<App deps={null} />);
+    ReactTestRenderer.create(<App deps={undefined} />);
+  });
+
+  it('warns if deps is not an array for useImperativeHandle', () => {
+    const {useImperativeHandle} = React;
+
+    const App = React.forwardRef((props, ref) => {
+      useImperativeHandle(ref, () => {}, props.deps);
+      return null;
+    });
+
+    expect(() => {
+      ReactTestRenderer.create(<App deps={'hello'} />);
+    }).toWarnDev([
+      'Warning: useImperativeHandle received a final argument that is not an array (instead, received `string`). ' +
+        'When specified, the final argument must be an array.',
+    ]);
+    ReactTestRenderer.create(<App deps={[]} />);
+    ReactTestRenderer.create(<App deps={null} />);
+    ReactTestRenderer.create(<App deps={undefined} />);
+  });
+
   it('assumes useEffect clean-up function is either a function or undefined', () => {
     const {useLayoutEffect} = React;