refactor: useMergedState event logic (#315)

* refactor: onChange in sync

* refacor: merge all

* test: test case

* test: more

src/hooks/useEvent.ts

@@ -9,5 +9,5 @@ export default function useEvent<T extends Function>(callback: T): T {
     [],
   );
 
-  return callback ? memoFn : undefined;
+  return memoFn;
 }

src/hooks/useMergedState.ts

@@ -8,6 +8,28 @@ type Updater<T> = (
   ignoreDestroy?: boolean,
 ) => void;
 
+enum Source {
+  INNER,
+  PROP,
+}
+
+type ValueRecord<T> = [T, Source, T];
+
+const useUpdateEffect: typeof React.useEffect = (callback, deps) => {
+  const [firstMount, setFirstMount] = React.useState(true);
+
+  useLayoutEffect(() => {
+    if (!firstMount) {
+      return callback();
+    }
+  }, deps);
+
+  // We tell react that first mount has passed
+  useLayoutEffect(() => {
+    setFirstMount(false);
+  }, []);
+};
+
 /**
  * Similar to `useState` but will use props value if provided.
  * Note that internal use rc-util `useState` hook.
@@ -22,53 +44,77 @@ export default function useMergedState<T, R = T>(
   },
 ): [R, Updater<T>] {
   const { defaultValue, value, onChange, postState } = option || {};
-  const [innerValue, setInnerValue] = useState<T>(() => {
+
+  // ======================= Init =======================
+  const [mergedValue, setMergedValue] = useState<ValueRecord<T>>(() => {
+    let finalValue: T = undefined;
+    let source: Source;
+
     if (value !== undefined) {
-      return value;
-    }
-    if (defaultValue !== undefined) {
-      return typeof defaultValue === 'function'
-        ? (defaultValue as any)()
-        : defaultValue;
+      finalValue = value;
+      source = Source.PROP;
+    } else if (defaultValue !== undefined) {
+      finalValue =
+        typeof defaultValue === 'function'
+          ? (defaultValue as any)()
+          : defaultValue;
+      source = Source.PROP;
+    } else {
+      finalValue =
+        typeof defaultStateValue === 'function'
+          ? (defaultStateValue as any)()
+          : defaultStateValue;
+      source = Source.INNER;
     }
-    return typeof defaultStateValue === 'function'
-      ? (defaultStateValue as any)()
-      : defaultStateValue;
+
+    return [finalValue, source, finalValue];
   });
 
-  const mergedValue = value !== undefined ? value : innerValue;
-  const postMergedValue = postState ? postState(mergedValue) : mergedValue;
+  const postMergedValue = postState
+    ? postState(mergedValue[0])
+    : mergedValue[0];
 
-  // setState
-  const onChangeFn = useEvent(onChange);
+  // ======================= Sync =======================
+  useUpdateEffect(() => {
+    setMergedValue(([prevValue]) => [value, Source.PROP, prevValue]);
+  }, [value]);
 
-  const [changePrevValue, setChangePrevValue] = useState<T>();
+  // ====================== Update ======================
+  const changeEventPrevRef = React.useRef<T>();
 
   const triggerChange: Updater<T> = useEvent((updater, ignoreDestroy) => {
-    setChangePrevValue(mergedValue, true);
-    setInnerValue(prev => {
-      const nextValue =
-        typeof updater === 'function' ? (updater as any)(prev) : updater;
-      return nextValue;
+    setMergedValue(prev => {
+      const [prevValue, prevSource, prevPrevValue] = prev;
+
+      const nextValue: T =
+        typeof updater === 'function' ? (updater as any)(prevValue) : updater;
+
+      // Do nothing if value not change
+      if (nextValue === prevValue) {
+        return prev;
+      }
+
+      // Use prev prev value if is in a batch update to avoid missing data
+      const overridePrevValue =
+        prevSource === Source.INNER &&
+        changeEventPrevRef.current !== prevPrevValue
+          ? prevPrevValue
+          : prevValue;
+
+      return [nextValue, Source.INNER, overridePrevValue];
     }, ignoreDestroy);
   });
 
-  // Effect to trigger onChange
-  useLayoutEffect(() => {
-    if (changePrevValue !== undefined && changePrevValue !== innerValue) {
-      onChangeFn?.(innerValue, changePrevValue);
-    }
-  }, [changePrevValue, innerValue, onChangeFn]);
+  // ====================== Change ======================
+  const onChangeFn = useEvent(onChange);
 
-  // Effect of reset value to `undefined`
-  const prevValueRef = React.useRef(value);
-  React.useEffect(() => {
-    if (value === undefined && value !== prevValueRef.current) {
-      setInnerValue(value);
+  useLayoutEffect(() => {
+    const [current, source, prev] = mergedValue;
+    if (current !== prev && source === Source.INNER) {
+      onChangeFn(current, prev);
+      changeEventPrevRef.current = prev;
     }
-
-    prevValueRef.current = value;
-  }, [value]);
+  }, [mergedValue]);
 
   return [postMergedValue as unknown as R, triggerChange];
 }

tests/hooks.test.js

@@ -67,10 +67,18 @@ describe('hooks', () => {
       expect(container.querySelector('input').value).toEqual('test');
     });
 
-    it('correct defaultValue', () => {
-      const { container } = render(<FC defaultValue="test" />);
+    describe('correct defaultValue', () => {
+      it('raw', () => {
+        const { container } = render(<FC defaultValue="test" />);
 
-      expect(container.querySelector('input').value).toEqual('test');
+        expect(container.querySelector('input').value).toEqual('test');
+      });
+
+      it('func', () => {
+        const { container } = render(<FC defaultValue={() => 'bamboo'} />);
+
+        expect(container.querySelector('input').value).toEqual('bamboo');
+      });
     });
 
     it('not rerender when setState as deps', () => {
@@ -125,48 +133,123 @@ 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, {
+    describe('not trigger onChange if props change', () => {
+      function test(name, postWrapper = node => node) {
+        it(name, () => {
+          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(
+            postWrapper(<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();
+        });
+      }
+
+      test('raw');
+      test('strict', node => <React.StrictMode>{node}</React.StrictMode>);
+    });
+
+    it('uncontrolled to controlled', () => {
+      const onChange = jest.fn();
+
+      const Demo = ({ value }) => {
+        const [mergedValue, setMergedValue] = useMergedState(() => 233, {
+          value,
           onChange,
         });
 
         return (
-          <>
-            <button
-              onClick={() => {
-                setValue(v => v + 1);
-              }}
-            >
-              {mergedValue}
-            </button>
-            <a
-              onClick={() => {
-                setValue(v => v + 1);
-                setValue(v => v + 1);
-              }}
-            />
-          </>
+          <span
+            onClick={() => {
+              setMergedValue(v => v + 1);
+              setMergedValue(v => v + 1);
+            }}
+          >
+            {mergedValue}
+          </span>
         );
       };
 
-      const onChange = jest.fn();
-      const { container } = render(<Demo onChange={onChange} />);
-
-      expect(container.querySelector('button').textContent).toEqual('0');
+      const { container, rerender } = render(<Demo />);
+      expect(container.textContent).toEqual('233');
       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();
+      // Update value
+      rerender(<Demo value={1} />);
+      expect(container.textContent).toEqual('1');
+      expect(onChange).not.toHaveBeenCalled();
 
-      // 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');
+      // Click update
+      fireEvent.click(container.querySelector('span'));
+      expect(container.textContent).toEqual('3');
       expect(onChange).toHaveBeenCalledWith(3, 1);
-      onChange.mockReset();
+    });
+
+    it('not trigger onChange if set same value', () => {
+      const onChange = jest.fn();
+
+      const Test = ({ value }) => {
+        const [mergedValue, setMergedValue] = useMergedState(undefined, {
+          value,
+          onChange,
+        });
+        return (
+          <span
+            onClick={() => {
+              setMergedValue(1);
+            }}
+            onMouseEnter={() => {
+              setMergedValue(2);
+            }}
+          >
+            {mergedValue}
+          </span>
+        );
+      };
+
+      const { container } = render(<Test value={1} />);
+      fireEvent.click(container.querySelector('span'));
+      expect(onChange).not.toHaveBeenCalled();
+
+      fireEvent.mouseEnter(container.querySelector('span'));
+      expect(onChange).toHaveBeenCalledWith(2, 1);
     });
   });