Merge pull request #1014 from acelaya-forks/feature/ref-warning-fix

Fix warning in AppUpdateBanner

src/common/AppUpdateBanner.tsx

@@ -1,7 +1,8 @@
 import { faSyncAlt as reloadIcon } from '@fortawesome/free-solid-svg-icons';
 import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
 import { SimpleCard, useToggle } from '@shlinkio/shlink-frontend-kit';
-import type { FC, MouseEventHandler } from 'react';
+import type { MouseEventHandler } from 'react';
+import { forwardRef, useCallback } from 'react';
 import { Alert, Button } from 'reactstrap';
 import './AppUpdateBanner.scss';
 
@@ -11,15 +12,15 @@ interface AppUpdateBannerProps {
   forceUpdate: Function;
 }
 
-export const AppUpdateBanner: FC<AppUpdateBannerProps> = ({ isOpen, toggle, forceUpdate }) => {
+export const AppUpdateBanner = forwardRef<HTMLElement, AppUpdateBannerProps>(({ isOpen, toggle, forceUpdate }, ref) => {
   const [isUpdating,, setUpdating] = useToggle();
-  const update = () => {
+  const update = useCallback(() => {
     setUpdating();
     forceUpdate();
-  };
+  }, [forceUpdate, setUpdating]);
 
   return (
-    <Alert className="app-update-banner" isOpen={isOpen} toggle={toggle} tag={SimpleCard} color="secondary">
+    <Alert className="app-update-banner" isOpen={isOpen} toggle={toggle} tag={SimpleCard} color="secondary" innerRef={ref}>
       <h4 className="mb-4">This app has just been updated!</h4>
       <p className="mb-0">
         Restart it to enjoy the new features.
@@ -30,4 +31,4 @@ export const AppUpdateBanner: FC<AppUpdateBannerProps> = ({ isOpen, toggle, forc
       </p>
     </Alert>
   );
-};
+});

test/app/App.test.tsx

@@ -1,4 +1,4 @@
-import { render, screen } from '@testing-library/react';
+import { act, render, screen } from '@testing-library/react';
 import { fromPartial } from '@total-typescript/shoehorn';
 import { MemoryRouter } from 'react-router-dom';
 import { AppFactory } from '../../src/app/App';
@@ -17,26 +17,25 @@ describe('<App />', () => {
       ShlinkVersionsContainer: () => <>ShlinkVersions</>,
     }),
   );
-  const setUp = (activeRoute = '/') => render(
+  const setUp = async (activeRoute = '/') => act(() => render(
     <MemoryRouter initialEntries={[{ pathname: activeRoute }]}>
       <App
         fetchServers={() => {}}
         servers={{}}
         settings={fromPartial({})}
-        appUpdated
+        appUpdated={false}
         resetAppUpdate={() => {}}
       />
     </MemoryRouter>,
-  );
+  ));
 
   it('passes a11y checks', () => checkAccessibility(setUp()));
 
-  it('renders children components', () => {
-    setUp();
+  it('renders children components', async () => {
+    await setUp();
 
     expect(screen.getByText('MainHeader')).toBeInTheDocument();
     expect(screen.getByText('ShlinkVersions')).toBeInTheDocument();
-    expect(screen.getByText('This app has just been updated!')).toBeInTheDocument();
   });
 
   it.each([
@@ -50,16 +49,16 @@ describe('<App />', () => {
     ['/server/def456/bar', 'ShlinkWebComponentContainer'],
     ['/other', 'Oops! We could not find requested route.'],
   ])('renders expected route', async (activeRoute, expectedComponent) => {
-    setUp(activeRoute);
-    expect(await screen.findByText(expectedComponent)).toBeInTheDocument();
+    await setUp(activeRoute);
+    expect(screen.getByText(expectedComponent)).toBeInTheDocument();
   });
 
   it.each([
     ['/foo', 'shlink-wrapper'],
     ['/bar', 'shlink-wrapper'],
     ['/', 'shlink-wrapper d-flex d-md-block align-items-center'],
-  ])('renders expected classes on shlink-wrapper based on current pathname', (pathname, expectedClasses) => {
-    const { container } = setUp(pathname);
+  ])('renders expected classes on shlink-wrapper based on current pathname', async (pathname, expectedClasses) => {
+    const { container } = await setUp(pathname);
     const shlinkWrapper = container.querySelector('.shlink-wrapper');
 
     expect(shlinkWrapper).toHaveAttribute('class', expectedClasses);

test/common/AppUpdateBanner.test.tsx

@@ -1,4 +1,4 @@
-import { screen } from '@testing-library/react';
+import { act, screen, waitFor } from '@testing-library/react';
 import { AppUpdateBanner } from '../../src/common/AppUpdateBanner';
 import { checkAccessibility } from '../__helpers__/accessibility';
 import { renderWithEvents } from '../__helpers__/setUpTest';
@@ -6,12 +6,19 @@ import { renderWithEvents } from '../__helpers__/setUpTest';
 describe('<AppUpdateBanner />', () => {
   const toggle = vi.fn();
   const forceUpdate = vi.fn();
-  const setUp = () => renderWithEvents(<AppUpdateBanner isOpen toggle={toggle} forceUpdate={forceUpdate} />);
+  const setUp = async () => {
+    const result = await act(
+      () => renderWithEvents(<AppUpdateBanner isOpen toggle={toggle} forceUpdate={forceUpdate} />),
+    );
+    await waitFor(() => screen.getByRole('alert'));
+
+    return result;
+  };
 
   it('passes a11y checks', () => checkAccessibility(setUp()));
 
-  it('renders initial state', () => {
-    setUp();
+  it('renders initial state', async () => {
+    await setUp();
 
     expect(screen.getByRole('heading')).toHaveTextContent('This app has just been updated!');
     expect(screen.queryByText('Restarting...')).not.toBeInTheDocument();
@@ -19,7 +26,7 @@ describe('<AppUpdateBanner />', () => {
   });
 
   it('invokes toggle when alert is closed', async () => {
-    const { user } = setUp();
+    const { user } = await setUp();
 
     expect(toggle).not.toHaveBeenCalled();
     await user.click(screen.getByLabelText('Close'));
@@ -27,7 +34,7 @@ describe('<AppUpdateBanner />', () => {
   });
 
   it('triggers the update when clicking the button', async () => {
-    const { user } = setUp();
+    const { user } = await setUp();
 
     expect(forceUpdate).not.toHaveBeenCalled();
     await user.click(screen.getByText(/^Restart now/));

test/servers/DeleteServerModal.test.tsx

@@ -1,4 +1,4 @@
-import { screen, waitFor } from '@testing-library/react';
+import { act, screen, waitFor } from '@testing-library/react';
 import { fromPartial } from '@total-typescript/shoehorn';
 import { createMemoryHistory } from 'history';
 import { Router } from 'react-router-dom';
@@ -10,11 +10,9 @@ import { TestModalWrapper } from '../__helpers__/TestModalWrapper';
 describe('<DeleteServerModal />', () => {
   const deleteServerMock = vi.fn();
   const serverName = 'the_server_name';
-  const setUp = () => {
+  const setUp = async () => {
     const history = createMemoryHistory({ initialEntries: ['/foo'] });
-    return {
-      history,
-      ...renderWithEvents(
+    const result = await act(() => renderWithEvents(
       <Router location={history.location} navigator={history}>
         <TestModalWrapper
           renderModal={(args) => (
@@ -26,21 +24,22 @@ describe('<DeleteServerModal />', () => {
           )}
         />
       </Router>,
-      ),
-    };
+    ));
+
+    return { history, ...result };
   };
 
   it('passes a11y checks', () => checkAccessibility(setUp()));
 
-  it('renders a modal window', () => {
-    setUp();
+  it('renders a modal window', async () => {
+    await setUp();
 
     expect(screen.getByRole('dialog')).toBeInTheDocument();
     expect(screen.getByRole('heading')).toHaveTextContent('Remove server');
   });
 
-  it('displays the name of the server as part of the content', () => {
-    setUp();
+  it('displays the name of the server as part of the content', async () => {
+    await setUp();
 
     expect(screen.getByText(/^Are you sure you want to remove/)).toBeInTheDocument();
     expect(screen.getByText(serverName)).toBeInTheDocument();
@@ -50,7 +49,7 @@ describe('<DeleteServerModal />', () => {
     [() => screen.getByRole('button', { name: 'Cancel' })],
     [() => screen.getByLabelText('Close')],
   ])('toggles when clicking cancel button', async (getButton) => {
-    const { user, history } = setUp();
+    const { user, history } = await setUp();
 
     expect(history.location.pathname).toEqual('/foo');
     await user.click(getButton());
@@ -60,7 +59,7 @@ describe('<DeleteServerModal />', () => {
   });
 
   it('deletes server when clicking accept button', async () => {
-    const { user, history } = setUp();
+    const { user, history } = await setUp();
 
     expect(deleteServerMock).not.toHaveBeenCalled();
     expect(history.location.pathname).toEqual('/foo');

test/servers/helpers/DuplicatedServersModal.test.tsx

@@ -1,4 +1,4 @@
-import { screen } from '@testing-library/react';
+import { act, screen } from '@testing-library/react';
 import { fromPartial } from '@total-typescript/shoehorn';
 import type { ServerData } from '../../../src/servers/data';
 import { DuplicatedServersModal } from '../../../src/servers/helpers/DuplicatedServersModal';
@@ -8,9 +8,9 @@ import { renderWithEvents } from '../../__helpers__/setUpTest';
 describe('<DuplicatedServersModal />', () => {
   const onDiscard = vi.fn();
   const onSave = vi.fn();
-  const setUp = (duplicatedServers: ServerData[] = []) => renderWithEvents(
+  const setUp = (duplicatedServers: ServerData[] = []) => act(() => renderWithEvents(
     <DuplicatedServersModal isOpen duplicatedServers={duplicatedServers} onDiscard={onDiscard} onSave={onSave} />,
-  );
+  ));
   const mockServer = (data: Partial<ServerData> = {}) => fromPartial<ServerData>(data);
 
   it('passes a11y checks', () => checkAccessibility(setUp()));
@@ -21,8 +21,8 @@ describe('<DuplicatedServersModal />', () => {
     [[mockServer(), mockServer()], 2],
     [[mockServer(), mockServer(), mockServer()], 3],
     [[mockServer(), mockServer(), mockServer(), mockServer()], 4],
-  ])('renders expected amount of items', (duplicatedServers, expectedItems) => {
-    setUp(duplicatedServers);
+  ])('renders expected amount of items', async (duplicatedServers, expectedItems) => {
+    await setUp(duplicatedServers);
     expect(screen.queryAllByRole('listitem')).toHaveLength(expectedItems);
   });
 
@@ -45,8 +45,8 @@ describe('<DuplicatedServersModal />', () => {
         discardBtn: 'Ignore duplicates',
       },
     ],
-  ])('renders expected texts based on amount of servers', (duplicatedServers, assertions) => {
-    setUp(duplicatedServers);
+  ])('renders expected texts based on amount of servers', async (duplicatedServers, assertions) => {
+    await setUp(duplicatedServers);
 
     expect(screen.getByRole('heading')).toHaveTextContent(assertions.header);
     expect(screen.getByText(assertions.firstParagraph)).toBeInTheDocument();
@@ -61,8 +61,8 @@ describe('<DuplicatedServersModal />', () => {
       mockServer({ url: 'url_1', apiKey: 'apiKey_1' }),
       mockServer({ url: 'url_2', apiKey: 'apiKey_2' }),
     ]],
-  ])('displays provided server data', (duplicatedServers) => {
-    setUp(duplicatedServers);
+  ])('displays provided server data', async (duplicatedServers) => {
+    await setUp(duplicatedServers);
 
     if (duplicatedServers.length === 0) {
       expect(screen.queryByRole('listitem')).not.toBeInTheDocument();
@@ -81,7 +81,7 @@ describe('<DuplicatedServersModal />', () => {
   });
 
   it('invokes onDiscard when appropriate button is clicked', async () => {
-    const { user } = setUp();
+    const { user } = await setUp();
 
     expect(onDiscard).not.toHaveBeenCalled();
     await user.click(screen.getByRole('button', { name: 'Discard' }));
@@ -89,7 +89,7 @@ describe('<DuplicatedServersModal />', () => {
   });
 
   it('invokes onSave when appropriate button is clicked', async () => {
-    const { user } = setUp();
+    const { user } = await setUp();
 
     expect(onSave).not.toHaveBeenCalled();
     await user.click(screen.getByRole('button', { name: 'Save anyway' }));