Skip to content

Commit

Permalink
refactor: Migrate from Next.js to pure webpack config (#360)
Browse files Browse the repository at this point in the history
Fix for #348 - migrate our NextJS project to a pure webpack project w/ a single bundle

- [x] Switch from `next/link` to `react-router-dom`'s link 

> This part was easy - just change the import to `import { Link } from "react-router-dom"` and `<Link href={...} />` to `<Link to={...} />`

- [x] Switch from `next/router` to `react-router-dom`'s paradigms (`useNavigation`, `useLocation`, and `useParams`)

> `router.push` can be converted to `navigate(...)` (provided by the `useNavigate` hook)
> `router.replace` can be converted `navigate(..., {replace: true})` 
>  Query parameters (`const { query } = useRouter`) can be converted to `const query = useParams()`)

- [x] Implement client-side routing with `react-router-dom`

> Parameterized routes in NextJS like `projects/[organization]/[project]` would look like:
> ```
>               <Route path="projects">
>                    <Route path=":organization/:project">
>                    <Route index element={<ProjectPage />} />
>                  </Route>
>               </Route>
> ```

I've hooked up a `build:analyze` command that spins up a server to show the bundle size:
<img width="1303" alt="image" src="https://user-images.githubusercontent.com/88213859/157496889-87c5fdcd-fad1-4f2e-b7b6-437aebf99641.png">

The bundle looks OK, but there are some opportunities for improvement - the heavy-weight dependencies, like React, ReactDOM, Material-UI, and lodash could be brought in via a CDN: https://stackoverflow.com/questions/50645796/how-to-import-reactjs-material-ui-using-a-cdn-through-webpacks-externals
  • Loading branch information
bryphe-coder committed Mar 12, 2022
1 parent bf1f858 commit ec077c6
Show file tree
Hide file tree
Showing 47 changed files with 1,903 additions and 1,302 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/coder.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,8 @@ jobs:
working-directory: site

- run: yarn playwright:test
env:
DEBUG: pw:api
working-directory: site

- name: Upload DataDog Trace
Expand Down
1 change: 0 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ provisionersdk/proto: provisionersdk/proto/provisioner.proto
site/out:
./scripts/yarn_install.sh
cd site && yarn build
cd site && yarn export
# Restores GITKEEP files!
git checkout HEAD site/out
.PHONY: site/out
Expand Down
2 changes: 1 addition & 1 deletion coderd/coderd.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ func New(options *Options) (http.Handler, func()) {
r.Get("/resources", api.workspaceBuildResources)
})
})
r.NotFound(site.Handler(options.Logger).ServeHTTP)
r.NotFound(site.DefaultHandler().ServeHTTP)
return r, api.websocketWaitGroup.Wait
}

Expand Down
14 changes: 14 additions & 0 deletions site/Main.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import React from "react"
import ReactDOM from "react-dom"

import { App } from "./app"

// This is the entry point for the app - where everything start.
// In the future, we'll likely bring in more bootstrapping logic -
// like: https://github.com/coder/m/blob/50898bd4803df7639bd181e484c74ac5d84da474/product/coder/site/pages/_app.tsx#L32
const main = () => {
const element = document.getElementById("root")
ReactDOM.render(<App />, element)
}

main()
10 changes: 0 additions & 10 deletions site/_jest/setupTests.ts

This file was deleted.

73 changes: 73 additions & 0 deletions site/app.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import React from "react"
import CssBaseline from "@material-ui/core/CssBaseline"
import ThemeProvider from "@material-ui/styles/ThemeProvider"
import { SWRConfig } from "swr"
import { UserProvider } from "./contexts/UserContext"
import { light } from "./theme"
import { BrowserRouter as Router, Route, Routes } from "react-router-dom"

import { CliAuthenticationPage } from "./pages/cli-auth"
import { NotFoundPage } from "./pages/404"
import { IndexPage } from "./pages/index"
import { SignInPage } from "./pages/login"
import { ProjectsPage } from "./pages/projects"
import { ProjectPage } from "./pages/projects/[organization]/[project]"
import { CreateWorkspacePage } from "./pages/projects/[organization]/[project]/create"
import { WorkspacePage } from "./pages/workspaces/[workspace]"

