Skip to content

Commit 707016a

Browse files
bryphe-coderbryphe-coder
authored
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
1 parent 2e12cb9 commit 707016a

File tree

10 files changed

+53
-101
lines changed

10 files changed

+53
-101
lines changed

‎site/components/Form/FormCloseButton.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ export const FormCloseButton: React.FC<FormCloseButtonProps> = ({ onClose }) =>
2323
return () => {
2424
document.body.removeEventListener("keydown", handleKeyPress, false)
2525
}
26-
}, [])
26+
}, [onClose])
2727

2828
return (
2929
<IconButton className={styles.closeButton} onClick={onClose} size="medium">

‎site/components/Navbar/UserDropdown.tsx

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,9 @@ export const UserDropdown: React.FC<UserDropdownProps> = ({ user, onSignOut }: U
3535
<div>
3636
<MenuItem onClick={handleDropdownClick}>
3737
<div className={styles.inner}>
38-
{user && (
39-
<Badge overlap="circle">
40-
<UserAvatar user={user} />
41-
</Badge>
42-
)}
38+
<Badge overlap="circle">
39+
<UserAvatar user={user} />
40+
</Badge>
4341
{anchorEl ? (
4442
<KeyboardArrowUp className={`${styles.arrowIcon} ${styles.arrowIconUp}`} />
4543
) : (
@@ -65,20 +63,18 @@ export const UserDropdown: React.FC<UserDropdownProps> = ({ user, onSignOut }: U
6563
variant="user-dropdown"
6664
onClose={onPopoverClose}
6765
>
68-
{user && (
69-
<div className={styles.userInfo}>
70-
<UserProfileCard user={user} />
66+
<div className={styles.userInfo}>
67+
<UserProfileCard user={user} />
7168

72-
<Divider className={styles.divider} />
69+
<Divider className={styles.divider} />
7370

74-
<MenuItem className={styles.menuItem} onClick={onSignOut}>
75-
<ListItemIcon className={styles.icon}>
76-
<LogoutIcon />
77-
</ListItemIcon>
78-
<ListItemText primary="Sign Out" />
79-
</MenuItem>
80-
</div>
81-
)}
71+
<MenuItem className={styles.menuItem} onClick={onSignOut}>
72+
<ListItemIcon className={styles.icon}>
73+
<LogoutIcon />
74+
</ListItemIcon>
75+
<ListItemText primary="Sign Out" />
76+
</MenuItem>
77+
</div>
8278
</BorderedMenu>
8379
</>
8480
)

‎site/components/Redirect.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,11 @@ export interface RedirectProps {
1313
* Helper component to perform a client-side redirect
1414
*/
1515
export const Redirect: React.FC<RedirectProps> = ({ to }) => {
16-
const router = useRouter()
16+
const { replace } = useRouter()
1717

1818
useEffect(() => {
19-
void router.replace(to)
20-
}, [])
19+
void replace(to)
20+
}, [replace, to])
2121

2222
return null
2323
}

‎site/components/SignIn/SignInForm.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ export const SignInForm: React.FC<SignInProps> = ({
123123

124124
const getRedirectFromRouter = (router: NextRouter) => {
125125
const defaultRedirect = "/"
126-
if (router.query?.redirect) {
126+
if (router.query.redirect) {
127127
return firstOrItem(router.query.redirect, defaultRedirect)
128128
} else {
129129
return defaultRedirect

‎site/contexts/UserContext.tsx

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,21 +25,24 @@ const UserContext = React.createContext<UserContext>({
2525

2626
export const useUser = (redirectOnError = false): UserContext => {
2727
const ctx = useContext(UserContext)
28-
const router = useRouter()
28+
const { push, asPath } = useRouter()
2929

3030
const requestError = ctx.error
3131
useEffect(() => {
3232
if (redirectOnError && requestError) {
3333
// 'void' means we are ignoring handling the promise returned
3434
// from router.push (and lets the linter know we're OK with that!)
35-
void router.push({
35+
void push({
3636
pathname: "/login",
3737
query: {
38-
redirect: router.asPath,
38+
redirect: asPath,
3939
},
4040
})
4141
}
42-
}, [redirectOnError, requestError])
42+
// Disabling exhaustive deps here because it can cause an
43+
// infinite useEffect loop. Should (hopefully) go away
44+
// when we switch to an alternate routing strategy.
45+
}, [redirectOnError, requestError]) // eslint-disable-line react-hooks/exhaustive-deps
4346

4447
return ctx
4548
}

‎site/jest-runner.eslint.config.js renamed to ‎site/jest-runner-eslint.config.js

File renamed without changes.

‎site/jest.config.js

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,15 @@ module.exports = {
2424
displayName: "lint",
2525
runner: "jest-runner-eslint",
2626
testMatch: ["<rootDir>/**/*.js", "<rootDir>/**/*.ts", "<rootDir>/**/*.tsx"],
27-
testPathIgnorePatterns: ["/.next/", "/out/", "/_jest/", "jest.config.js", "jest-runner.*.js", "next.config.js"],
27+
testPathIgnorePatterns: [
28+
"/.next/",
29+
"/out/",
30+
"/_jest/",
31+
"dev.ts",
32+
"jest.config.js",
33+
"jest-runner.*.js",
34+
"next.config.js",
35+
],
2836
},
2937
],
3038
collectCoverageFrom: [

‎site/pages/projects/[organization]/[project]/create.tsx

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import React from "react"
1+
import React, { useCallback } from "react"
22
import { makeStyles } from "@material-ui/core/styles"
33
import { useRouter } from "next/router"
44
import useSWR from "swr"
@@ -10,14 +10,24 @@ import { FullScreenLoader } from "../../../../components/Loader/FullScreenLoader
1010
import { CreateWorkspaceForm } from "../../../../forms/CreateWorkspaceForm"
1111

1212
const CreateWorkspacePage: React.FC = () => {
13-
const router = useRouter()
13+
const { push, query } = useRouter()
1414
const styles = useStyles()
1515
const { me } = useUser(/* redirectOnError */ true)
16-
const { organization, project: projectName } = router.query
16+
const { organization, project: projectName } = query
1717
const { data: project, error: projectError } = useSWR<API.Project, Error>(
1818
`/api/v2/projects/${organization}/${projectName}`,
1919
)
2020

21+
const onCancel = useCallback(async () => {
22+
await push(`/projects/${organization}/${projectName}`)
23+
}, [push, organization, projectName])
24+
25+
const onSubmit = async (req: API.CreateWorkspaceRequest) => {
26+
const workspace = await API.Workspace.create(req)
27+
await push(`/workspaces/me/${workspace.name}`)
28+
return workspace
29+
}
30+
2131
if (projectError) {
2232
return <ErrorSummary error={projectError} />
2333
}
@@ -26,16 +36,6 @@ const CreateWorkspacePage: React.FC = () => {
2636
return <FullScreenLoader />
2737
}
2838

29-
const onCancel = async () => {
30-
await router.push(`/projects/${organization}/${projectName}`)
31-
}
32-
33-
const onSubmit = async (req: API.CreateWorkspaceRequest) => {
34-
const workspace = await API.Workspace.create(req)
35-
await router.push(`/workspaces/me/${workspace.name}`)
36-
return workspace
37-
}
38-
3939
return (
4040
<div className={styles.root}>
4141
<CreateWorkspaceForm onCancel={onCancel} onSubmit={onSubmit} project={project} />

‎site/pages/projects/create.tsx

Lines changed: 0 additions & 58 deletions
This file was deleted.

‎site/pages/workspaces/[user]/[workspace].tsx

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,12 @@ const WorkspacesPage: React.FC = () => {
2727
// So if the user is the same as 'me', use 'me' as the parameter
2828
const normalizedUserParam = me && userParam === me.id ? "me" : userParam
2929

30-
// The SWR API expects us to 'throw' if the query isn't ready yet, so these casts to `any` are OK
31-
// because the API expects exceptions.
32-
return `/api/v2/workspaces/${(normalizedUserParam as any).toString()}/${(workspaceParam as any).toString()}`
30+
// The SWR API expects us to 'throw' if the query isn't ready yet:
31+
if (normalizedUserParam === null || workspaceParam === null) {
32+
throw "Data not yet available to make API call"
33+
}
34+
35+
return `/api/v2/workspaces/${normalizedUserParam}/${workspaceParam}`
3336
})
3437

3538
if (workspaceError) {

0 commit comments

Comments
 (0)