refactor: Fix front-end lint issues (#347)

Noticed while running through the build steps (`make build`) that we were getting some lint warnings that weren't blocking build: ```sh ./pages/workspaces/[user]/[workspace].tsx 32:58 Warning: Unexpected any. Specify a different type. @typescript-eslint/no-explicit-any 32:96 Warning: Unexpected any. Specify a different type. @typescript-eslint/no-explicit-any ./components/Form/FormCloseButton.tsx 26:6 Warning: React Hook useEffect has a missing dependency: 'onClose'. Either include it or remove the dependency array. If 'onClose' changes too often, find the parent component that defines it and wrap that definition in useCallback. react-hooks/exhaustive-deps ./components/Navbar/UserDropdown.tsx 38:14 Warning: Unnecessary conditional, value is always truthy. @typescript-eslint/no-unnecessary-condition 68:10 Warning: Unnecessary conditional, value is always truthy. @typescript-eslint/no-unnecessary-condition ./components/Redirect.tsx 20:6 Warning: React Hook useEffect has missing dependencies: 'router' and 'to'. Either include them or remove the dependency array. react-hooks/exhaustive-deps ./components/SignIn/SignInForm.tsx 126:19 Warning: Unnecessary optional chain on a non-nullish value. @typescript-eslint/no-unnecessary-condition ``` It turns out our ESLint config wasn't being picked up, so I fixed that (it wasn't properly named). This PR turns warnings-as-errors on, fixes the issues, and also removes the "Project Create" page, because it isn't used at this time. Wanted to clean this up before on-boarding more FE developers
coder · Feb 23, 2022 · 707016a · 707016a
1 parent 2e12cb9
commit 707016a
Show file tree

Hide file tree

Showing 10 changed files with 53 additions and 101 deletions.
diff --git a/site/components/Form/FormCloseButton.tsx b/site/components/Form/FormCloseButton.tsx
@@ -23,7 +23,7 @@ export const FormCloseButton: React.FC<FormCloseButtonProps> = ({ onClose }) =>
     return () => {
       document.body.removeEventListener("keydown", handleKeyPress, false)
     }
-  }, [])
+  }, [onClose])
 
   return (
     <IconButton className={styles.closeButton} onClick={onClose} size="medium">

diff --git a/site/components/Navbar/UserDropdown.tsx b/site/components/Navbar/UserDropdown.tsx
@@ -35,11 +35,9 @@ export const UserDropdown: React.FC<UserDropdownProps> = ({ user, onSignOut }: U
       <div>
         <MenuItem onClick={handleDropdownClick}>
           <div className={styles.inner}>
-            {user && (
-              <Badge overlap="circle">
-                <UserAvatar user={user} />
-              </Badge>
-            )}
+            <Badge overlap="circle">
+              <UserAvatar user={user} />
+            </Badge>
             {anchorEl ? (
               <KeyboardArrowUp className={`${styles.arrowIcon} ${styles.arrowIconUp}`} />
             ) : (
@@ -65,20 +63,18 @@ export const UserDropdown: React.FC<UserDropdownProps> = ({ user, onSignOut }: U
         variant="user-dropdown"
         onClose={onPopoverClose}
       >
-        {user && (
-          <div className={styles.userInfo}>
-            <UserProfileCard user={user} />
+        <div className={styles.userInfo}>
+          <UserProfileCard user={user} />
 
-            <Divider className={styles.divider} />
+          <Divider className={styles.divider} />
 
-            <MenuItem className={styles.menuItem} onClick={onSignOut}>
-              <ListItemIcon className={styles.icon}>
-                <LogoutIcon />
-              </ListItemIcon>
-              <ListItemText primary="Sign Out" />
-            </MenuItem>
-          </div>
-        )}
+          <MenuItem className={styles.menuItem} onClick={onSignOut}>
+            <ListItemIcon className={styles.icon}>
+              <LogoutIcon />
+            </ListItemIcon>
+            <ListItemText primary="Sign Out" />
+          </MenuItem>
+        </div>
       </BorderedMenu>
     </>
   )

diff --git a/site/components/Redirect.tsx b/site/components/Redirect.tsx
@@ -13,11 +13,11 @@ export interface RedirectProps {
  * Helper component to perform a client-side redirect
  */
 export const Redirect: React.FC<RedirectProps> = ({ to }) => {
-  const router = useRouter()
+  const { replace } = useRouter()
 
   useEffect(() => {
-    void router.replace(to)
-  }, [])
+    void replace(to)
+  }, [replace, to])
 
   return null
 }
diff --git a/site/components/SignIn/SignInForm.tsx b/site/components/SignIn/SignInForm.tsx
@@ -123,7 +123,7 @@ export const SignInForm: React.FC<SignInProps> = ({
 
 const getRedirectFromRouter = (router: NextRouter) => {
   const defaultRedirect = "/"
-  if (router.query?.redirect) {
+  if (router.query.redirect) {
     return firstOrItem(router.query.redirect, defaultRedirect)
   } else {
     return defaultRedirect

diff --git a/site/contexts/UserContext.tsx b/site/contexts/UserContext.tsx
@@ -25,21 +25,24 @@ const UserContext = React.createContext<UserContext>({
 
 export const useUser = (redirectOnError = false): UserContext => {
   const ctx = useContext(UserContext)
-  const router = useRouter()
+  const { push, asPath } = useRouter()
 
   const requestError = ctx.error
   useEffect(() => {
     if (redirectOnError && requestError) {
       // 'void' means we are ignoring handling the promise returned
       // from router.push (and lets the linter know we're OK with that!)
-      void router.push({
+      void push({
         pathname: "/login",
         query: {
-          redirect: router.asPath,
+          redirect: asPath,
         },
       })
     }
-  }, [redirectOnError, requestError])
+    // Disabling exhaustive deps here because it can cause an
+    // infinite useEffect loop. Should (hopefully) go away
+    // when we switch to an alternate routing strategy.
+  }, [redirectOnError, requestError]) // eslint-disable-line react-hooks/exhaustive-deps
 
   return ctx
 }

diff --git a/site/jest-runner.eslint.config.js → site/jest-runner-eslint.config.js b/site/jest-runner.eslint.config.js → site/jest-runner-eslint.config.js
diff --git a/site/jest.config.js b/site/jest.config.js
@@ -24,7 +24,15 @@ module.exports = {
       displayName: "lint",
       runner: "jest-runner-eslint",
       testMatch: ["<rootDir>/**/*.js", "<rootDir>/**/*.ts", "<rootDir>/**/*.tsx"],
-      testPathIgnorePatterns: ["/.next/", "/out/", "/_jest/", "jest.config.js", "jest-runner.*.js", "next.config.js"],
+      testPathIgnorePatterns: [
+        "/.next/",
+        "/out/",
+        "/_jest/",
+        "dev.ts",
+        "jest.config.js",
+        "jest-runner.*.js",
+        "next.config.js",
+      ],
     },
   ],
   collectCoverageFrom: [

diff --git a/site/pages/projects/[organization]/[project]/create.tsx b/site/pages/projects/[organization]/[project]/create.tsx
@@ -1,4 +1,4 @@
-import React from "react"
+import React, { useCallback } from "react"
 import { makeStyles } from "@material-ui/core/styles"
 import { useRouter } from "next/router"
 import useSWR from "swr"
@@ -10,14 +10,24 @@ import { FullScreenLoader } from "../../../../components/Loader/FullScreenLoader
 import { CreateWorkspaceForm } from "../../../../forms/CreateWorkspaceForm"
 
 const CreateWorkspacePage: React.FC = () => {
-  const router = useRouter()
+  const { push, query } = useRouter()
   const styles = useStyles()
   const { me } = useUser(/* redirectOnError */ true)
-  const { organization, project: projectName } = router.query
+  const { organization, project: projectName } = query
   const { data: project, error: projectError } = useSWR<API.Project, Error>(
     `/api/v2/projects/${organization}/${projectName}`,
   )
 
+  const onCancel = useCallback(async () => {
+    await push(`/projects/${organization}/${projectName}`)
+  }, [push, organization, projectName])
+
+  const onSubmit = async (req: API.CreateWorkspaceRequest) => {
+    const workspace = await API.Workspace.create(req)
+    await push(`/workspaces/me/${workspace.name}`)
+    return workspace
+  }
+
   if (projectError) {
     return <ErrorSummary error={projectError} />
   }
@@ -26,16 +36,6 @@ const CreateWorkspacePage: React.FC = () => {
     return <FullScreenLoader />
   }
 
-  const onCancel = async () => {
-    await router.push(`/projects/${organization}/${projectName}`)
-  }
-
-  const onSubmit = async (req: API.CreateWorkspaceRequest) => {
-    const workspace = await API.Workspace.create(req)
-    await router.push(`/workspaces/me/${workspace.name}`)
-    return workspace
-  }
-
   return (
     <div className={styles.root}>
       <CreateWorkspaceForm onCancel={onCancel} onSubmit={onSubmit} project={project} />

diff --git a/site/pages/projects/create.tsx b/site/pages/projects/create.tsx
diff --git a/site/pages/workspaces/[user]/[workspace].tsx b/site/pages/workspaces/[user]/[workspace].tsx
@@ -27,9 +27,12 @@ const WorkspacesPage: React.FC = () => {
     // So if the user is the same as 'me', use 'me' as the parameter
     const normalizedUserParam = me && userParam === me.id ? "me" : userParam
 
-    // The SWR API expects us to 'throw' if the query isn't ready yet, so these casts to `any` are OK
-    // because the API expects exceptions.
-    return `/api/v2/workspaces/${(normalizedUserParam as any).toString()}/${(workspaceParam as any).toString()}`
+    // The SWR API expects us to 'throw' if the query isn't ready yet:
+    if (normalizedUserParam === null || workspaceParam === null) {
+      throw "Data not yet available to make API call"
+    }
+
+    return `/api/v2/workspaces/${normalizedUserParam}/${workspaceParam}`
   })
 
   if (workspaceError) {