export const App: React.FC = () => {
return (
<Router>
<SWRConfig
value={{
// This code came from the SWR documentation:
// https://swr.vercel.app/docs/error-handling#status-code-and-error-object
fetcher: async (url: string) => {
const res = await fetch(url)

// By default, `fetch` won't treat 4xx or 5xx response as errors.
// However, we want SWR to treat these as errors - so if `res.ok` is false,
// we want to throw an error to bubble that up to SWR.
if (!res.ok) {
const err = new Error((await res.json()).error?.message || res.statusText)
throw err
}
return res.json()
},
}}
>
<UserProvider>
<ThemeProvider theme={light}>
<CssBaseline />

<Routes>
<Route path="/">
<Route index element={<IndexPage />} />

<Route path="login" element={<SignInPage />} />
<Route path="cli-auth" element={<CliAuthenticationPage />} />

<Route path="projects">
<Route index element={<ProjectsPage />} />
<Route path=":organization/:project">
<Route index element={<ProjectPage />} />
<Route path="create" element={<CreateWorkspacePage />} />
</Route>
</Route>

<Route path="workspaces">
<Route path=":workspace" element={<WorkspacePage />} />
</Route>

{/* Using path="*"" means "match anything", so this route
acts like a catch-all for URLs that we don't have explicit
routes for. */}
<Route path="*" element={<NotFoundPage />} />
</Route>
</Routes>
</ThemeProvider>
</UserProvider>
</SWRConfig>
</Router>
)
}
4 changes: 2 additions & 2 deletions site/components/Navbar/index.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from "react"
import Button from "@material-ui/core/Button"
import { makeStyles } from "@material-ui/core/styles"
import Link from "next/link"
import { Link } from "react-router-dom"

