Warn about incorrect use of useImperativeHandle() (#14647)

facebook · Jan 21, 2019 · 8f45a7f · 8f45a7f
1 parent 1fcbd22
commit 8f45a7f
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 0 deletions.
diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js
@@ -674,6 +674,12 @@ export function useImperativeHandle<T>(
 ): 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<T>(
       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
@@ -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
@@ -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(<App />);
+      }).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(<App />);
+    }).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;