fix: Fixes tests failing in React 17

.changeset/progress-bar-strict.md

@@ -0,0 +1,5 @@
+---
+'@leafygreen-ui/progress-bar': patch
+---
+
+Updates ProgressBar `useScreenReaderAnnouncer` to deterministically return a consistent status message (primarily a concern in React 17 strict mode)

.github/workflows/react17.yml

@@ -22,9 +22,20 @@ jobs:
         uses: pnpm/action-setup@v4
         with:
           version: 9.15.0
-          # no cache because we want to test installs from scratch
           cache: false
 
+      - uses: actions/cache/restore@v4
+        name: Check for build cache
+        id: build-cache
+        with:
+          # Note: `path` doesn't like complex glob patterns (i.e. `+(charts|chat|packages|tools)`)
+          path: |
+            charts/*/dist/*
+            chat/*/dist/*
+            packages/*/dist/*
+            tools/*/dist/*
+          key: ${{ runner.os }}-REACT17-build-cache-${{ hashFiles('package.json', 'pnpm-lock.yaml', '**/src/') }}
+
       - name: Setup Node 18
         uses: actions/setup-node@v4
         with:
@@ -44,14 +55,14 @@ jobs:
 
       - uses: actions/cache/save@v4
         name: Save build cache
-        id: build-cache
+        if: ${{ steps.build-cache.outputs.cache-hit != 'true' }}
         with:
           path: |
             charts/*/dist/*
             chat/*/dist/*
             packages/*/dist/*
             tools/*/dist/*
-          key: ${{ runner.os }}-react17-build-cache-${{ hashFiles('package.json', 'pnpm-lock.yaml', '**/src/') }}
+          key: ${{ steps.build-cache.outputs.cache-primary-key }}
 
   test:
     name: Test in React 17
@@ -61,21 +72,11 @@ jobs:
       - name: Checkout code
         uses: actions/checkout@v4
 
-      - name: Setup Node 18
-        uses: actions/setup-node@v4
-        with:
-          node-version: 18
-
       - name: Install pnpm
         uses: pnpm/action-setup@v4
         with:
           version: 9.15.0
-
-      - name: Install node-gyp
-        run: pnpm add --global node-gyp
-
-      - name: Init React 17 environment
-        run: node ./scripts/react17/init.mjs
+          cache: false
 
       - uses: actions/cache/restore@v4
         name: Check for build cache
