From 56e7086397a7cc59b4f0acb201101a002e646bec Mon Sep 17 00:00:00 2001 From: Bryan Phelps Date: Mon, 21 Feb 2022 23:11:58 +0000 Subject: [PATCH 1/6] Rename jest-runner config so it is properly picked up --- ...{jest-runner.eslint.config.js => jest-runner-eslint.config.js} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename site/{jest-runner.eslint.config.js => jest-runner-eslint.config.js} (100%) diff --git a/site/jest-runner.eslint.config.js b/site/jest-runner-eslint.config.js similarity index 100% rename from site/jest-runner.eslint.config.js rename to site/jest-runner-eslint.config.js From ff68093e0f1f5032715cc67e3b161ab5ec5557b7 Mon Sep 17 00:00:00 2001 From: Bryan Phelps Date: Mon, 21 Feb 2022 23:12:18 +0000 Subject: [PATCH 2/6] Pick up automatic fixes --- site/components/SignIn/SignInForm.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/components/SignIn/SignInForm.tsx b/site/components/SignIn/SignInForm.tsx index fdb0e0c43ae3f..3216e53ba4799 100644 --- a/site/components/SignIn/SignInForm.tsx +++ b/site/components/SignIn/SignInForm.tsx @@ -123,7 +123,7 @@ export const SignInForm: React.FC = ({ const getRedirectFromRouter = (router: NextRouter) => { const defaultRedirect = "/" - if (router.query?.redirect) { + if (router.query.redirect) { return firstOrItem(router.query.redirect, defaultRedirect) } else { return defaultRedirect From e8baef367a140b83e76d9eeb4f9503822ad6d731 Mon Sep 17 00:00:00 2001 From: Bryan Phelps Date: Mon, 21 Feb 2022 23:13:48 +0000 Subject: [PATCH 3/6] Fix unnecessary checks in UserDropdown --- site/components/Navbar/UserDropdown.tsx | 30 +++++++++++-------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/site/components/Navbar/UserDropdown.tsx b/site/components/Navbar/UserDropdown.tsx index be002c4741f15..164d5822651df 100644 --- a/site/components/Navbar/UserDropdown.tsx +++ b/site/components/Navbar/UserDropdown.tsx @@ -35,11 +35,9 @@ export const UserDropdown: React.FC = ({ user, onSignOut }: U
- {user && ( - - - - )} + + + {anchorEl ? ( ) : ( @@ -65,20 +63,18 @@ export const UserDropdown: React.FC = ({ user, onSignOut }: U variant="user-dropdown" onClose={onPopoverClose} > - {user && ( -
- +
+ - + - - - - - - -
- )} + + + + + + +
) From b37f4857b745bbd5258c6debd742f58bfdf0248d Mon Sep 17 00:00:00 2001 From: Bryan Phelps Date: Mon, 21 Feb 2022 23:15:40 +0000 Subject: [PATCH 4/6] Fix missed useEffect dependencies in Redirect --- site/components/Redirect.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/site/components/Redirect.tsx b/site/components/Redirect.tsx index 5d9e01258cd74..1429421d2ad7f 100644 --- 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 = ({ to }) => { - const router = useRouter() + const { replace } = useRouter() useEffect(() => { - void router.replace(to) - }, []) + void replace(to) + }, [replace, to]) return null } From 1a490dfb33ef557dd74be9687e1cf765ec1a1c93 Mon Sep 17 00:00:00 2001 From: Bryan Phelps Date: Mon, 21 Feb 2022 23:33:34 +0000 Subject: [PATCH 5/6] Fix remaining lint issues --- site/components/Form/FormCloseButton.tsx | 2 +- site/contexts/UserContext.tsx | 8 +-- site/jest.config.js | 10 +++- .../[organization]/[project]/create.tsx | 26 ++++----- site/pages/projects/create.tsx | 58 ------------------- site/pages/workspaces/[user]/[workspace].tsx | 9 ++- 6 files changed, 33 insertions(+), 80 deletions(-) delete mode 100644 site/pages/projects/create.tsx diff --git a/site/components/Form/FormCloseButton.tsx b/site/components/Form/FormCloseButton.tsx index a22f71e8c5b7d..9a04a3171d468 100644 --- a/site/components/Form/FormCloseButton.tsx +++ b/site/components/Form/FormCloseButton.tsx @@ -23,7 +23,7 @@ export const FormCloseButton: React.FC = ({ onClose }) => return () => { document.body.removeEventListener("keydown", handleKeyPress, false) } - }, []) + }, [onClose]) return ( diff --git a/site/contexts/UserContext.tsx b/site/contexts/UserContext.tsx index 673c767d1e771..7fa2a44209fbb 100644 --- a/site/contexts/UserContext.tsx +++ b/site/contexts/UserContext.tsx @@ -25,21 +25,21 @@ const UserContext = React.createContext({ 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]) + }, [asPath, push, redirectOnError, requestError]) return ctx } diff --git a/site/jest.config.js b/site/jest.config.js index 0d58cf7396528..6db0792abc814 100644 --- a/site/jest.config.js +++ b/site/jest.config.js @@ -24,7 +24,15 @@ module.exports = { displayName: "lint", runner: "jest-runner-eslint", testMatch: ["/**/*.js", "/**/*.ts", "/**/*.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 index f93876bfd9e29..8f78206801055 100644 --- 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/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 } @@ -26,16 +36,6 @@ const CreateWorkspacePage: React.FC = () => { return } - 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 (
diff --git a/site/pages/projects/create.tsx b/site/pages/projects/create.tsx deleted file mode 100644 index ac49d29b6cecc..0000000000000 --- a/site/pages/projects/create.tsx +++ /dev/null @@ -1,58 +0,0 @@ -import React from "react" -import { makeStyles } from "@material-ui/core/styles" -import { useRouter } from "next/router" -import useSWR from "swr" - -import * as API from "../../api" -import { useUser } from "../../contexts/UserContext" -import { ErrorSummary } from "../../components/ErrorSummary" -import { FullScreenLoader } from "../../components/Loader/FullScreenLoader" -import { CreateProjectForm } from "../../forms/CreateProjectForm" - -const CreateProjectPage: React.FC = () => { - const router = useRouter() - const styles = useStyles() - const { me } = useUser(true) - const { data: organizations, error } = useSWR("/api/v2/users/me/organizations") - - if (error) { - return - } - - if (!me || !organizations) { - return - } - - const onCancel = async () => { - await router.push("/projects") - } - - const onSubmit = async (req: API.CreateProjectRequest) => { - const project = await API.Project.create(req) - await router.push(`/projects/${req.organizationId}/${project.name}`) - return project - } - - return ( -
- -
- ) -} - -const useStyles = makeStyles((theme) => ({ - root: { - display: "flex", - flexDirection: "column", - alignItems: "center", - height: "100vh", - backgroundColor: theme.palette.background.paper, - }, -})) - -export default CreateProjectPage diff --git a/site/pages/workspaces/[user]/[workspace].tsx b/site/pages/workspaces/[user]/[workspace].tsx index 6ae0c52713697..3540e0c317f4c 100644 --- 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) { From 3b02d211042b0735a3f7be497702cc898f257de0 Mon Sep 17 00:00:00 2001 From: Bryan Phelps Date: Wed, 23 Feb 2022 03:53:00 +0000 Subject: [PATCH 6/6] Fix infinite loop in useEffect --- site/contexts/UserContext.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/site/contexts/UserContext.tsx b/site/contexts/UserContext.tsx index 7fa2a44209fbb..7db4169140226 100644 --- a/site/contexts/UserContext.tsx +++ b/site/contexts/UserContext.tsx @@ -39,7 +39,10 @@ export const useUser = (redirectOnError = false): UserContext => { }, }) } - }, [asPath, push, 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 }