fix: Patch for getPopupContainer check (#200)

* chore: Not trigger getContainer when dom not ready

* test: check for nodes test

* update deps

* clean up

package.json

@@ -61,6 +61,6 @@
     "classnames": "^2.2.6",
     "rc-align": "^4.0.0",
     "rc-motion": "^2.0.0",
-    "rc-util": "^5.2.1"
+    "rc-util": "^5.3.4"
   }
 }

src/index.tsx

@@ -146,6 +146,8 @@ export function generateTrigger(
 
     triggerRef = React.createRef<React.ReactInstance>();
 
+    attachId?: number;
+
     clickOutsideHandler: CommonEventHandler;
 
     touchOutsideHandler: CommonEventHandler;
@@ -540,16 +542,26 @@ export function generateTrigger(
     };
 
     attachParent = (popupContainer: HTMLDivElement) => {
-      const { props } = this;
-      const mountNode = props.getPopupContainer
-        ? props.getPopupContainer(this.getRootDomNode())
-        : props.getDocument().body;
+      raf.cancel(this.attachId);
+
+      const { getPopupContainer, getDocument } = this.props;
+      const domNode = this.getRootDomNode();
+
+      let mountNode: HTMLElement;
+      if (!getPopupContainer) {
+        mountNode = getDocument().body;
+      } else if (domNode || getPopupContainer.length === 0) {
+        // Compatible for legacy getPopupContainer with domNode argument.
+        // If no need `domNode` argument, will call directly.
+        // https://codesandbox.io/s/eloquent-mclean-ss93m?file=/src/App.js
+        mountNode = getPopupContainer(domNode);
+      }
 
       if (mountNode) {
         mountNode.appendChild(popupContainer);
       } else {
         // Retry after frame render in case parent not ready
-        raf(() => {
+        this.attachId = raf(() => {
           this.attachParent(popupContainer);
         });
       }

tests/basic.test.jsx

@@ -1,5 +1,6 @@
 import React from 'react';
 import { mount } from 'enzyme';
+import { act } from 'react-dom/test-utils';
 import { spyElementPrototypes } from 'rc-util/lib/test/domHook';
 import Portal from 'rc-util/lib/Portal';
 import Trigger from '../src';
@@ -683,4 +684,60 @@ describe('Trigger.Basic', () => {
     expect(popupDomNode.style.color).toBe(style.color);
     expect(popupDomNode.style.top).toBe(`${style.top}px`);
   });
+
+  describe('getContainer', () => {
+    it('not trigger when dom not ready', () => {
+      const getPopupContainer = jest.fn(node => node.parentElement);
+
+      function Demo() {
+        return (
+          <Trigger
+            popupVisible
+            getPopupContainer={getPopupContainer}
+            popup={<strong className="x-content">tooltip2</strong>}
+          >
+            <div className="target">click</div>
+          </Trigger>
+        );
+      }
+
+      const wrapper = mount(<Demo />);
+
+      expect(getPopupContainer).toHaveReturnedTimes(0);
+
+      act(() => {
+        jest.runAllTimers();
+      });
+      expect(getPopupContainer).toHaveReturnedTimes(1);
+      expect(getPopupContainer).toHaveBeenCalledWith(
+        wrapper.find('.target').instance(),
+      );
+    });
+
+    it('not trigger when dom no need', () => {
+      let triggerTimes = 0;
+      const getPopupContainer = () => {
+        triggerTimes += 1;
+        return document.body;
+      };
+
+      function Demo() {
+        return (
+          <Trigger
+            popupVisible
+            getPopupContainer={getPopupContainer}
+            popup={<strong className="x-content">tooltip2</strong>}
+          >
+            <div className="target">click</div>
+          </Trigger>
+        );
+      }
+
+      const wrapper = mount(<Demo />);
+
+      expect(triggerTimes).toEqual(1);
+
+      wrapper.unmount();
+    });
+  });
 });