From fbc47846e3e1a09116bd2ace2edc9c071064377f Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandrocelaya@gmail.com>
Date: Sat, 2 Sep 2023 19:08:12 +0200
Subject: [PATCH] Fix some instances of react-hooks/exhaustive-deps

---
 package-lock.json                  | 14 +++++++-------
 package.json                       |  2 +-
 src/common/Home.tsx                |  2 +-
 src/common/MainHeader.tsx          | 11 ++++++-----
 src/servers/CreateServer.tsx       | 26 +++++++++++++-------------
 test/servers/CreateServer.test.tsx | 24 +++++++++++++++++++-----
 6 files changed, 47 insertions(+), 32 deletions(-)

diff --git a/package-lock.json b/package-lock.json
index 3a2adf85..40082368 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -15,7 +15,7 @@
         "@fortawesome/react-fontawesome": "^0.2.0",
         "@json2csv/plainjs": "^7.0.3",
         "@reduxjs/toolkit": "^1.9.5",
-        "@shlinkio/shlink-frontend-kit": "^0.2.0",
+        "@shlinkio/shlink-frontend-kit": "^0.2.1",
         "@shlinkio/shlink-js-sdk": "^0.1.0",
         "@shlinkio/shlink-web-component": "^0.3.3",
         "bootstrap": "5.2.3",
