Consider invalidating all queries on refresh instead of listing relevant ones

[david-crespo]

I was thinking about how we could make #3082 harder to mess up, and I realized that it might work better to have the refresh button invalidate all queries on the page instead of a hard-coded list of relevant ones. This would mean we can't miss anything. And really we could extend this to every mutation we do — right now we manually invalidate the queries that we determine are relevant to the mutation that just happened, and I'm sure we get it wrong in some cases.

Diff showing what it would be like on instance detail

diff --git a/app/components/RefreshButton.tsx b/app/components/RefreshButton.tsx
index 2308ca7a8d..6ee3d6ed56 100644
--- a/app/components/RefreshButton.tsx
+++ b/app/components/RefreshButton.tsx
@@ -8,17 +8,22 @@
 
 import { useState } from 'react'
 
+import { queryClient } from '@oxide/api'
 import { Refresh16Icon } from '@oxide/design-system/icons/react'
 
 import { Button } from '~/ui/lib/Button'
 import { SpinnerLoader } from '~/ui/lib/Spinner'
 
-export function RefreshButton({ onClick }: { onClick: () => Promise<unknown> }) {
+/**
+ * Invalidates all queries, which causes active (currently rendered) queries to
+ * refetch. Inactive cached queries are marked stale and refetch on next use.
+ */
+export function RefreshButton() {
   const [refreshing, setRefreshing] = useState(false)
 
   async function refresh() {
     setRefreshing(true)
-    await onClick()
+    await queryClient.invalidateQueries()
     setRefreshing(false)
   }
 
diff --git a/app/pages/project/instances/InstancePage.tsx b/app/pages/project/instances/InstancePage.tsx
index e968ee18f0..596f365c54 100644
--- a/app/pages/project/instances/InstancePage.tsx
+++ b/app/pages/project/instances/InstancePage.tsx
@@ -76,22 +76,6 @@
   return nic ? nic.vpcId : undefined
 }
 
-// this is meant to cover everything that we fetch in the page
-async function refreshData() {
-  await Promise.all([
-    queryClient.invalidateEndpoint('instanceView'),
-    queryClient.invalidateEndpoint('instanceExternalIpList'),
-    queryClient.invalidateEndpoint('instanceNetworkInterfaceList'),
-    queryClient.invalidateEndpoint('instanceDiskList'), // storage tab
-    queryClient.invalidateEndpoint('instanceAntiAffinityGroupList'), // settings tab
-    queryClient.invalidateEndpoint('antiAffinityGroupList'), // settings tab
-    queryClient.invalidateEndpoint('antiAffinityGroupMemberList'), // settings tab
-    // note that we do not include timeseriesQuery because the charts on the
-    // metrics tab will manage their own refresh intervals when we turn that
-    // back on
-  ])
-}
-
 export async function clientLoader({ params }: LoaderFunctionArgs) {
   const selector = getInstanceSelector(params)
   await Promise.all([
@@ -128,7 +112,7 @@
   const navigate = useNavigate()
 
   const { makeButtonActions, makeMenuActions } = useMakeInstanceActions(instanceSelector, {
-    onSuccess: refreshData,
+    onSuccess: () => queryClient.invalidateQueries(),
     // go to project instances list since there's no more instance
     onDelete: () => {
       queryClient.invalidateEndpoint('instanceList')
@@ -175,7 +159,7 @@
       <PageHeader>
         <PageTitle icon={<Instances24Icon />}>{instance.name}</PageTitle>
         <div className="inline-flex gap-2">
-          <RefreshButton onClick={refreshData} />
+          <RefreshButton />
           <InstanceDocsPopover />
           <div className="border-default flex space-x-2 border-l pl-2">
             {makeButtonActions(instance).map((action) => (
diff --git a/app/pages/project/instances/InstancesPage.tsx b/app/pages/project/instances/InstancesPage.tsx
index 9f313cace9..92eb5d4f43 100644
--- a/app/pages/project/instances/InstancesPage.tsx
+++ b/app/pages/project/instances/InstancesPage.tsx
@@ -69,11 +69,7 @@
   return null
 }
 
-const refetchInstances = () =>
-  Promise.all([
-    queryClient.invalidateEndpoint('instanceList'),
-    queryClient.invalidateEndpoint('antiAffinityGroupMemberList'),
-  ])
+const refetchInstances = () => queryClient.invalidateQueries()
 
 const sec = 1000 // ms, obviously
 const POLL_FAST_TIMEOUT = 30 * sec
@@ -221,7 +217,7 @@
        * fix this properly when we add refresh and filtering for all tables. */}
       <TableActions className="justify-between!">
         <div className="flex items-center gap-2">
-          <RefreshButton onClick={refetchInstances} />
+          <RefreshButton />
           <Tooltip
             content="Auto-refresh is more frequent after instance actions"
             delay={150}
diff --git a/app/pages/system/UpdatePage.tsx b/app/pages/system/UpdatePage.tsx
index e9f3de05d5..8542babd38 100644
--- a/app/pages/system/UpdatePage.tsx
+++ b/app/pages/system/UpdatePage.tsx
@@ -74,11 +74,7 @@
   query: { limit: ALL_ISH },
 })
 
-const refreshData = () =>
-  Promise.all([
-    queryClient.invalidateEndpoint('systemUpdateStatus'),
-    queryClient.invalidateEndpoint('systemUpdateRepositoryList'),
-  ])
+const refreshData = () => queryClient.invalidateQueries()
 
 export async function clientLoader() {
   await Promise.all([
@@ -124,7 +120,7 @@
       <PageHeader>
         <PageTitle icon={<SoftwareUpdate24Icon />}>System Update</PageTitle>
         <div className="flex items-center gap-2">
-          <RefreshButton onClick={refreshData} />
+          <RefreshButton />
           <DocsPopover
             heading="system update"
             icon={<SoftwareUpdate16Icon />}

React Query is designed so that only queries considered active (queries that are live on the current page) would refetch, so it's not like we would trigger refetches on all the queries we've ever fired. Considering most of our API responses are pretty small, it wouldn't be a big deal to refetch the requests powering the top bar and sidebar.

My biggest concern is unnecessarily refreshing metrics queries, because they can be big and they have their own little refresh button or interval fetcher associated with them. Gzip oxidecomputer/dropshot#1448 may help with the size, though. I'm more worried about someone looking at a chart and then it changes when they didn't expect it to.

One approach to avoid automatically invalidating some queries would be to add prefix segments to the query key (currently just method name and params) that would act as a category for those calls. But I'm not sure this would really help because it just means that we're pushing the problem to a different place because we have to make sure to get the category right at call time unless we were able to make a static categorization based on what endpoint it is, and I don't think we would.

console/app/api/client.ts

Lines 240 to 242 in 3b995b5

	// method name first means we can invalidate all calls to this endpoint by
	// invalidating [f.name] (see invalidateEndpoint)
	queryKey: [f.name, params],