diff --git a/app/components/InstancePageHeader.tsx b/app/components/InstancePageHeader.tsx index 6d62567673..a6299f935a 100644 --- a/app/components/InstancePageHeader.tsx +++ b/app/components/InstancePageHeader.tsx @@ -1,18 +1,17 @@ import React from 'react' -import { useParams, useNavigate } from 'react-router-dom' +import { useNavigate } from 'react-router-dom' import { Menu, MenuList, MenuButton, MenuItem } from '@reach/menu-button' import { instanceCan, useApiQuery, useApiMutation } from '@oxide/api' - import { Icon, PageHeader, PageTitle } from '@oxide/ui' +import { useParams, useToast } from '../hooks' import { InstanceDetails } from './InstanceDetails' -import { useToast } from '../hooks' export const InstancePageHeader = () => { const navigate = useNavigate() const addToast = useToast() - const { projectName, instanceName } = useParams() + const { projectName, instanceName } = useParams('projectName', 'instanceName') const { data: instance, diff --git a/app/components/InstancesTable.tsx b/app/components/InstancesTable.tsx index c07e12ed52..0152e5a992 100644 --- a/app/components/InstancesTable.tsx +++ b/app/components/InstancesTable.tsx @@ -1,5 +1,5 @@ import React from 'react' -import { useParams, Link } from 'react-router-dom' +import { Link } from 'react-router-dom' import type { Row } from 'react-table' import { useTable, useRowSelect } from 'react-table' import { Menu, MenuList, MenuButton, MenuItem } from '@reach/menu-button' @@ -15,7 +15,7 @@ import { import { classed, Icon, selectCol, Table } from '@oxide/ui' import { StatusBadge } from './StatusBadge' import { timeAgoAbbr } from '../util/date' -import { usePagination, useToast } from '../hooks' +import { usePagination, useParams, useToast } from '../hooks' const columns = [ { @@ -24,7 +24,7 @@ const columns = [ Cell: ({ value }: { value: string }) => { // TODO: is it weird to pull directly from params here and in the menu // column? seems easier than passing it in somehow - const { projectName } = useParams() + const { projectName } = useParams('projectName') return ( { const { currentPage, goToNextPage, goToPrevPage, hasPrev } = usePagination() - const { projectName } = useParams() + const { projectName } = useParams('projectName') const { data: instances } = useApiQuery( 'projectInstancesGet', { projectName, pageToken: currentPage, limit: PAGE_SIZE }, diff --git a/app/hooks/index.ts b/app/hooks/index.ts index 9494eb4d1e..82c25a91d8 100644 --- a/app/hooks/index.ts +++ b/app/hooks/index.ts @@ -1,3 +1,4 @@ -export * from './use-toast' -export * from './use-pagination' export * from './use-key' +export * from './use-pagination' +export * from './use-params' +export * from './use-toast' diff --git a/app/hooks/use-params.ts b/app/hooks/use-params.ts new file mode 100644 index 0000000000..5a6aaf49a6 --- /dev/null +++ b/app/hooks/use-params.ts @@ -0,0 +1,23 @@ +import { useParams as _useParams } from 'react-router' +import { invariant } from '../util/invariant' + +/** + * Wrapper for React Router's `useParams` that throws (in dev) if any of the + * specified params is missing. + * + * @returns an object where the params are guaranteed (in dev) to be present + */ +export function useParams( + ...paramNames: K[] +): Record { + const params = _useParams() + if (process.env.NODE_ENV !== 'production') { + for (const k of paramNames) { + invariant( + k in params, + `Param '${k}' not found in route. You might be rendering a component under the wrong route.` + ) + } + } + return params as Record +} diff --git a/app/layouts/InstanceLayout.tsx b/app/layouts/InstanceLayout.tsx index b0de7ce597..c52d9bf2dc 100644 --- a/app/layouts/InstanceLayout.tsx +++ b/app/layouts/InstanceLayout.tsx @@ -1,5 +1,5 @@ import React from 'react' -import { Outlet, useParams } from 'react-router-dom' +import { Outlet } from 'react-router-dom' import { Icon, SkipLinkTarget } from '@oxide/ui' import { @@ -13,9 +13,10 @@ import { } from './helpers' import { Breadcrumbs } from '../components/Breadcrumbs' import { InstancePageHeader } from '../components/InstancePageHeader' +import { useParams } from '../hooks' const InstanceLayout = () => { - const { instanceName } = useParams() + const { instanceName } = useParams('instanceName') return ( diff --git a/app/layouts/ProjectLayout.tsx b/app/layouts/ProjectLayout.tsx index ead86df0bb..fb8c85ebab 100644 --- a/app/layouts/ProjectLayout.tsx +++ b/app/layouts/ProjectLayout.tsx @@ -1,5 +1,5 @@ import React from 'react' -import { Outlet, useParams } from 'react-router-dom' +import { Outlet } from 'react-router-dom' import { Icon, SkipLinkTarget } from '@oxide/ui' import { @@ -12,9 +12,10 @@ import { SidebarDivider, } from './helpers' import { Breadcrumbs } from '../components/Breadcrumbs' +import { useParams } from '../hooks' const ProjectLayout = () => { - const { projectName } = useParams() + const { projectName } = useParams('projectName') return ( diff --git a/app/pages/__tests__/InstanceCreateForm.spec.tsx b/app/pages/__tests__/InstanceCreateForm.spec.tsx index 51ad67eac1..8021f8d836 100644 --- a/app/pages/__tests__/InstanceCreateForm.spec.tsx +++ b/app/pages/__tests__/InstanceCreateForm.spec.tsx @@ -15,10 +15,13 @@ import { InstanceCreateForm } from '../instances/create' const submitButton = () => screen.getByRole('button', { name: 'Create instance' }) -const postUrl = `/api/projects/${project.name}/instances` +const instancesUrl = `/api/projects/${project.name}/instances` +const disksUrl = `/api/projects/${project.name}/disks` describe('InstanceCreateForm', () => { beforeEach(() => { + // existing disk modal fetches disks on render even if it's not visible + fetchMock.get(disksUrl, 200) renderWithRouter() }) @@ -27,21 +30,21 @@ describe('InstanceCreateForm', () => { }) it('disables submit button on submit and enables on response', async () => { - const mock = fetchMock.post(postUrl, { status: 201 }) + const mock = fetchMock.post(instancesUrl, 201) const submit = submitButton() expect(submit).not.toBeDisabled() fireEvent.click(submit) - expect(mock.called()).toBeFalsy() + expect(mock.called(instancesUrl)).toBeFalsy() await waitFor(() => expect(submit).toBeDisabled()) expect(mock.done()).toBeTruthy() expect(submit).not.toBeDisabled() }) it('shows specific message for known server error code', async () => { - fetchMock.post(postUrl, { + fetchMock.post(instancesUrl, { status: 400, body: { error_code: 'ObjectAlreadyExists' }, }) @@ -54,7 +57,7 @@ describe('InstanceCreateForm', () => { }) it('shows generic message for unknown server error', async () => { - fetchMock.post(postUrl, { status: 400 }) + fetchMock.post(instancesUrl, 400) fireEvent.click(submitButton()) @@ -62,7 +65,7 @@ describe('InstanceCreateForm', () => { }) it('posts form on submit', async () => { - const mock = fetchMock.post(postUrl, { status: 201 }) + const mock = fetchMock.post(instancesUrl, 201) fireEvent.change(screen.getByLabelText('Choose a name'), { target: { value: 'new-instance' }, @@ -82,14 +85,14 @@ describe('InstanceCreateForm', () => { }) it('navigates to project page on success', async () => { - const mock = fetchMock.post(postUrl, { status: 201, body: instance }) + const mock = fetchMock.post(instancesUrl, { status: 201, body: instance }) const projectPath = `/projects/${project.name}` expect(window.location.pathname).not.toEqual(projectPath) fireEvent.click(submitButton()) - await waitFor(() => expect(mock.called()).toBeTruthy()) + await waitFor(() => expect(mock.called(instancesUrl)).toBeTruthy()) await waitFor(() => expect(mock.done()).toBeTruthy()) await waitFor(() => expect(window.location.pathname).toEqual(projectPath)) }) diff --git a/app/pages/instances/Storage.tsx b/app/pages/instances/Storage.tsx index ee03bdc9a3..5f3552654c 100644 --- a/app/pages/instances/Storage.tsx +++ b/app/pages/instances/Storage.tsx @@ -1,10 +1,10 @@ import React from 'react' -import { useParams } from 'react-router' import { useTable } from 'react-table' import type { DiskAttachment } from '@oxide/api' import { useApiQuery } from '@oxide/api' import { Button, Table } from '@oxide/ui' +import { useParams } from '../../hooks' const columns = [ { @@ -36,7 +36,7 @@ const columns = [ ] function Storage() { - const { projectName, instanceName } = useParams() + const { projectName, instanceName } = useParams('projectName', 'instanceName') const { data } = useApiQuery( 'instanceDisksGet', { projectName, instanceName }, diff --git a/app/pages/instances/create/existing-disk-modal.tsx b/app/pages/instances/create/existing-disk-modal.tsx index 35a5b3c877..d0cd23a8e7 100644 --- a/app/pages/instances/create/existing-disk-modal.tsx +++ b/app/pages/instances/create/existing-disk-modal.tsx @@ -1,5 +1,4 @@ import React from 'react' -import { useParams } from 'react-router' import { Dialog } from '@reach/dialog' import { Formik, Form } from 'formik' @@ -12,6 +11,7 @@ const headingStyle = 'font-medium mt-6 mb-3' type Props = { isOpen: boolean onDismiss: () => void + projectName: string } const isUnattached = ({ state }: Disk) => { @@ -23,8 +23,8 @@ const isUnattached = ({ state }: Disk) => { ) } -export function ExistingDiskModal({ isOpen, onDismiss }: Props) { - const { projectName } = useParams() +export function ExistingDiskModal({ isOpen, onDismiss, projectName }: Props) { + // TODO: maybe wait to fetch until you open the modal const { data } = useApiQuery('projectDisksGet', { projectName }) const disks = (data?.items || []) .filter(isUnattached) diff --git a/app/pages/instances/create/index.tsx b/app/pages/instances/create/index.tsx index 054a77e31a..d40b946a17 100644 --- a/app/pages/instances/create/index.tsx +++ b/app/pages/instances/create/index.tsx @@ -1,5 +1,5 @@ import React, { useState } from 'react' -import { useParams, useNavigate } from 'react-router-dom' +import { useNavigate } from 'react-router-dom' import { Tab, TabList, TabPanel, TabPanels, Tabs } from '@reach/tabs' import cn from 'classnames' import { Formik, Form } from 'formik' @@ -24,6 +24,7 @@ import { INSTANCE_SIZES } from './instance-types' import { NewDiskModal } from './new-disk-modal' import { ExistingDiskModal } from './existing-disk-modal' import { NetworkModal } from './network-modal' +import { useParams } from '../../../hooks' // TODO: these probably should not both exist const headingStyle = 'text-white text-display-xl font-sans font-light' @@ -237,6 +238,7 @@ export function InstanceCreateForm({ projectName }: { projectName: string }) { setShowExistingDiskModal(false)} + projectName={projectName} /> @@ -310,7 +312,7 @@ export function InstanceCreateForm({ projectName }: { projectName: string }) { } const InstanceCreatePage = () => { - const { projectName } = useParams() + const { projectName } = useParams('projectName') return ( <> diff --git a/app/pages/project/Storage.tsx b/app/pages/project/Storage.tsx index e8ee46d255..c2482c6825 100644 --- a/app/pages/project/Storage.tsx +++ b/app/pages/project/Storage.tsx @@ -1,9 +1,9 @@ import React from 'react' -import { useParams } from 'react-router' import { useTable } from 'react-table' import { useApiQuery } from '@oxide/api' import { Table } from '@oxide/ui' +import { useParams } from '../../hooks' const columns = [ { @@ -19,7 +19,7 @@ const columns = [ ] export default function ProjectStorage() { - const { projectName } = useParams() + const { projectName } = useParams('projectName') const { data } = useApiQuery( 'projectDisksGet', { projectName }, diff --git a/app/pages/project/index.tsx b/app/pages/project/index.tsx index f710974dbd..4d368fc5d0 100644 --- a/app/pages/project/index.tsx +++ b/app/pages/project/index.tsx @@ -1,12 +1,13 @@ import React from 'react' -import { useParams, Link } from 'react-router-dom' +import { Link } from 'react-router-dom' import { useApiQuery } from '@oxide/api' import { buttonStyle, PageHeader, PageTitle } from '@oxide/ui' import { InstancesTable } from '../../components/InstancesTable' +import { useParams } from '../../hooks' const ProjectPage = () => { - const { projectName } = useParams() + const { projectName } = useParams('projectName') const { data: project } = useApiQuery('projectsGetProject', { projectName, }) diff --git a/app/util/invariant.ts b/app/util/invariant.ts new file mode 100644 index 0000000000..f865305b8c --- /dev/null +++ b/app/util/invariant.ts @@ -0,0 +1,18 @@ +/** + * Throw with message if condition is falsy. Entire call stripped out by + * Rollup in prod. + */ +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function invariant(condition: any, message: string) { + if (process.env.NODE_ENV !== 'production') { + if (!condition) { + throw new Error(message) + } + } +} + +// Miraculously, Rollup not only minifies out the inside of the function when +// `NODE_ENV` is 'production', it actually removes the entire function call from +// the calling code, which means long error messages don't end up in the +// production bundle. This was manually tested in the production build to make +// sure, but here's an example in Rollup REPL: https://bit.ly/3z5cbQG