Fix useImperativeHandle to have no deps by default (#14801)

* Fix useImperativeHandle to have no deps by default * Save a byte? * Nit: null
facebook · Feb 11, 2019 · f24a0da · f24a0da
1 parent 1fecba9
commit f24a0da
Show file tree

Hide file tree

Showing 2 changed files with 107 additions and 2 deletions.
diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js
@@ -905,7 +905,7 @@ function mountImperativeHandle<T>(
 
   // TODO: If deps are provided, should we skip comparing the ref itself?
   const effectDeps =
-    deps !== null && deps !== undefined ? deps.concat([ref]) : [ref];
+    deps !== null && deps !== undefined ? deps.concat([ref]) : null;
 
   return mountEffectImpl(
     UpdateEffect,
@@ -931,7 +931,7 @@ function updateImperativeHandle<T>(
 
   // TODO: If deps are provided, should we skip comparing the ref itself?
   const effectDeps =
-    deps !== null && deps !== undefined ? deps.concat([ref]) : [ref];
+    deps !== null && deps !== undefined ? deps.concat([ref]) : null;
 
   return updateEffectImpl(
     UpdateEffect,

diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js
@@ -1593,6 +1593,111 @@ describe('ReactHooksWithNoopRenderer', () => {
     });
   });
 
+  describe('useImperativeHandle', () => {
+    it('does not update when deps are the same', () => {
+      const INCREMENT = 'INCREMENT';
+
+      function reducer(state, action) {
+        return action === INCREMENT ? state + 1 : state;
+      }
+
+      function Counter(props, ref) {
+        const [count, dispatch] = useReducer(reducer, 0);
+        useImperativeHandle(ref, () => ({count, dispatch}), []);
+        return <Text text={'Count: ' + count} />;
+      }
+
+      Counter = forwardRef(Counter);
+      const counter = React.createRef(null);
+      ReactNoop.render(<Counter ref={counter} />);
+      ReactNoop.flush();
+      expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]);
+      expect(counter.current.count).toBe(0);
+
+      act(() => {
+        counter.current.dispatch(INCREMENT);
+      });
+      ReactNoop.flush();
+      expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]);
+      // Intentionally not updated because of [] deps:
+      expect(counter.current.count).toBe(0);
+    });
+
+    // Regression test for https://github.com/facebook/react/issues/14782
+    it('automatically updates when deps are not specified', () => {
+      const INCREMENT = 'INCREMENT';
+
+      function reducer(state, action) {
+        return action === INCREMENT ? state + 1 : state;
+      }
+
+      function Counter(props, ref) {
+        const [count, dispatch] = useReducer(reducer, 0);
+        useImperativeHandle(ref, () => ({count, dispatch}));
+        return <Text text={'Count: ' + count} />;
+      }
+
+      Counter = forwardRef(Counter);
+      const counter = React.createRef(null);
+      ReactNoop.render(<Counter ref={counter} />);
+      ReactNoop.flush();
+      expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]);
+      expect(counter.current.count).toBe(0);
+
+      act(() => {
+        counter.current.dispatch(INCREMENT);
+      });
+      ReactNoop.flush();
+      expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]);
+      expect(counter.current.count).toBe(1);
+    });
+
+    it('updates when deps are different', () => {
+      const INCREMENT = 'INCREMENT';
+
+      function reducer(state, action) {
+        return action === INCREMENT ? state + 1 : state;
+      }
+
+      let totalRefUpdates = 0;
+      function Counter(props, ref) {
+        const [count, dispatch] = useReducer(reducer, 0);
+        useImperativeHandle(
+          ref,
+          () => {
+            totalRefUpdates++;
+            return {count, dispatch};
+          },
+          [count],
+        );
+        return <Text text={'Count: ' + count} />;
+      }
+
+      Counter = forwardRef(Counter);
+      const counter = React.createRef(null);
+      ReactNoop.render(<Counter ref={counter} />);
+      ReactNoop.flush();
+      expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]);
+      expect(counter.current.count).toBe(0);
+      expect(totalRefUpdates).toBe(1);
+
+      act(() => {
+        counter.current.dispatch(INCREMENT);
+      });
+      ReactNoop.flush();
+      expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]);
+      expect(counter.current.count).toBe(1);
+      expect(totalRefUpdates).toBe(2);
+
+      // Update that doesn't change the ref dependencies
+      ReactNoop.render(<Counter ref={counter} />);
+      ReactNoop.flush();
+      expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]);
+      expect(counter.current.count).toBe(1);
+      expect(totalRefUpdates).toBe(2); // Should not increase since last time
+    });
+  });
+
   describe('progressive enhancement (not supported)', () => {
     it('mount additional state', () => {
       let updateA;