From 64841ee48291cd9ccfb111315c1be033291890a7 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 21 Jan 2019 16:52:34 +0000 Subject: [PATCH] Warn about incorrect use of useImperativeHandle() --- .../react-reconciler/src/ReactFiberHooks.js | 14 +++++++ .../src/ReactFiberScheduler.js | 13 +++++++ .../src/__tests__/ReactHooks-test.internal.js | 39 +++++++++++++++++++ 3 files changed, 66 insertions(+) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 1e050b1febc7..cbd2a255bbde 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -674,6 +674,12 @@ export function useImperativeHandle( ): void { if (__DEV__) { currentHookNameInDev = 'useImperativeHandle'; + warning( + typeof create === 'function', + 'Expected useImperativeHandle() second argument to be a function ' + + 'that creates a handle. Instead received: %s.', + create !== null ? typeof create : 'null', + ); } // TODO: If deps are provided, should we skip comparing the ref itself? const nextDeps = @@ -690,6 +696,14 @@ export function useImperativeHandle( return () => refCallback(null); } else if (ref !== null && ref !== undefined) { const refObject = ref; + if (__DEV__) { + warning( + refObject.hasOwnProperty('current'), + 'Expected useImperativeHandle() first argument to either be a ' + + 'ref callback or React.createRef() object. Instead received: %s.', + 'an object with keys {' + Object.keys(refObject).join(', ') + '}', + ); + } const inst = create(); refObject.current = inst; return () => { diff --git a/packages/react-reconciler/src/ReactFiberScheduler.js b/packages/react-reconciler/src/ReactFiberScheduler.js index 4dfc4e8bebd4..bcc41fe3670a 100644 --- a/packages/react-reconciler/src/ReactFiberScheduler.js +++ b/packages/react-reconciler/src/ReactFiberScheduler.js @@ -489,6 +489,9 @@ function commitAllLifeCycles( } } while (nextEffect !== null) { + if (__DEV__) { + setCurrentFiber(nextEffect); + } const effectTag = nextEffect.effectTag; if (effectTag & (Update | Callback)) { @@ -513,6 +516,9 @@ function commitAllLifeCycles( nextEffect = nextEffect.nextEffect; } + if (__DEV__) { + resetCurrentFiber(); + } } function commitPassiveEffects(root: FiberRoot, firstEffect: Fiber): void { @@ -526,6 +532,10 @@ function commitPassiveEffects(root: FiberRoot, firstEffect: Fiber): void { let effect = firstEffect; do { + if (__DEV__) { + setCurrentFiber(effect); + } + if (effect.effectTag & Passive) { let didError = false; let error; @@ -549,6 +559,9 @@ function commitPassiveEffects(root: FiberRoot, firstEffect: Fiber): void { } effect = effect.nextEffect; } while (effect !== null); + if (__DEV__) { + resetCurrentFiber(); + } isRendering = previousIsRendering; diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js index d13755ebddb9..0008a27b1485 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js @@ -569,6 +569,45 @@ describe('ReactHooks', () => { ]); }); + it('warns for bad useImperativeHandle first arg', () => { + const {useImperativeHandle} = React; + function App() { + useImperativeHandle({ + focus() {}, + }); + return null; + } + + expect(() => { + expect(() => { + ReactTestRenderer.create(); + }).toThrow('create is not a function'); + }).toWarnDev([ + 'Expected useImperativeHandle() first argument to either be a ' + + 'ref callback or React.createRef() object. ' + + 'Instead received: an object with keys {focus}.', + 'Expected useImperativeHandle() second argument to be a function ' + + 'that creates a handle. Instead received: undefined.', + ]); + }); + + it('warns for bad useImperativeHandle second arg', () => { + const {useImperativeHandle} = React; + const App = React.forwardRef((props, ref) => { + useImperativeHandle(ref, { + focus() {}, + }); + return null; + }); + + expect(() => { + ReactTestRenderer.create(); + }).toWarnDev([ + 'Expected useImperativeHandle() second argument to be a function ' + + 'that creates a handle. Instead received: object.', + ]); + }); + // https://github.com/facebook/react/issues/14022 it('works with ReactDOMServer calls inside a component', () => { const {useState} = React;