From 53e4e3bc5f7a3c4f52eb0677376530fc4e443617 Mon Sep 17 00:00:00 2001
From: zombiej <smith3816@gmail.com>
Date: Fri, 17 Jun 2022 17:55:48 +0800
Subject: [PATCH 1/2] refactor: Rewrite onChange logic

---
 src/hooks/useEvent.ts       | 11 +++++++++
 src/hooks/useMergedState.ts | 48 +++++++++++++++++++++++--------------
 2 files changed, 41 insertions(+), 18 deletions(-)
 create mode 100644 src/hooks/useEvent.ts

diff --git a/src/hooks/useEvent.ts b/src/hooks/useEvent.ts
new file mode 100644
index 00000000..86bdb211
--- /dev/null
+++ b/src/hooks/useEvent.ts
@@ -0,0 +1,11 @@
+import * as React from 'react';
+
+export default function useEvent<T extends Function>(callback: T): T {
+  const fnRef = React.useRef<any>();
+  fnRef.current = callback;
+
+  return React.useCallback<T>(
+    ((...args: any) => fnRef.current?.(...args)) as any,
+    [],
+  );
+}
diff --git a/src/hooks/useMergedState.ts b/src/hooks/useMergedState.ts
index 44b609c1..0bf43f72 100644
--- a/src/hooks/useMergedState.ts
+++ b/src/hooks/useMergedState.ts
@@ -1,6 +1,13 @@
 import * as React from 'react';
+import useEvent from './useEvent';
+import useLayoutEffect from './useLayoutEffect';
 import useState from './useState';
 
+type Updater<T> = (
+  updater: T | ((origin: T) => T),
+  ignoreDestroy?: boolean,
+) => void;
+
 /**
  * Similar to `useState` but will use props value if provided.
  * Note that internal use rc-util `useState` hook.
@@ -13,7 +20,7 @@ export default function useMergedState<T, R = T>(
     onChange?: (value: T, prevValue: T) => void;
     postState?: (value: T) => T;
   },
-): [R, (value: T, ignoreDestroy?: boolean) => void] {
+): [R, Updater<T>] {
   const { defaultValue, value, onChange, postState } = option || {};
   const [innerValue, setInnerValue] = useState<T>(() => {
     if (value !== undefined) {
@@ -29,24 +36,29 @@ export default function useMergedState<T, R = T>(
       : defaultStateValue;
   });
 
-  let mergedValue = value !== undefined ? value : innerValue;
-  if (postState) {
-    mergedValue = postState(mergedValue);
-  }
+  const mergedValue = value !== undefined ? value : innerValue;
+  const postMergedValue = postState ? postState(mergedValue) : mergedValue;
 
   // setState
-  const onChangeRef = React.useRef(onChange);
-  onChangeRef.current = onChange;
-
-  const triggerChange = React.useCallback(
-    (newValue: T, ignoreDestroy?: boolean) => {
-      setInnerValue(newValue, ignoreDestroy);
-      if (mergedValue !== newValue && onChangeRef.current) {
-        onChangeRef.current(newValue, mergedValue);
-      }
-    },
-    [mergedValue, onChangeRef],
-  );
+  const onChangeFn = useEvent(onChange);
+
+  const [changePrevValue, setChangePrevValue] = React.useState<T>();
+
+  const triggerChange: Updater<T> = useEvent(updater => {
+    setChangePrevValue(mergedValue);
+    setInnerValue(prev => {
+      const nextValue =
+        typeof updater === 'function' ? (updater as any)(prev) : updater;
+      return nextValue;
+    });
+  });
+
+  // Effect to trigger onChange
+  useLayoutEffect(() => {
+    if (changePrevValue !== undefined && changePrevValue !== innerValue) {
+      onChangeFn(innerValue, changePrevValue);
+    }
+  }, [changePrevValue, innerValue, onChangeFn]);
 
   // Effect of reset value to `undefined`
   const prevValueRef = React.useRef(value);
@@ -58,5 +70,5 @@ export default function useMergedState<T, R = T>(
     prevValueRef.current = value;
   }, [value]);
 
-  return [mergedValue as unknown as R, triggerChange];
+  return [postMergedValue as unknown as R, triggerChange];
 }

From 46d0d413dfdf72199a4aaa8c07672d603a71cd17 Mon Sep 17 00:00:00 2001
From: zombiej <smith3816@gmail.com>
Date: Fri, 17 Jun 2022 21:01:14 +0800
Subject: [PATCH 2/2] test: Test case

---
 tests/hooks.test.js | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/tests/hooks.test.js b/tests/hooks.test.js
index 99274d90..58f4ac43 100644
--- a/tests/hooks.test.js
+++ b/tests/hooks.test.js
@@ -124,6 +124,50 @@ describe('hooks', () => {
 
       expect(container.querySelector('div').textContent).toEqual('2');
     });
+
+    it('not trigger onChange if props change', () => {
+      const Demo = ({ value, onChange }) => {
+        const [mergedValue, setValue] = useMergedState(0, {
+          onChange,
+        });
+
+        return (
+          <>
+            <button
+              onClick={() => {
+                setValue(v => v + 1);
+              }}
+            >
+              {mergedValue}
+            </button>
+            <a
+              onClick={() => {
+                setValue(v => v + 1);
+                setValue(v => v + 1);
+              }}
+            />
+          </>
+        );
+      };
+
+      const onChange = jest.fn();
+      const { container } = render(<Demo onChange={onChange} />);
+
+      expect(container.querySelector('button').textContent).toEqual('0');
+      expect(onChange).not.toHaveBeenCalled();
+
+      // Click to change
+      fireEvent.click(container.querySelector('button'));
+      expect(container.querySelector('button').textContent).toEqual('1');
+      expect(onChange).toHaveBeenCalledWith(1, 0);
+      onChange.mockReset();
+
+      // Click to change twice in same time so should not trigger onChange twice
+      fireEvent.click(container.querySelector('a'));
+      expect(container.querySelector('button').textContent).toEqual('3');
+      expect(onChange).toHaveBeenCalledWith(3, 1);
+      onChange.mockReset();
+    });
   });
 
   describe('useLayoutEffect', () => {