@@ -2801,9 +2801,9 @@
       }
     },
     "node_modules/@shlinkio/shlink-frontend-kit": {
-      "version": "0.2.0",
-      "resolved": "https://registry.npmjs.org/@shlinkio/shlink-frontend-kit/-/shlink-frontend-kit-0.2.0.tgz",
-      "integrity": "sha512-8kGaae0bTiGzbLzPsolLvJ5ud37BR2b1WeDy8lyXIiwoFiSAMIgWpqro0nMdBVBQXovjmMbtiS6BFYsaoBo9/g==",
+      "version": "0.2.1",
+      "resolved": "https://registry.npmjs.org/@shlinkio/shlink-frontend-kit/-/shlink-frontend-kit-0.2.1.tgz",
+      "integrity": "sha512-5mRpQII9bGHAJQ1ghgGY+jFC5tD0y0ufgPqco9vLVGXGVf+BSNQrTiw/Cx6f9eCInqFPgFo8CdJzWUHbIzvC+Q==",
       "dependencies": {
         "classnames": "^2.3.2",
         "qs": "^6.11.2",
@@ -12390,9 +12390,9 @@
       }
     },
     "@shlinkio/shlink-frontend-kit": {
-      "version": "0.2.0",
-      "resolved": "https://registry.npmjs.org/@shlinkio/shlink-frontend-kit/-/shlink-frontend-kit-0.2.0.tgz",
-      "integrity": "sha512-8kGaae0bTiGzbLzPsolLvJ5ud37BR2b1WeDy8lyXIiwoFiSAMIgWpqro0nMdBVBQXovjmMbtiS6BFYsaoBo9/g==",
+      "version": "0.2.1",
+      "resolved": "https://registry.npmjs.org/@shlinkio/shlink-frontend-kit/-/shlink-frontend-kit-0.2.1.tgz",
+      "integrity": "sha512-5mRpQII9bGHAJQ1ghgGY+jFC5tD0y0ufgPqco9vLVGXGVf+BSNQrTiw/Cx6f9eCInqFPgFo8CdJzWUHbIzvC+Q==",
       "requires": {
         "classnames": "^2.3.2",
         "qs": "^6.11.2",
diff --git a/package.json b/package.json
index 0c79b7a6..a064e41e 100644
--- a/package.json
+++ b/package.json
@@ -31,7 +31,7 @@
     "@fortawesome/react-fontawesome": "^0.2.0",
     "@json2csv/plainjs": "^7.0.3",
     "@reduxjs/toolkit": "^1.9.5",
-    "@shlinkio/shlink-frontend-kit": "^0.2.0",
+    "@shlinkio/shlink-frontend-kit": "^0.2.1",
     "@shlinkio/shlink-js-sdk": "^0.1.0",
     "@shlinkio/shlink-web-component": "^0.3.3",
     "bootstrap": "5.2.3",
diff --git a/src/common/Home.tsx b/src/common/Home.tsx
index 4029e39b..bf7b79bd 100644
--- a/src/common/Home.tsx
+++ b/src/common/Home.tsx
@@ -23,7 +23,7 @@ export const Home = ({ servers }: HomeProps) => {
     // Try to redirect to the first server marked as auto-connect
     const autoConnectServer = serversList.find(({ autoConnect }) => autoConnect);
     autoConnectServer && navigate(`/server/${autoConnectServer.id}`);
-  }, []);
+  }, [serversList, navigate]);
 
   return (
     <div className="home">
diff --git a/src/common/MainHeader.tsx b/src/common/MainHeader.tsx
index 74cd028a..750ba744 100644
--- a/src/common/MainHeader.tsx
+++ b/src/common/MainHeader.tsx
@@ -10,14 +10,15 @@ import { ShlinkLogo } from './img/ShlinkLogo';
 import './MainHeader.scss';
 
 export const MainHeader = (ServersDropdown: FC) => () => {
-  const [isOpen, toggleOpen, , close] = useToggle();
+  const [isNotCollapsed, toggleCollapse, , collapse] = useToggle();
   const location = useLocation();
   const { pathname } = location;
 
-  useEffect(close, [location]);
+  // In mobile devices, collapse the navbar when location changes
+  useEffect(collapse, [location, collapse]);
 
   const settingsPath = '/settings';
-  const toggleClass = classNames('main-header__toggle-icon', { 'main-header__toggle-icon--opened': isOpen });
+  const toggleClass = classNames('main-header__toggle-icon', { 'main-header__toggle-icon--opened': isNotCollapsed });
 
   return (
     <Navbar color="primary" dark fixed="top" className="main-header" expand="md">
@@ -25,11 +26,11 @@ export const MainHeader = (ServersDropdown: FC) => () => {
         <ShlinkLogo className="main-header__brand-logo" color="white" /> Shlink
       </NavbarBrand>
 
-      <NavbarToggler onClick={toggleOpen}>
+      <NavbarToggler onClick={toggleCollapse}>
         <FontAwesomeIcon icon={arrowIcon} className={toggleClass} />
       </NavbarToggler>
 
-      <Collapse navbar isOpen={isOpen}>
+      <Collapse navbar isOpen={isNotCollapsed}>
         <Nav navbar className="ms-auto">
           <NavItem>
             <NavLink tag={Link} to={settingsPath} active={pathname.startsWith(settingsPath)}>
diff --git a/src/servers/CreateServer.tsx b/src/servers/CreateServer.tsx
index 7587e53a..428672e9 100644
--- a/src/servers/CreateServer.tsx
+++ b/src/servers/CreateServer.tsx
@@ -1,6 +1,6 @@
 import { Result, useToggle } from '@shlinkio/shlink-frontend-kit';
 import type { FC } from 'react';
-import { useEffect, useState } from 'react';
+import { useCallback, useEffect, useState } from 'react';
 import { useNavigate } from 'react-router-dom';
 import { Button } from 'reactstrap';
 import { v4 as uuid } from 'uuid';
@@ -37,25 +37,25 @@ export const CreateServer = (ImportServersBtn: FC<ImportServersBtnProps>, useTim
   const [serversImported, setServersImported] = useTimeoutToggle(false, SHOW_IMPORT_MSG_TIME);
   const [errorImporting, setErrorImporting] = useTimeoutToggle(false, SHOW_IMPORT_MSG_TIME);
   const [isConfirmModalOpen, toggleConfirmModal] = useToggle();
-  const [serverData, setServerData] = useState<ServerData | undefined>();
-  const save = () => {
+  const [serverData, setServerData] = useState<ServerData>();
+  const saveNewServer = useCallback((theServerData: ServerData) => {
+    const id = uuid();
+
+    createServers([{ ...theServerData, id }]);
+    navigate(`/server/${id}`);
+  }, [createServers, navigate]);
+
+  useEffect(() => {
     if (!serverData) {
       return;
     }
 
-    const id = uuid();
-
-    createServers([{ ...serverData, id }]);
-    navigate(`/server/${id}`);
-  };
-
-  useEffect(() => {
     const serverExists = Object.values(servers).some(
       ({ url, apiKey }) => serverData?.url === url && serverData?.apiKey === apiKey,
     );
 
-    serverExists ? toggleConfirmModal() : save();
-  }, [serverData]);
+    serverExists ? toggleConfirmModal() : saveNewServer(serverData);
+  }, [saveNewServer, serverData, servers, toggleConfirmModal]);
 
   return (
     <NoMenuLayout>
@@ -74,7 +74,7 @@ export const CreateServer = (ImportServersBtn: FC<ImportServersBtnProps>, useTim
         isOpen={isConfirmModalOpen}
         duplicatedServers={serverData ? [serverData] : []}
         onDiscard={goBack}
-        onSave={save}
+        onSave={() => serverData && saveNewServer(serverData)}
       />
     </NoMenuLayout>
   );
diff --git a/test/servers/CreateServer.test.tsx b/test/servers/CreateServer.test.tsx
index 27a54a20..0d0f5e21 100644
--- a/test/servers/CreateServer.test.tsx
+++ b/test/servers/CreateServer.test.tsx
@@ -2,7 +2,7 @@ import { fireEvent, screen, waitFor } from '@testing-library/react';
 import { fromPartial } from '@total-typescript/shoehorn';
 import { useNavigate } from 'react-router-dom';
 import { CreateServer as createCreateServer } from '../../src/servers/CreateServer';
-import type { ServerWithId } from '../../src/servers/data';
+import type { ServersMap } from '../../src/servers/data';
 import { renderWithEvents } from '../__helpers__/setUpTest';
 
 vi.mock('react-router-dom', async () => ({
@@ -10,11 +10,19 @@ vi.mock('react-router-dom', async () => ({
   useNavigate: vi.fn(),
 }));
 
+type SetUpOptions = {
+  serversImported?: boolean;
+  importFailed?: boolean;
+  servers?: ServersMap;
+};
+
 describe('<CreateServer />', () => {
   const createServersMock = vi.fn();
   const navigate = vi.fn();
-  const servers = { foo: fromPartial<ServerWithId>({ url: 'https://existing_url.com', apiKey: 'existing_api_key' }) };
-  const setUp = (serversImported = false, importFailed = false) => {
+  const defaultServers: ServersMap = {
+    foo: fromPartial({ url: 'https://existing_url.com', apiKey: 'existing_api_key' }),
+  };
+  const setUp = ({ serversImported = false, importFailed = false, servers = defaultServers }: SetUpOptions = {}) => {
     (useNavigate as any).mockReturnValue(navigate);
 
     let callCount = 0;
@@ -29,16 +37,17 @@ describe('<CreateServer />', () => {
   };
 
   it('shows success message when imported is true', () => {
-    setUp(true);
+    setUp({ serversImported: true });
 
     expect(screen.getByText('Servers properly imported. You can now select one from the list :)')).toBeInTheDocument();
     expect(
       screen.queryByText('The servers could not be imported. Make sure the format is correct.'),
     ).not.toBeInTheDocument();
+    expect(screen.queryByText('ImportServersBtn')).not.toBeInTheDocument();
   });
 
   it('shows error message when import failed', () => {
-    setUp(false, true);
+    setUp({ importFailed: true });
 
     expect(
       screen.queryByText('Servers properly imported. You can now select one from the list :)'),
@@ -46,6 +55,11 @@ describe('<CreateServer />', () => {
     expect(screen.getByText('The servers could not be imported. Make sure the format is correct.')).toBeInTheDocument();
   });
 
+  it('shows import button when no servers exist yet', () => {
+    setUp({ servers: {} });
+    expect(screen.queryByText('ImportServersBtn')).toBeInTheDocument();
+  });
+
   it('creates server data when form is submitted', async () => {
     const { user } = setUp();