From fd557d453d37eab29eca18f0507750ab2093669d Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 5 Mar 2019 17:41:27 +0000 Subject: [PATCH] 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. --- .../react-reconciler/src/ReactFiberHooks.js | 21 ++++++ .../src/__tests__/ReactHooks-test.internal.js | 71 +++++++++++++++++++ 2 files changed, 92 insertions(+) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 9244864b11c2f..03e4533602501 100644 --- 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(callback: T, deps: Array | void | null): T { currentHookNameInDev = 'useCallback'; mountHookTypesDev(); + checkDepsAreArrayDev(deps); return mountCallback(callback, deps); }, useContext( @@ -1265,6 +1282,7 @@ if (__DEV__) { ): void { currentHookNameInDev = 'useEffect'; mountHookTypesDev(); + checkDepsAreArrayDev(deps); return mountEffect(create, deps); }, useImperativeHandle( @@ -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(create: () => T, deps: Array | 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 index b960406504925..087c8572cd66e 100644 --- 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(); + }).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(); + }).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(); + }).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(); + ReactTestRenderer.create(); + ReactTestRenderer.create(); + }); + + 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(); + }).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(); + ReactTestRenderer.create(); + ReactTestRenderer.create(); + }); + it('assumes useEffect clean-up function is either a function or undefined', () => { const {useLayoutEffect} = React;