fix(Toast): fix infinite loop in unit tests

timolins/react-hot-toast#107 (comment)
kiwicom · Jan 20, 2022 · 4fdbe0e · 4fdbe0e
1 parent 5d6fdb9
commit 4fdbe0e
Show file tree

Hide file tree

Showing 6 changed files with 100 additions and 70 deletions.
diff --git a/packages/orbit-components/src/Toast/ToastMessage.jsx b/packages/orbit-components/src/Toast/ToastMessage.jsx
@@ -6,7 +6,7 @@ import mq from "../utils/mediaQuery";
 import Stack from "../Stack";
 import defaultTheme from "../defaultTheme";
 import Text from "../Text";
-import { fadeIn, fadeOut, lightAnimation, getPositionStyle } from "./helpers";
+import { fadeIn, fadeOut, lightAnimation, getPositionStyle, createRectRef } from "./helpers";
 import useTheme from "../hooks/useTheme";
 import useSwipe from "./hooks/useSwipe";
 import mergeRefs from "../utils/mergeRefs";
@@ -83,65 +83,69 @@ StyledInnerWrapper.defaultProps = {
   theme: defaultTheme,
 };
 
-const ToastMessage: React.AbstractComponent<Props, HTMLDivElement> = React.forwardRef(
-  (
-    {
-      onMouseEnter,
-      onMouseLeave,
-      visible,
-      onDismiss,
-      dismissTimeout,
-      placement,
-      icon,
-      children,
-      offset,
-      ariaLive,
-    },
-    ref,
-  ): React.Node => {
-    const theme = useTheme();
-    const innerRef = React.useRef(null);
-    const [isPaused, setPaused] = React.useState(false);
-    const { swipeOffset, swipeOpacity } = useSwipe(
-      innerRef,
-      onDismiss,
-      50,
-      placement.match(/right|center/) ? "right" : "left",
-    );
+const ToastMessage = ({
+  id,
+  onUpdateHeight,
+  onMouseEnter,
+  onMouseLeave,
+  visible,
+  onDismiss,
+  dismissTimeout,
+  placement,
+  icon,
+  children,
+  offset,
+  ariaLive,
+}: Props): React.Node => {
+  const theme = useTheme();
+  const measurerRef = React.useMemo(
+    () => createRectRef(({ height }) => onUpdateHeight(id, height)),
+    // it's safer to include children as well because if they change then we need to remeasure
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+    [onUpdateHeight, id, children],
+  );
+  const innerRef = React.useRef(null);
+  const mergedRef = React.useMemo(() => mergeRefs([measurerRef, innerRef]), [measurerRef]);
+  const [isPaused, setPaused] = React.useState(false);
+  const { swipeOffset, swipeOpacity } = useSwipe(
+    innerRef,
+    onDismiss,
+    50,
+    placement.match(/right|center/) ? "right" : "left",
+  );
 
-    return (
-      <StyledWrapper
-        ariaLive={ariaLive}
-        opacity={swipeOpacity}
+  return (
+    <StyledWrapper
+      ariaLive={ariaLive}
+      opacity={swipeOpacity}
+      visible={visible}
+      offsetY={offset}
+      offsetX={swipeOffset}
+      placement={placement}
+    >
+      <StyledInnerWrapper
         visible={visible}
-        offsetY={offset}
-        offsetX={swipeOffset}
-        placement={placement}
+        ref={mergedRef}
+        isPaused={isPaused}
+        duration={dismissTimeout}
+        onMouseEnter={() => {
+          onMouseEnter();
+          setPaused(true);
+        }}
+        onMouseLeave={() => {
+          onMouseLeave();
+          setPaused(false);
+        }}
       >
-        <StyledInnerWrapper
-          visible={visible}
-          ref={mergeRefs([ref, innerRef])}
-          isPaused={isPaused}
-          duration={dismissTimeout}
-          onMouseEnter={() => {
-            onMouseEnter();
-            setPaused(true);
-          }}
-          onMouseLeave={() => {
-            onMouseLeave();
-            setPaused(false);
-          }}
-        >
-          <Stack flex shrink spacing="XSmall">
-            {icon &&
-              React.isValidElement(icon) &&
-              React.cloneElement(icon, { size: "small", customColor: theme.orbit.paletteWhite })}
-            <Text type="white">{children}</Text>
-          </Stack>
-        </StyledInnerWrapper>
-      </StyledWrapper>
-    );
-  },
-);
+        <Stack flex shrink spacing="XSmall">
+          {icon &&
+            React.isValidElement(icon) &&
+            React.cloneElement(icon, { size: "small", customColor: theme.orbit.paletteWhite })}
+          <Text type="white">{children}</Text>
+        </Stack>
+      </StyledInnerWrapper>
+    </StyledWrapper>
+  );
+};
 
 export default ToastMessage;
diff --git a/packages/orbit-components/src/Toast/ToastRoot.jsx b/packages/orbit-components/src/Toast/ToastRoot.jsx
@@ -3,7 +3,6 @@ import * as React from "react";
 import { useToaster, toast as notify } from "react-hot-toast";
 import styled, { css } from "styled-components";
 