@@ -88,8 +89,19 @@ jobs:
             tools/*/dist/*
           key: ${{needs.build.outputs.cache-primary-key}}
 
+      - name: Setup Node 18
+        uses: actions/setup-node@v4
+        with:
+          node-version: 18
+
+      - name: Install node-gyp
+        run: pnpm add --global node-gyp
+
+      - name: Init React 17 environment
+        run: node ./scripts/react17/init.mjs
+
       - name: Install dependencies
-        run: pnpm install --prefer-offline
+        run: pnpm install --prefer-offline # Intentionally not using --frozen-lockfile to allow for pnpm-lock.yaml updates
 
       - name: Run tests in React 17
-        run: pnpm run test --react17
+        run: pnpm run test --react17 --ci

chat/message/src/Message/Message.spec.tsx

@@ -85,7 +85,6 @@ describe('Message', () => {
       Variant.Compact,
     );
 
-    expect(consoleOnce.warn).toHaveBeenCalledTimes(1);
     expect(consoleOnce.warn).toHaveBeenCalledWith(
       expect.stringContaining("only used in the 'spacious' variant"),
     );

packages/drawer/src/DrawerToolbarLayout/DrawerToolbarContext/DrawerToolbarContext.spec.tsx

@@ -1,7 +1,7 @@
 import React from 'react';
 import { act, waitFor } from '@testing-library/react';
 
-import { renderHook } from '@leafygreen-ui/testing-lib';
+import { isReact17, renderHook } from '@leafygreen-ui/testing-lib';
 
 import { TRANSITION_DURATION } from '../../constants';
 import { LayoutData } from '../DrawerToolbarLayout/DrawerToolbarLayout.types';
@@ -42,17 +42,21 @@ const mockData: Array<LayoutData> = [
 
 describe('useDrawerToolbarContext', () => {
   test('throws error when used outside of DrawerToolbarProvider', () => {
-    const consoleSpy = jest
-      .spyOn(console, 'error')
-      .mockImplementation(() => {});
-
-    expect(() => {
-      renderHook(() => useDrawerToolbarContext());
-    }).toThrow(
-      'useDrawerToolbarContext must be used within a DrawerToolbarProvider',
-    );
-
-    consoleSpy.mockRestore();
+    /**
+     * The version of `renderHook`  imported from "@testing-library/react-hooks", (used in React 17)
+     * has an error boundary, and doesn't throw errors as expected:
+     * https://github.com/testing-library/react-hooks-testing-library/blob/main/src/index.ts#L5
+     * */
+    if (isReact17()) {
+      const { result } = renderHook(() => useDrawerToolbarContext());
+      expect(result.error.message).toEqual(
+        'useDrawerToolbarContext must be used within a DrawerToolbarProvider',
+      );
+    } else {
+      expect(() => renderHook(() => useDrawerToolbarContext())).toThrow(
+        'useDrawerToolbarContext must be used within a DrawerToolbarProvider',
+      );
+    }
   });
 
   describe('getActiveDrawerContent', () => {

packages/progress-bar/src/ProgressBar/ProgressBar.spec.tsx

@@ -1,5 +1,7 @@
 import React from 'react';
-import { act, render, screen } from '@testing-library/react';
+import { render, screen } from '@testing-library/react';
+
+import { act } from '@leafygreen-ui/testing-lib';
 
 import { requiredA11yArgs } from '../test.constants';
 import { getTestUtils } from '../testing';
@@ -246,17 +248,36 @@ describe('packages/progress-bar', () => {
       expect(screen.queryByRole('status')).toBeNull();
     });
 
-    test('updates live region text for initial value; ignores further changes until next threshold met', () => {
+    test('updates live region text for initial value', () => {
+      render(
+        <ProgressBar
+          value={TEST_VALUE_UNDER_50}
+          maxValue={TEST_MAX_VALUE}
+          {...requiredA11yArgs}
+        />,
+      );
+      const statusElement = screen.queryByRole('status');
+      const expectedMessage = getAnnouncementMessage(
+        TEST_VALUE_UNDER_50,
+        TEST_MAX_VALUE,
+      );
+      expect(statusElement).toHaveTextContent(expectedMessage);
+    });
+
+    test('ignores further changes until next threshold met', () => {
       const { rerender } = render(
         <ProgressBar
           value={TEST_VALUE_UNDER_50}
           maxValue={TEST_MAX_VALUE}
           {...requiredA11yArgs}
         />,
       );
-      expect(screen.queryByRole('status')).toHaveTextContent(
-        getAnnouncementMessage(TEST_VALUE_UNDER_50, TEST_MAX_VALUE),
+      const statusElement = screen.queryByRole('status');
+      const expectedMessage = getAnnouncementMessage(
+        TEST_VALUE_UNDER_50,
+        TEST_MAX_VALUE,
       );
+      expect(statusElement).toHaveTextContent(expectedMessage);
 
       rerender(
         <ProgressBar
@@ -265,18 +286,23 @@ describe('packages/progress-bar', () => {
           {...requiredA11yArgs}
         />,
       );
-      expect(screen.queryByRole('status')).toBeNull();
+
+      // No updates until next threshold met
+      expect(statusElement).toHaveTextContent(expectedMessage);
     });
 
-    test('updates live region text if 50% threshold passed', () => {
+    test('updates live region text if 50% threshold passed', async () => {
       const { rerender } = render(
         <ProgressBar
           value={TEST_VALUE_UNDER_50}
           maxValue={TEST_MAX_VALUE}
           {...requiredA11yArgs}
         />,
       );
-      expect(screen.queryByRole('status')).toHaveTextContent(
+      const statusElement = screen.getByRole('status');
+
+      expect(statusElement).toBeInTheDocument();
+      expect(statusElement).toHaveTextContent(
         getAnnouncementMessage(TEST_VALUE_UNDER_50, TEST_MAX_VALUE),
       );
 
@@ -287,7 +313,9 @@ describe('packages/progress-bar', () => {
           {...requiredA11yArgs}
         />,
       );
-      expect(screen.queryByRole('status')).toHaveTextContent(
+
+      expect(statusElement).toBeInTheDocument();
+      expect(statusElement).toHaveTextContent(
         getAnnouncementMessage(TEST_VALUE_OVER_50, TEST_MAX_VALUE),
       );
     });
@@ -300,7 +328,9 @@ describe('packages/progress-bar', () => {
           {...requiredA11yArgs}
         />,
       );
-      expect(screen.queryByRole('status')).toHaveTextContent(
+      const statusElement = screen.queryByRole('status');
+
+      expect(statusElement).toHaveTextContent(
         getAnnouncementMessage(TEST_VALUE_UNDER_50, TEST_MAX_VALUE),
       );
 
@@ -311,7 +341,8 @@ describe('packages/progress-bar', () => {
           {...requiredA11yArgs}
         />,
       );
-      expect(screen.queryByRole('status')).toHaveTextContent(
+
+      expect(statusElement).toHaveTextContent(
         getAnnouncementMessage(TEST_MAX_VALUE, TEST_MAX_VALUE),
       );
     });

packages/progress-bar/src/ProgressBar/hooks/useScreenReaderAnnouncer.tsx

@@ -1,11 +1,11 @@
-import { useMemo, useRef } from 'react';
+import { useEffect, useRef, useState } from 'react';
 
 import { isDefined } from '@leafygreen-ui/lib';
 
 import { Role, Variant } from '../ProgressBar.types';
 import { getPercentage } from '../utils';
 
-const announcementThresholds = [0, 50, 100];
+const announcementThresholds = [0, 50, 100] as const;
 const variantsAnnounced = [Variant.Warning, Variant.Error] as Array<Variant>;
 
 interface UseScreenReaderAnnouncerParams {
@@ -23,7 +23,8 @@ interface UseScreenReaderAnnouncerParams {
 }
 
 /**
- * Generates an accessible live region message for screen readers when progress bar updates cross defined thresholds.
+ * Generates an accessible live region message for screen readers
+ * when the value updates pass defined thresholds. See: {@link announcementThresholds}.
  */
 export const useScreenReaderAnnouncer = ({
   role,
@@ -33,10 +34,13 @@ export const useScreenReaderAnnouncer = ({
 }: UseScreenReaderAnnouncerParams): string | undefined => {
   const thresholdIndexRef = useRef(-1);
 
-  const message = useMemo(() => {
+  const [message, setMessage] = useState<string | undefined>();
+
+  useEffect(() => {
     // no live region messages for non-loader types or if value is undefined
     if (role === Role.Meter || !isDefined(value)) {
       thresholdIndexRef.current = -1;
+      setMessage(undefined);
       return;
     }
 
@@ -52,7 +56,7 @@ export const useScreenReaderAnnouncer = ({
     }
 
     if (newThresholdIndex === thresholdIndexRef.current) {
-      return;
+      return; // no threshold crossed, do not update message (but don't remove message)
     }
 
     // if new threshold was passed, update live region message
@@ -68,7 +72,7 @@ export const useScreenReaderAnnouncer = ({
         ? `${baseMessage} Status is ${variant}.`
         : baseMessage;
 
-    return newMessage;
+    setMessage(newMessage);
   }, [role, value, maxValue, variant]);
 
   return message;

packages/resizable/src/useResizable/useResizable.spec.tsx

@@ -1,7 +1,7 @@
+import { MutableRefObject } from 'react';
 import { fireEvent } from '@testing-library/dom';
-import { act } from '@testing-library/react';
 
-import { renderHook } from '@leafygreen-ui/testing-lib';
+import { act, renderHook } from '@leafygreen-ui/testing-lib';
 
 import { useResizable } from './useResizable';
 import { KEYBOARD_RESIZE_PIXEL_STEP } from './useResizable.constants';
@@ -11,16 +11,20 @@ import { Position } from './useResizable.types';
 Object.defineProperty(window, 'innerWidth', { value: 1024 });
 Object.defineProperty(window, 'innerHeight', { value: 768 });
 
+const mockDiv: HTMLDivElement = document.createElement('div');
+
 describe('useResizable', () => {
   const mockRef = {
     current: {
+      ...mockDiv,
       offsetWidth: 300,
       offsetHeight: 300,
       style: {
+        ...mockDiv.style,
         setProperty: jest.fn(),
         removeProperty: jest.fn(),
       },
-    },
+    } as HTMLDivElement,
   };
 
   beforeEach(() => {
@@ -73,17 +77,20 @@ describe('useResizable', () => {
     );
 
     // current is read-only from outside the hook but for testing we can set it directly
-    (result.current.resizableRef as any).current = mockRef.current;
+    const resultRef = result.current
+      .resizableRef as MutableRefObject<HTMLDivElement>;
+    resultRef.current = mockRef.current;
 
     // Start resizing
     const resizerProps = result.current.getResizerProps();
+
+    // Trigger a MouseDown event to initiate resizing
     act(() => {
-      // @ts-expect-error - onMouseDown expects all properties of MouseEvent
-      resizerProps?.onMouseDown({
-        preventDefault: jest.fn(),
+      const mouseDownEvent = new MouseEvent('mousedown', {
         clientX: 300,
         clientY: 300,
       });
+      resizerProps?.onMouseDown(mouseDownEvent);
     });
 
     // Simulate mouse movement