Track EuiPopover's setTimeout and requestAnimationFrame and clear the…

…m on unmount (#2614)

* Now tracking all setTimeout and requestAnimationFrame calls by EuiPopover and clears them on unmount

* changelog

* Better quality unit test for clearing popover's listeners

CHANGELOG.md

@@ -11,6 +11,7 @@
 - Fixed `EuiIcon` accessibility by adding a `title` prop and a default `aria-label` ([#2554](https://github.com/elastic/eui/pull/2554))
 - Fixed `EuiDataGrid`'s in-memory sorting of numeric columns when the cell data contains multiple digit groups ([#2603](https://github.com/elastic/eui/pull/2603))
 - Improved pagination in `EuiBasicTable`. `paginationBar` is hidden when there is no data and `EuiPagination` is displayed even when there is only one page ([#2598](https://github.com/elastic/eui/pull/#2598))
+- Fixed react-dom warning when `EuiPopover` was unmounted before calls to setState ([#2614](https://github.com/elastic/eui/pull/2614))
 
 ## [`17.0.0`](https://github.com/elastic/eui/tree/v17.0.0)
 

src/components/popover/popover.test.tsx

@@ -254,6 +254,64 @@ describe('EuiPopover', () => {
       });
     });
   });
+
+  describe('listener cleanup', () => {
+    let _raf: typeof window['requestAnimationFrame'];
+    let _caf: typeof window['cancelAnimationFrame'];
+    beforeAll(() => {
+      jest.useFakeTimers();
+      _raf = window.requestAnimationFrame;
+      _caf = window.cancelAnimationFrame;
+
+      const activeAnimationFrames = new Map<number, number>();
+      let nextAnimationFrameId = 0;
+      window.requestAnimationFrame = fn => {
+        const animationFrameId = nextAnimationFrameId++;
+        activeAnimationFrames.set(animationFrameId, setTimeout(fn));
+        return animationFrameId;
+      };
+      window.cancelAnimationFrame = (id: number) => {
+        const timeoutId = activeAnimationFrames.get(id);
+        if (timeoutId) {
+          clearTimeout(timeoutId);
+          activeAnimationFrames.delete(id);
+        }
+      };
+    });
+
+    afterAll(() => {
+      jest.useRealTimers();
+      window.requestAnimationFrame = _raf;
+      window.cancelAnimationFrame = _caf;
+    });
+
+    it('cleans up timeouts and rAFs on unmount', () => {
+      const component = mount(
+        <EuiPopover
+          id={getId()}
+          button={<button />}
+          closePopover={() => {}}
+          panelPaddingSize="s"
+          isOpen={false}
+        />
+      );
+
+      component.setProps({ isOpen: true });
+
+      component.unmount();
+
+      // EUI's jest configuration throws an error if there are any console.error calls, like
+      // React's setState on an unmounted component warning
+      // to be future proof, verify that's still the case
+      expect(() => {
+        console.error('This is a test');
+      }).toThrow();
+
+      // execute any pending timeouts or animation frame callbacks
+      // and validate the timeout/rAF clearing done by EuiPopover
+      jest.advanceTimersByTime(10);
+    });
+  });
 });
 
 describe('getPopoverPositionFromAnchorPosition', () => {

src/components/popover/popover.tsx

@@ -270,7 +270,10 @@ export class EuiPopover extends Component<Props, State> {
     return null;
   }
 
+  private respositionTimeout: number | undefined;
   private closingTransitionTimeout: number | undefined;
+  private closingTransitionAnimationFrame: number | undefined;
+  private updateFocusAnimationFrame: number | undefined;
   private button: HTMLElement | null = null;
   private panel: HTMLElement | null = null;
 
@@ -304,7 +307,7 @@ export class EuiPopover extends Component<Props, State> {
 
   updateFocus() {
     // Wait for the DOM to update.
-    window.requestAnimationFrame(() => {
+    this.updateFocusAnimationFrame = window.requestAnimationFrame(() => {
       if (!this.props.ownFocus || !this.panel) {
         return;
       }
@@ -374,11 +377,13 @@ export class EuiPopover extends Component<Props, State> {
       clearTimeout(this.closingTransitionTimeout);
       // We need to set this state a beat after the render takes place, so that the CSS
       // transition can take effect.
-      window.requestAnimationFrame(() => {
-        this.setState({
-          isOpening: true,
-        });
-      });
+      this.closingTransitionAnimationFrame = window.requestAnimationFrame(
+        () => {
+          this.setState({
+            isOpening: true,
+          });
+        }
+      );
 
       // for each child element of `this.panel`, find any transition duration we should wait for before stabilizing
       const { durationMatch, delayMatch } = Array.prototype.slice
@@ -398,7 +403,7 @@ export class EuiPopover extends Component<Props, State> {
           { durationMatch: 0, delayMatch: 0 }
         );
 
-      setTimeout(() => {
+      this.respositionTimeout = window.setTimeout(() => {
         this.setState({ isOpenStable: true }, () => {
           this.positionPopoverFixed();
           this.updateFocus();
@@ -429,7 +434,10 @@ export class EuiPopover extends Component<Props, State> {
 
   componentWillUnmount() {
     window.removeEventListener('scroll', this.positionPopoverFixed);
+    clearTimeout(this.respositionTimeout);
     clearTimeout(this.closingTransitionTimeout);
+    cancelAnimationFrame(this.closingTransitionAnimationFrame!);
+    cancelAnimationFrame(this.updateFocusAnimationFrame!);
   }
 
   onMutation = (records: MutationRecord[]) => {