-import { createRectRef } from "./helpers";
 import ToastMessage from "./ToastMessage";
 import defaultTheme from "../defaultTheme";
 import { left, right } from "../utils/rtl";
@@ -42,6 +41,8 @@ const ToastRoot = ({
   });
 
   const { startPause, endPause, calculateOffset, updateHeight } = handlers;
+  // eslint-disable-next-line react-hooks/exhaustive-deps
+  const handleUpdateHeight = React.useCallback(updateHeight, []);
 
   return (
     <StyledWrapper
@@ -58,24 +59,19 @@ const ToastRoot = ({
           gutter,
         });
 
-        const ref = toast.height
-          ? undefined
-          : createRectRef(({ height }) => {
-              updateHeight(id, height);
-            });
-
         return (
           <ToastMessage
             key={id}
-            ref={ref}
+            id={id}
             dismissTimeout={dismissTimeout}
             visible={visible}
             icon={icon}
             offset={offset}
+            onUpdateHeight={handleUpdateHeight}
             onMouseEnter={startPause}
             onMouseLeave={endPause}
             placement={placement}
-            onDismiss={() => notify.dismiss(id)}
+            onDismiss={notify.dismiss}
             ariaLive={ariaProps["aria-live"]}
           >
             {message}

diff --git a/packages/orbit-components/src/Toast/__tests__/index.test.jsx b/packages/orbit-components/src/Toast/__tests__/index.test.jsx
@@ -7,13 +7,17 @@ import { ToastRoot, createToast } from "..";
 import { Airplane } from "../../icons";
 import Toast from "../ToastMessage";
 import Button from "../../Button";
-import { SWIPE_DISMISS_DELAY } from "../hooks/useSwipe";
+import { EXPIRE_DISMISS_DELAY, SWIPE_DISMISS_DELAY } from "../consts";
 
 beforeEach(() => {
   // reset mocks before each test
   jest.useFakeTimers();
 });
 
+afterEach(() => {
+  jest.runOnlyPendingTimers();
+});
+
 afterAll(() => {
   jest.useRealTimers();
 });
@@ -26,7 +30,9 @@ describe("Toast", () => {
 
     render(
       <Toast
+        id="1"
         icon={<Airplane dataTest="airplane" />}
+        onUpdateHeight={() => {}}
         onMouseEnter={onMouseEnter}
         onMouseLeave={onMouseLeave}
         ariaLive="polite"
@@ -68,7 +74,13 @@ describe("Toast", () => {
           topOffset={30}
           bottomOffset={40}
         />
-        <Button onClick={() => createToast("kek", { icon: <Airplane /> })}>Add toast</Button>
+        <Button
+          onClick={() => {
+            createToast("kek", { icon: <Airplane /> });
+          }}
+        >
+          Add toast
+        </Button>
       </>,
     );
 
@@ -84,4 +96,13 @@ describe("Toast", () => {
 
     expect(screen.getByRole("status")).toBeInTheDocument();
   });
+
+  it("should be removed from DOM on dismiss", () => {
+    const dismissTimeout = 300;
+    render(<ToastRoot dismissTimeout={dismissTimeout} />);
+    act(() => createToast("kek", { icon: <Airplane /> }));
+    // TODO: find out why it needs an additional millisecond
+    act(() => jest.advanceTimersByTime(dismissTimeout + EXPIRE_DISMISS_DELAY + 1));
+    expect(screen.queryByRole("status")).not.toBeInTheDocument();
+  });
 });
diff --git a/packages/orbit-components/src/Toast/consts.js b/packages/orbit-components/src/Toast/consts.js
@@ -0,0 +1,7 @@
+// @flow
+
+// this is hardcoded into react-hot-toast
+// https://github.com/timolins/react-hot-toast/blob/c5e59351d511d8702b065378a6859c795b05547d/src/core/store.ts#L64
+export const EXPIRE_DISMISS_DELAY = 1000;
+
+export const SWIPE_DISMISS_DELAY = 300;
diff --git a/packages/orbit-components/src/Toast/hooks/useSwipe.js b/packages/orbit-components/src/Toast/hooks/useSwipe.js
@@ -1,7 +1,7 @@
 // @flow
 import { useCallback, useEffect, useState } from "react";
 
-export const SWIPE_DISMISS_DELAY = 300;
+import { SWIPE_DISMISS_DELAY } from "../consts";
 
 export default function useSwipeToDismiss(
   ref: {| current: HTMLElement | null |},

diff --git a/packages/orbit-components/src/Toast/index.jsx.flow b/packages/orbit-components/src/Toast/index.jsx.flow
@@ -15,10 +15,12 @@ export type Placement =
   | "bottom-right";
 
 export type Toast = {|
+  +id: string,
   +icon?: React.Element<any>,
   +visible?: boolean,
   +children: React.Node,
   +dismissTimeout?: number,
+  +onUpdateHeight: (id: string, height: number) => void,
   +onMouseEnter: () => void,
   +onMouseLeave: () => void,
   +onDismiss: () => void,