fix(summary-detail): fixes issue in react <18 (#3741)

* fix: jest memory leaking * test: fix summary detail tests * test: simplify tests * test: log more visibly the error * test: fix react 17 * chore: add changeset
twilio-labs · Jan 29, 2024 · d8b33e2 · d8b33e2
1 parent d9f96d7
commit d8b33e2
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 33 deletions.
diff --git a/.changeset/warm-ads-notice.md b/.changeset/warm-ads-notice.md
@@ -0,0 +1,6 @@
+---
+"@twilio-paste/summary-detail": patch
+"@twilio-paste/core": patch
+---
+
+[SummaryDetail] Fixed an issue with React versions below 18.
diff --git a/package.json b/package.json
@@ -60,8 +60,8 @@
     "clean:full": "tsx ./tools/build/clean-repo.ts && yarn nx run-many --target=clean && rm -rf node_modules/ && yarn",
     "clean:core": "yarn workspace @twilio-paste/core clean",
     "pre-test": "tsx ./tools/build/pre-test.ts",
-    "test": "jest",
-    "test:coverage": "jest --coverage",
+    "test": "node --expose-gc --no-compilation-cache ./node_modules/.bin/jest --runInBand --logHeapUsage",
+    "test:coverage": "node --expose-gc --no-compilation-cache ./node_modules/.bin/jest --logHeapUsage --coverage",
     "test:website": "yarn cypress run --record",
     "test:website-percy": "percy exec -- yarn cypress run --record",
     "test:website-gui": "WAIT_ON_TIMEOUT=600000 start-server-and-test 'yarn start:website' http://localhost:3000 'yarn cypress open'",

diff --git a/packages/paste-core/components/summary-detail/__tests__/index.spec.tsx b/packages/paste-core/components/summary-detail/__tests__/index.spec.tsx
@@ -1,7 +1,6 @@
 import { render, screen, waitFor } from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
 import { CustomizationProvider } from "@twilio-paste/customization";
-import { Theme } from "@twilio-paste/theme";
 import * as React from "react";
 
 import {
@@ -18,17 +17,15 @@ const MockSummaryDetail: React.FC<{
   visible?: SummaryDetailProps["visible"];
 }> = ({ visible }) => {
   return (
-    <Theme.Provider theme="default">
-      <SummaryDetail baseId="summary-detail" visible={visible} data-testid="summary-detail">
-        <SummaryDetailHeading data-testid="summary-detail-heading">
-          <SummaryDetailToggleButton data-testid="summary-detail-toggle-button" />
-          <SummaryDetailHeadingContent data-testid="summary-detail-heading-content">
-            Inbound Call
-          </SummaryDetailHeadingContent>
-        </SummaryDetailHeading>
-        <SummaryDetailContent data-testid="summary-detail-content">Agent: John Doe</SummaryDetailContent>
-      </SummaryDetail>
-    </Theme.Provider>
+    <SummaryDetail baseId="summary-detail" visible={visible} data-testid="summary-detail">
+      <SummaryDetailHeading data-testid="summary-detail-heading">
+        <SummaryDetailToggleButton data-testid="summary-detail-toggle-button" />
+        <SummaryDetailHeadingContent data-testid="summary-detail-heading-content">
+          Inbound Call
+        </SummaryDetailHeadingContent>
+      </SummaryDetailHeading>
+      <SummaryDetailContent data-testid="summary-detail-content">Agent: John Doe</SummaryDetailContent>
+    </SummaryDetail>
   );
 };
 
@@ -72,7 +69,7 @@ const MockCustomElementSummaryDetail = (): JSX.Element => {
 const StateHookMock = (): JSX.Element => {
   const summaryDetail = useSummaryDetailState();
   return (
-    <Theme.Provider theme="default">
+    <>
       <button
         onClick={() => {
           summaryDetail.toggle();
@@ -97,7 +94,7 @@ const StateHookMock = (): JSX.Element => {
           Agent: John Doe
         </SummaryDetailContent>
       </SummaryDetail>
-    </Theme.Provider>
+    </>
   );
 };
 
@@ -136,9 +133,7 @@ describe("SummaryDetail", () => {
       expect(summaryDetailContent).not.toBeVisible();
       userEvent.click(summaryDetailButton);
       expect(summaryDetailButton.getAttribute("aria-expanded")).toEqual("true");
-      waitFor(() => {
-        expect(summaryDetailContent).toBeVisible();
-      });
+      expect(summaryDetailContent).toBeVisible();
     });
 
     it("should toggle open state correctly when using a state hook", async () => {
@@ -150,19 +145,13 @@ describe("SummaryDetail", () => {
       expect(summaryDetailContent).not.toBeVisible();
       userEvent.click(toggleButton);
       expect(summaryDetailButton.getAttribute("aria-expanded")).toEqual("true");
-      waitFor(() => {
-        expect(summaryDetailContent).toBeVisible();
-      });
+      expect(summaryDetailContent).toBeVisible();
     });
   });
 
   describe("Customization", () => {
     it("should set an element data attribute for SummaryDetail components", () => {
-      render(
-        <Theme.Provider theme="default">
-          <MockDefaultElementSummaryDetail />
-        </Theme.Provider>,
-      );
+      render(<MockDefaultElementSummaryDetail />);
 
       const renderedSummaryDetailHeading = screen.getByTestId("summary-detail-heading");
       const renderedSummaryDetail = screen.getByTestId("summary-detail");
@@ -182,11 +171,7 @@ describe("SummaryDetail", () => {
     });
 
     it("should set a custom element data attribute for custom named SummaryDetail components", () => {
-      render(
-        <Theme.Provider theme="default">
-          <MockCustomElementSummaryDetail />
-        </Theme.Provider>,
-      );
+      render(<MockCustomElementSummaryDetail />);
 
       const renderedSummaryDetailHeading = screen.getByTestId("summary-detail-heading");
       const renderedSummaryDetail = screen.getByTestId("summary-detail");

diff --git a/packages/paste-core/components/summary-detail/src/SummaryDetail.tsx b/packages/paste-core/components/summary-detail/src/SummaryDetail.tsx
@@ -5,6 +5,7 @@ import {
   type DisclosurePrimitveStateReturn,
   useDisclosurePrimitiveState,
 } from "@twilio-paste/disclosure-primitive";
+import { useUID } from "@twilio-paste/uid-library";
 import * as React from "react";
 
 import { SummaryDetailContext } from "./SummaryDetailContext";
@@ -34,10 +35,11 @@ export interface SummaryDetailProps extends DisclosurePrimitiveInitialState {
 
 export const SummaryDetail = React.forwardRef<HTMLDivElement, SummaryDetailProps>(
   ({ children, element = "SUMMARY_DETAIL", state, ...props }, ref) => {
+    const headerId = useUID();
     const stateForContext = state || useDisclosurePrimitiveState(props);
 
     return (
-      <SummaryDetailContext.Provider value={{ ...stateForContext, headerId: React.useId() }}>
+      <SummaryDetailContext.Provider value={{ ...stateForContext, headerId }}>
         <Box
           {...safelySpreadBoxProps(props)}
           ref={ref}