fix: when popupVisible back to undefined, it should be false (#335)

Author: zombieJ

src/index.tsx

@@ -5,7 +5,6 @@ import ResizeObserver from 'rc-resize-observer';
 import useEvent from 'rc-util/lib/hooks/useEvent';
 import useId from 'rc-util/lib/hooks/useId';
 import useLayoutEffect from 'rc-util/lib/hooks/useLayoutEffect';
-import useMergedState from 'rc-util/lib/hooks/useMergedState';
 import * as React from 'react';
 import type { TriggerContextProps } from './context';
 import TriggerContext from './context';
@@ -266,12 +265,24 @@ export function generateTrigger(
     );
 
     // ============================ Open ============================
-    const [mergedOpen, setMergedOpen] = useMergedState(
+    const [internalOpen, setInternalOpen] = React.useState(
       defaultPopupVisible || false,
-      {
-        value: popupVisible,
-      },
     );
+
+    // Render still use props as first priority
+    const mergedOpen = popupVisible ?? internalOpen;
+
+    // We use effect sync here in case `popupVisible` back to `undefined`
+    const setMergedOpen = useEvent((nextOpen: boolean) => {
+      if (popupVisible === undefined) {
+        setInternalOpen(nextOpen);
+      }
+    });
+
+    useLayoutEffect(() => {
+      setInternalOpen(popupVisible || false);
+    }, [popupVisible]);
+
     const openRef = React.useRef(mergedOpen);
     openRef.current = mergedOpen;
 

tests/basic.test.jsx

@@ -1,16 +1,11 @@
 /* eslint-disable max-classes-per-file */
 
-import {
-  act,
-  cleanup,
-  fireEvent,
-  render,
-} from '@testing-library/react';
+import { act, cleanup, fireEvent, render } from '@testing-library/react';
 import { spyElementPrototypes } from 'rc-util/lib/test/domHook';
 import React, { createRef, StrictMode } from 'react';
 import ReactDOM from 'react-dom';
 import Trigger from '../src';
-import { placementAlignMap, awaitFakeTimer } from './util';
+import { awaitFakeTimer, placementAlignMap } from './util';
 
 describe('Trigger.Basic', () => {
   beforeEach(() => {
@@ -866,14 +861,23 @@ describe('Trigger.Basic', () => {
               <div
                 id="btn"
                 onClick={() => {
-                  setPlacementAlign(prev => prev === placementAlignMap.left ? placementAlignMap.leftTop: placementAlignMap.left);
+                  setPlacementAlign((prev) =>
+                    prev === placementAlignMap.left
+                      ? placementAlignMap.leftTop
+                      : placementAlignMap.left,
+                  );
                 }}
               >
                 click
               </div>
-              <div id="close" onClick={() => {
-                setOpen(false);
-              }}>close</div>
+              <div
+                id="close"
+                onClick={() => {
+                  setOpen(false);
+                }}
+              >
+                close
+              </div>
             </div>
           </Trigger>
         </>
@@ -891,4 +895,32 @@ describe('Trigger.Basic', () => {
     await awaitFakeTimer();
     expect(onPopupAlign).toHaveBeenCalledTimes(2);
   });
+
+  it.only('popupVisible switch `undefined` and `false` should work', async () => {
+    const Demo = (props) => (
+      <Trigger
+        action={['click']}
+        popupAlign={placementAlignMap.topRight}
+        popup={<strong>trigger</strong>}
+        {...props}
+      >
+        <div className="target">click</div>
+      </Trigger>
+    );
+
+    const { container, rerender } = render(<Demo />);
+
+    trigger(container, '.target');
+    expect(document.querySelector('.rc-trigger-popup')).toBeTruthy();
+
+    // Back to false
+    rerender(<Demo popupVisible={false} />);
+    await awaitFakeTimer();
+    expect(document.querySelector('.rc-trigger-popup-hidden')).toBeTruthy();
+
+    // Back to undefined
+    rerender(<Demo popupVisible={undefined} />);
+    await awaitFakeTimer();
+    expect(document.querySelector('.rc-trigger-popup-hidden')).toBeTruthy();
+  });
 });