* fix(site): remove duplicate mask-icon
/favicon.svg should 404, but it's also a duplicate of the mask-icon
that has crossorigin="use-credentials"
refactor(site): link attribute ordering
I believe the former duplication of rel="mask-icon" was because the
crossorigin and rel attributes made it less readable.
* refactor(site): reorder link and meta tags
* refactor(site): match v1 tsconfig
Summary of changes:
* ordered configurations for ease of readability
* allowJs removed -> not needed after Next.js
* noImplicitAny removed -> this is set to true by strict
* strictNullChecks removed -> this is set to true by strict
* removed lingering next-env.d.ts in include
* refactor(site): simplify tsconfig.test.json
Because that the base tsconfig was simplified when changing from
Next.js, this one now duplicates most of it. I still believe having a
separate tsconfig is a good idea for the purposes of separating tests
from source code. It especially can make it easier for eslint and jest
performance.
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
This brings an initial E2E test (really, an integration test - it's only running the server locally, as opposed to against a deployment - but it'd be easy to point playwright to a deployment).
Demo gif:
This test exercises a minimal flow for login:
- Run the `coderd` binary to start a server on 3000
- Create an initial user as part of setup
- Go through the login flow and verify we land on the projects page
It will be useful to have to ensure that #360 doesn't introduce a regression in the login flow
Future E2E tests that would be useful:
- Create a project & verify it shows in the UI
- Create a workspace and verify it shows in the UI
While going through the create projects flow, I realized there's a typo on the projects page.
The command should read `coder projects create`, but instead it is currently showing `coder project create`:
<img width="466" alt="Screen Shot 2022-03-01 at 5 31 19 PM" src="https://user-images.githubusercontent.com/88213859/156277646-6fc03977-6dda-4db4-9fce-e48b4b5efba3.png">
Fixing this typo will alleviate some confusion as we on-board developers to v2
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
* Switch from memfs to built-in fstest.MapFS
* Ensure httptest servers are closed during test cleanup
* Swap ordering of expected/actual values in assertion functions
* Use http.StatusOK constants for status codes
* Add a 1-second context timeout for each request
* Use the test httptest.Server.Client() so that we can handle
TLS server certificates if desired
Fixes#210 - this isPR implements `coder login` in the case where the default user is already created.
This change adds:
- A prompt in the case where there is not an initial user that opens the server URL + requests a session token
- This ports over some code from v1 for the `openURL` and `isWSL` functions to support opening the browser
- A `/api/v2/api-keys` endpoint that can be `POST`'d to in order to request a new api key for a user
- This route was inspired by the v1 functionality
- A `cli-auth` route + page that shows the generated api key
- Tests for the new code + storybook for the new UI
The `/cli-auth` route, like in v1, is very minimal:
<img width="624" alt="Screen Shot 2022-02-16 at 5 05 07 PM" src="https://user-images.githubusercontent.com/88213859/154384627-78ab9841-27bf-490f-9bbe-23f8173c9e97.png">
And the terminal UX looks like this:
This is just a quick fix so that the redirection is correct after creating a workspace - so that we make it to our minimal 'workspaces' page. Mainly so we have a path for testing / onboarding more people.
Unfortunately the NextJS pages are a bit tricky to test - we don't have infra for it because of the special pathing requirements - we can potentially bring in a library like [next-page-tester](https://github.com/next-page-tester/next-page-tester) and create a separate test directory like `pages_test` - but since we may pick up the RFC to move away from Next, it doesn't seem like a useful effort.
With this, creating a project lands on our minimal workspaces page, and the links on the Projects page correctly navigate to the minimal workspaces page.
Fixes#304
Unblocks #298
After logging in, the login flow should redirect to a whatever path is specified by the `?redirect` query parameter. This is important for cases like #298 - where we need to set `?redirect=%2Fcli_auth`, but also really any case where the user is linked and might have to go back to the login screen.
The fix is simple - just check if the `redirect` query parameter is set, and if it is, use that as the path to redirect to on success. Also adds a test case - we had one checking that we redirect to the default (root `/`) url, but not one of the `?redirect` param
In v1, we had a quick and easy way to leave creation forms - an escape button (and key handler) in the top right corner.
This was especially helpful in cases where the form was long enough that the 'Cancel' button was off-screen.
This ports over that component into v2 and hooks up into our two existing forms:
<img width="977" alt="Screen Shot 2022-02-09 at 5 48 03 PM" src="https://user-images.githubusercontent.com/88213859/153321503-af957f2b-d674-4ebf-9ac5-97939cb9153f.png">
In addition, this adds test cases + a storybook story for it.
Fixes#244
- Adds `jest-junit`
- Configures `jest-junit` to output `junit.xml` in `site/test_results`
- Uploads the emitted `junit.xml` to datadog as part of the `test/js` workflow
While working on the projects page, I noticed the `/projects` endpoint would return 'null' instead of an empty array. An empty array would simplify the behavior on the client page.
There were already tests in place for this, but they only validated that the item's length is 0... and it turns out `len(nil)` is also `0`.
So this does a couple things:
- Updates the empty-project tests to check for `NotNil` as well
- Update the project endpoints to return an empty array if no rows are returned
- Remove the hack in `/projects` page for populating data, as it is no longer needed
An issue came up last week... our `embed.go` strategy doesn't handle dynamic NextJS-style routes! This is a blocker, because I'm aiming to set up CD on Monday, and the v2 UI makes heavy use of dynamic routing.
As a potential solution, this implements a go pkg `nextrouter` that serves `html` files, but respecting the dynamic routing behavior of NextJS:
- Files that have square brackets - ie `[providers]` provide a single-level dynamic route
- Files that have `[[...` prefix - ie `[[...any]]` - are catch-all routes.
- Files should be preferred over folders (ie, `providers.html` is preferred over `/providers`)
- Fixes the trailing-slash bug we hit in the previous `embed` strategy
This also integrates with `slog.Logger` for tracing, and handles injecting template parameters - a feature we need in v1 and v2 to be able to inject stuff like CSRF tokens.
This implements testing by using an in-memory file-system, so that we can exercise all of these cases.
In addition, this adjust V2's `embed.go` strategy to use `nextrouter`, which simplifies that file considerably. I'm tempted to factor out the `secureheaders` logic into a separate package, too.
If this works OK, it could be used for V1 too (although that scenario is more complex due to our hybrid-routing strategy). Based on our FE variety meeting, there's always a chance we could move away from NextJS in v1 - if that's the case, this router will still work and be more tested than our previous strategy (it just won't make use of dynamic routing). So I figured this was worth doing to make sure we can make forward progress in V2.
G r e y
