Consider invalidating all queries on refresh instead of listing relevant ones
メンテナーはふだん 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.
- 主要言語
- TypeScript
- スター
- 228
- フォーク
- 22
- 平均マージ
- 8時間 59分
- マージ済み PR(30日)
- 21
環境構築
- Dockerfile・Docker Compose ファイルなし
- プルリクエストのテンプレートなし
- コントリビューションガイドを読む
はじめの一歩
- issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
- 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
- リポジトリをフォークし、ブランチを切って変更します。
- issue 番号を参照したプルリクエストを送ります。
oxidecomputer/console のほかの issue
-
Disks: Disable 'Size' field when selecting 'Make disk read-only' and 'Image'/'Snapshot' as a sourceオープン
難易度 2/5 1〜3時間 初心者へのやさしさ 72/100
oxidecomputer/console#3391 · コメント 2 件 ·
メンテナーはふだん 1 日以内に返信
-
Alerting > Receivers: Error message for subscriptions gets hidden by empty dropdown when zoomed outオープン
難易度 2/5 1〜3時間 初心者へのやさしさ 68/100
oxidecomputer/console#3390 ·
メンテナーはふだん 1 日以内に返信
-
難易度 2/5 半日 初心者へのやさしさ 72/100
oxidecomputer/console#3375 ·
メンテナーはふだん 1 日以内に返信
-
難易度 2/5 1〜3時間 初心者へのやさしさ 65/100
oxidecomputer/console#3338 · コメント 1 件 ·
メンテナーはふだん 1 日以内に返信
-
難易度 2/5 1〜3時間 初心者へのやさしさ 72/100
oxidecomputer/console#3327 · コメント 2 件 ·
メンテナーはふだん 1 日以内に返信
oxidecomputer/console の issue をすべて見る
似ている issue
-
needs:triage
難易度 2/5 1〜3時間 初心者へのやさしさ 84/100
メンテナーはふだん 1 日以内に返信
-
ai-discovered
難易度 2/5 1〜3時間 初心者へのやさしさ 83/100
メンテナーはふだん 1 日以内に返信
-
難易度 2/5 1〜3時間 初心者へのやさしさ 88/100
jessepollak/home#1627 ·
メンテナーはふだん 1 日以内に返信
-
agent-canvas bug llm priority:low ready-for-dev
難易度 2/5 1〜3時間 初心者へのやさしさ 82/100
OpenHands/OpenHands#17806 · コメント 3 件 ·
メンテナーはふだん 1 日以内に返信
-
bug
難易度 2/5 1〜3時間 初心者へのやさしさ 76/100
radius-project/ai-extensions#923 ·
メンテナーはふだん 1 日以内に返信