From 3e40ff716c44b421ca014da66059d9def4e61851 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Fri, 22 Nov 2024 18:02:34 -0600 Subject: [PATCH] Fix instance polling bug by switching to new `useQueryTable` (#2571) * redo useQueryTable so that the types are legit and it returns the data * fix junky getRowId and use new QueryTable on sleds and physical disks * get wild with a list-specific helper to make the call sites clean * encapsulate pageSize in the query config so it is defined in one place * do the placeholderData thing for all lists * scroll to top when page changes * loading spinner on page changes! * fix the pagination test, test lovely new scroll reset logic * fix other e2es, don't scroll reset on browser forward/back * fix bug found while converting other tables, extract useScrollReset * move columns up * convert instance list to new QueryTable, fix polling bug --- app/pages/project/instances/InstancesPage.tsx | 127 +++++++++--------- 1 file changed, 67 insertions(+), 60 deletions(-) diff --git a/app/pages/project/instances/InstancesPage.tsx b/app/pages/project/instances/InstancesPage.tsx index 97de119b4..17e1ada8f 100644 --- a/app/pages/project/instances/InstancesPage.tsx +++ b/app/pages/project/instances/InstancesPage.tsx @@ -5,12 +5,20 @@ * * Copyright Oxide Computer Company */ +import { type UseQueryOptions } from '@tanstack/react-query' import { createColumnHelper } from '@tanstack/react-table' import { filesize } from 'filesize' import { useMemo, useRef } from 'react' import { useNavigate, type LoaderFunctionArgs } from 'react-router-dom' -import { apiQueryClient, usePrefetchedApiQuery, type Instance } from '@oxide/api' +import { + apiQueryClient, + getListQFn, + queryClient, + type ApiError, + type Instance, + type InstanceResultsPage, +} from '@oxide/api' import { Instances24Icon } from '@oxide/design-system/icons/react' import { instanceTransitioning } from '~/api/util' @@ -22,7 +30,7 @@ import { InstanceStateCell } from '~/table/cells/InstanceStateCell' import { makeLinkCell } from '~/table/cells/LinkCell' import { getActionsCol } from '~/table/columns/action-col' import { Columns } from '~/table/columns/common' -import { PAGE_SIZE, useQueryTable } from '~/table/QueryTable' +import { useQueryTable } from '~/table/QueryTable2' import { CreateLink } from '~/ui/lib/CreateButton' import { EmptyMessage } from '~/ui/lib/EmptyMessage' import { PageHeader, PageTitle } from '~/ui/lib/PageHeader' @@ -46,11 +54,16 @@ const EmptyState = () => ( const colHelper = createColumnHelper() +const instanceList = ( + project: string, + // kinda gnarly, but we need refetchInterval in the component but not in the loader. + // pick refetchInterval to avoid annoying type conflicts on the full object + options?: Pick, 'refetchInterval'> +) => getListQFn('instanceList', { query: { project } }, options) + InstancesPage.loader = async ({ params }: LoaderFunctionArgs) => { const { project } = getProjectSelector(params) - await apiQueryClient.prefetchQuery('instanceList', { - query: { project, limit: PAGE_SIZE }, - }) + await queryClient.prefetchQuery(instanceList(project).optionsFn()) return null } @@ -69,6 +82,46 @@ export function InstancesPage() { { onSuccess: refetchInstances, onDelete: refetchInstances } ) + const columns = useMemo( + () => [ + colHelper.accessor('name', { + cell: makeLinkCell((instance) => pb.instance({ project, instance })), + }), + colHelper.accessor('ncpus', { + header: 'CPU', + cell: (info) => ( + <> + {info.getValue()} vCPU + + ), + }), + colHelper.accessor('memory', { + header: 'Memory', + cell: (info) => { + const memory = filesize(info.getValue(), { output: 'object', base: 2 }) + return ( + <> + {memory.value} {memory.unit} + + ) + }, + }), + colHelper.accessor( + (i) => ({ runState: i.runState, timeRunStateUpdated: i.timeRunStateUpdated }), + { + header: 'state', + cell: (info) => , + } + ), + colHelper.accessor('timeCreated', Columns.timeCreated), + getActionsCol((instance: Instance) => [ + ...makeButtonActions(instance), + ...makeMenuActions(instance), + ]), + ], + [project, makeButtonActions, makeMenuActions] + ) + // this is a whole thing. sit down. // We initialize this set as empty because we don't have the instances on hand @@ -77,10 +130,8 @@ export function InstancesPage() { const transitioningInstances = useRef>(new Set()) const pollingStartTime = useRef(Date.now()) - const { data: instances, dataUpdatedAt } = usePrefetchedApiQuery( - 'instanceList', - { query: { project, limit: PAGE_SIZE } }, - { + const { table, query } = useQueryTable({ + query: instanceList(project, { // The point of all this is to poll quickly for a certain amount of time // after some instance in the current page enters a transitional state // like starting or stopping. After that, it will keep polling, but more @@ -122,8 +173,12 @@ export function InstancesPage() { ? POLL_INTERVAL_FAST : POLL_INTERVAL_SLOW }, - } - ) + }), + columns, + emptyState: , + }) + + const { data: instances, dataUpdatedAt } = query const navigate = useNavigate() useQuickActions( @@ -143,54 +198,6 @@ export function InstancesPage() { ) ) - const { Table } = useQueryTable( - 'instanceList', - { query: { project } }, - { placeholderData: (x) => x } - ) - - const columns = useMemo( - () => [ - colHelper.accessor('name', { - cell: makeLinkCell((instance) => pb.instance({ project, instance })), - }), - colHelper.accessor('ncpus', { - header: 'CPU', - cell: (info) => ( - <> - {info.getValue()} vCPU - - ), - }), - colHelper.accessor('memory', { - header: 'Memory', - cell: (info) => { - const memory = filesize(info.getValue(), { output: 'object', base: 2 }) - return ( - <> - {memory.value} {memory.unit} - - ) - }, - }), - colHelper.accessor( - (i) => ({ runState: i.runState, timeRunStateUpdated: i.timeRunStateUpdated }), - { - header: 'state', - cell: (info) => , - } - ), - colHelper.accessor('timeCreated', Columns.timeCreated), - getActionsCol((instance: Instance) => [ - ...makeButtonActions(instance), - ...makeMenuActions(instance), - ]), - ], - [project, makeButtonActions, makeMenuActions] - ) - - if (!instances) return null - return ( <> @@ -213,7 +220,7 @@ export function InstancesPage() { New Instance - } /> + {table} ) }