import { User } from "../../contexts/UserContext"
import { Logo } from "../Icons"
Expand All @@ -17,7 +17,7 @@ export const Navbar: React.FC<NavbarProps> = ({ user, onSignOut }) => {
return (
<div className={styles.root}>
<div className={styles.fixed}>
<Link href="/">
<Link to="/">
<Button className={styles.logo} variant="text">
<Logo fill="white" opacity={1} />
</Button>
Expand Down
22 changes: 0 additions & 22 deletions site/components/Redirect.test.tsx

This file was deleted.

23 changes: 0 additions & 23 deletions site/components/Redirect.tsx

This file was deleted.

13 changes: 6 additions & 7 deletions site/components/SignIn/SignInForm.test.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
import React from "react"
import singletonRouter from "next/router"
import mockRouter from "next-router-mock"
import { act, fireEvent, render, screen, waitFor } from "@testing-library/react"
import { act, fireEvent, screen, waitFor } from "@testing-library/react"
import { history, render } from "../../test_helpers"

import { SignInForm } from "./SignInForm"

describe("SignInForm", () => {
beforeEach(() => {
mockRouter.setCurrentUrl("/login")
history.replace("/")
})

it("renders content", async () => {
Expand Down Expand Up @@ -56,14 +55,14 @@ describe("SignInForm", () => {

// Then
// Should redirect because login was successful
await waitFor(() => expect(singletonRouter).toMatchObject({ asPath: "/" }))
await waitFor(() => expect(history.location.pathname).toEqual("/"))
})

it("respects ?redirect query parameter when complete", async () => {
// Given
const loginHandler = (_email: string, _password: string) => Promise.resolve()
// Set a path to redirect to after login is successful
mockRouter.setCurrentUrl("/login?redirect=%2Fsome%2Fother%2Fpath")
history.replace("/login?redirect=%2Fsome%2Fother%2Fpath")

// When
// Render the component
Expand All @@ -78,6 +77,6 @@ describe("SignInForm", () => {

// Then
// Should redirect to /some/other/path because ?redirect was specified and login was successful
await waitFor(() => expect(singletonRouter).toMatchObject({ asPath: "/some/other/path" }))
await waitFor(() => expect(history.location.pathname).toEqual("/some/other/path"))
})
})
22 changes: 11 additions & 11 deletions site/components/SignIn/SignInForm.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { makeStyles } from "@material-ui/core/styles"
import { FormikContextType, useFormik } from "formik"
import { NextRouter, useRouter } from "next/router"
import { Location } from "history"
import { useNavigate, useLocation } from "react-router-dom"
import React from "react"
import { useSWRConfig } from "swr"
import * as Yup from "yup"
Expand All @@ -9,7 +10,6 @@ import { Welcome } from "./Welcome"
import { FormTextField } from "../Form"
import * as API from "./../../api"
import { LoadingButton } from "./../Button"
import { firstOrItem } from "../../util/array"

/**
* BuiltInAuthFormValues describes a form using built-in (email/password)
Expand Down Expand Up @@ -47,7 +47,8 @@ export interface SignInProps {
export const SignInForm: React.FC<SignInProps> = ({
loginHandler = (email: string, password: string) => API.login(email, password),
}) => {
const router = useRouter()
const navigate = useNavigate()
const location = useLocation()
const styles = useStyles()
const { mutate } = useSWRConfig()

Expand All @@ -63,8 +64,8 @@ export const SignInForm: React.FC<SignInProps> = ({
// Tell SWR to invalidate the cache for the user endpoint
await mutate("/api/v2/users/me")

const redirect = getRedirectFromRouter(router)
await router.push(redirect)
const redirect = getRedirectFromLocation(location)
await navigate(redirect)
} catch (err) {
helpers.setFieldError("password", "The username or password is incorrect.")
}
Expand Down Expand Up @@ -121,11 +122,10 @@ export const SignInForm: React.FC<SignInProps> = ({
)
}

const getRedirectFromRouter = (router: NextRouter) => {
const getRedirectFromLocation = (location: Location) => {
const defaultRedirect = "/"
if (router.query.redirect) {
return firstOrItem(router.query.redirect, defaultRedirect)
} else {
return defaultRedirect
}

const searchParams = new URLSearchParams(location.search)
const redirect = searchParams.get("redirect")
return redirect ? redirect : defaultRedirect
}
4 changes: 2 additions & 2 deletions site/components/Workspace/Workspace.test.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { render, screen } from "@testing-library/react"
import { screen } from "@testing-library/react"
import React from "react"
import { Workspace } from "./Workspace"
import { MockOrganization, MockProject, MockWorkspace } from "../../test_helpers"
import { MockOrganization, MockProject, MockWorkspace, render } from "../../test_helpers"

describe("Workspace", () => {
it("renders", async () => {
Expand Down
4 changes: 2 additions & 2 deletions site/components/Workspace/Workspace.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import Paper from "@material-ui/core/Paper"
import Typography from "@material-ui/core/Typography"
import { makeStyles } from "@material-ui/core/styles"
import CloudCircleIcon from "@material-ui/icons/CloudCircle"
import Link from "next/link"
import { Link } from "react-router-dom"
import React from "react"
import * as Constants from "./constants"
import * as API from "../../api"
Expand Down Expand Up @@ -68,7 +68,7 @@ export const WorkspaceHeader: React.FC<WorkspaceProps> = ({ organization, projec
<div className={styles.vertical}>
<Typography variant="h4">{workspace.name}</Typography>
<Typography variant="body2" color="textSecondary">
<Link href={projectLink}>{project.name}</Link>
<Link to={projectLink}>{project.name}</Link>
</Typography>
</div>
</div>
Expand Down
1 change: 0 additions & 1 deletion site/components/index.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
export * from "./Button"
export * from "./EmptyState"
export * from "./Page"
export * from "./Redirect"
18 changes: 9 additions & 9 deletions site/contexts/UserContext.test.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
import singletonRouter from "next/router"
import mockRouter from "next-router-mock"
import React from "react"
import { SWRConfig } from "swr"
import { render, screen, waitFor } from "@testing-library/react"

import { screen, waitFor } from "@testing-library/react"
import { User, UserProvider, useUser } from "./UserContext"
import { MockUser } from "../test_helpers"
import { history, MockUser, render } from "../test_helpers"

namespace Helpers {
// Helper component that renders out the state of the `useUser` hook.
Expand Down Expand Up @@ -54,7 +51,7 @@ describe("UserContext", () => {

// Reset the router to '/' before every test
beforeEach(() => {
mockRouter.setCurrentUrl("/")
history.replace("/")
})

it("shouldn't redirect if user fails to load and redirectOnFailure is false", async () => {
Expand All @@ -67,7 +64,8 @@ describe("UserContext", () => {
expect(screen.queryByText("Error:", { exact: false })).toBeDefined()
})
// ...and the route should be unchanged
expect(singletonRouter).toMatchObject({ asPath: "/" })
expect(history.location.pathname).toEqual("/")
expect(history.location.search).toEqual("")
})

it("should redirect if user fails to load and redirectOnFailure is true", async () => {
Expand All @@ -76,7 +74,8 @@ describe("UserContext", () => {

// Then
// Verify we route to the login page
await waitFor(() => expect(singletonRouter).toMatchObject({ asPath: "/login?redirect=%2F" }))
await waitFor(() => expect(history.location.pathname).toEqual("/login"))
await waitFor(() => expect(history.location.search).toEqual("?redirect=%2F"))
})

it("should not redirect if user loads and redirectOnFailure is true", async () => {
Expand All @@ -89,6 +88,7 @@ describe("UserContext", () => {
expect(screen.queryByText("Me:", { exact: false })).toBeDefined()
})
// ...and the route should be unchanged
expect(singletonRouter).toMatchObject({ asPath: "/" })
expect(history.location.pathname).toEqual("/")
expect(history.location.search).toEqual("")
})
})

0 comments on commit ec077c6

Please sign in to comment.