Hacktoberfest 2026:メンテナが10月に向けて印を付けた、オープンで初心者向けの issue。 Hacktoberfest の issue を見る

Consider invalidating all queries on refresh instead of listing relevant ones

オープン
#3,083 コメント 1 件 リアクション 0 件 担当者 0 名 GitHub で見る

メンテナーはふだん 1 日以内に返信

まだ誰も着手していません。

評価

難易度
5/5
見積もり時間
1週間以上
初心者へのやさしさ
30/100
issue の種類
機能追加
明瞭さ
説明が足りない
活発さ
静か
技術スタック
react, typescript
領域
frontend

調査の方向性

Start with app/components/RefreshButton.tsx and the refresh call sites in app/pages/project/instances/InstancePage.tsx, app/pages/project/instances/InstancesPage.tsx, and app/pages/system/UpdatePage.tsx. Read the query-key handling near app/api/client.ts lines 240-242, then evaluate the effect on active queries and metrics queries. Done requires an agreed invalidation strategy implemented consistently without unexpected metrics refreshes.

索引モデルが issue の本文から書いたものです。

説明

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 https://github.com/oxidecomputer/dropshot/pull/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.

https://github.com/oxidecomputer/console/blob/3b995b5edce99b7a190d0053e6eb908f376fc476/app/api/client.ts#L240-L242

主要言語
TypeScript
スター
228
フォーク
22
平均マージ
8時間 59分
マージ済み PR(30日)
21

環境構築

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

oxidecomputer/console のほかの issue

oxidecomputer/console の issue をすべて見る

似ている issue

TypeScript の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。