From 6964f75cd0fa2afac869de126b52db1e75cb1f56 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 10 Aug 2026 20:40:26 -0700 Subject: [PATCH] refactor(renderer): share metadata list request lifecycle (#13524) --- config/max-lines-baseline.txt | 1 - .../src/hooks/useGitHubSlugMetadata.ts | 204 ++------- .../src/hooks/useIssueMetadata.test.tsx | 63 ++- src/renderer/src/hooks/useIssueMetadata.ts | 407 +++--------------- .../src/hooks/useMetadataListRequest.test.tsx | 68 +++ .../src/hooks/useMetadataListRequest.ts | 72 ++++ 6 files changed, 314 insertions(+), 501 deletions(-) create mode 100644 src/renderer/src/hooks/useMetadataListRequest.test.tsx create mode 100644 src/renderer/src/hooks/useMetadataListRequest.ts diff --git a/config/max-lines-baseline.txt b/config/max-lines-baseline.txt index 087e2fece05..8710b4e82e3 100644 --- a/config/max-lines-baseline.txt +++ b/config/max-lines-baseline.txt @@ -269,7 +269,6 @@ inline src/renderer/src/hooks/useComposerState.ts inline src/renderer/src/hooks/useEditorExternalWatch.ts inline src/renderer/src/hooks/useIpcEvents.test.ts inline src/renderer/src/hooks/useIpcEvents.ts -inline src/renderer/src/hooks/useIssueMetadata.ts inline src/renderer/src/hooks/useSettingsNavigationMetadata.ts inline src/renderer/src/lib/active-agent-note-send.test.ts inline src/renderer/src/lib/file-type-icons.ts diff --git a/src/renderer/src/hooks/useGitHubSlugMetadata.ts b/src/renderer/src/hooks/useGitHubSlugMetadata.ts index 4635fe85250..3da1c66787d 100644 --- a/src/renderer/src/hooks/useGitHubSlugMetadata.ts +++ b/src/renderer/src/hooks/useGitHubSlugMetadata.ts @@ -1,31 +1,13 @@ -// Why: when the dialog opens for a Project row whose repo differs from the -// active workspace, label/assignee lookups must target the row's repo via -// slug-addressed IPCs (`listLabelsBySlug` / `listAssignableUsersBySlug`), -// not via the workspace path. These hooks live in their own module so the -// existing repoPath-keyed hooks stay focused on the local-workspace flow -// and so this file remains under the lint line cap. -/* oxlint-disable react-doctor/no-adjust-state-on-prop-change -- Why: slug metadata hooks clear stale rows and track loading while async provider cache requests are in flight. */ -import { useEffect, useRef, useState } from 'react' import { callRuntimeRpc, getActiveRuntimeTarget } from '@/runtime/runtime-rpc-client' import type { GitHubAssignableUser, GlobalSettings } from '../../../shared/types' import type { ListAssignableUsersBySlugResult, ListLabelsBySlugResult } from '../../../shared/github-project-types' -import { - clearMetadataRequestStore, - createMetadataRequestStore, - getFreshMetadata, - loadMetadata -} from './metadata-request-cache' +import { clearMetadataRequestStore, createMetadataRequestStore } from './metadata-request-cache' import { githubRepoIdentityKey } from '../../../shared/github-repository-identity-key' import { githubProjectHost } from '../../../shared/github-project-identity' - -type MetadataState = { - data: T - loading: boolean - error: string | null -} +import { useMetadataListRequest, type MetadataListState } from './useMetadataListRequest' const slugLabelStore = createMetadataRequestStore() const slugAssigneeStore = createMetadataRequestStore() @@ -40,86 +22,41 @@ export function useRepoLabelsBySlug( repo: string | null, settings?: Pick | null, host?: string -): MetadataState { - const [state, setState] = useState>({ - data: [], - loading: false, - error: null - }) - const activeKeyRef = useRef(null) - // Why: parent selectors can pass a fresh settings object each render; keying - // the effect on the primitive env id keeps a failure's setState from re-running - // the effect and re-issuing the fetch in a render-paced loop (same class as - // the seedKey stabilization below). +): MetadataListState { const activeRuntimeEnvironmentId = settings?.activeRuntimeEnvironmentId ?? null + const target = getActiveRuntimeTarget({ activeRuntimeEnvironmentId }) + const selectedOwner = owner ?? '' + const selectedRepo = repo ?? '' + const repositoryKey = owner && repo ? githubRepoIdentityKey({ owner, repo, host }) : null + const cacheKey = + repositoryKey && target.kind === 'environment' + ? `runtime:${target.environmentId}:${repositoryKey}` + : repositoryKey - useEffect(() => { - if (!owner || !repo) { - return - } - const target = getActiveRuntimeTarget({ activeRuntimeEnvironmentId }) - const repositoryKey = githubRepoIdentityKey({ owner, repo, host }) - const key = - target.kind === 'environment' - ? `runtime:${target.environmentId}:${repositoryKey}` - : repositoryKey - - const cached = getFreshMetadata(slugLabelStore, key) - if (cached) { - if (activeKeyRef.current !== key) { - setState({ data: cached.data, loading: false, error: null }) - } - activeKeyRef.current = key - return - } - - if (activeKeyRef.current === key) { - return - } - activeKeyRef.current = key - const requestKey = key - setState((s) => ({ - ...s, - data: s.data.length ? ([] as typeof s.data) : s.data, - loading: true, - error: null - })) - loadMetadata(slugLabelStore, key, () => + return useMetadataListRequest({ + cacheKey, + store: slugLabelStore, + errorFallback: 'Failed to load labels', + load: () => (target.kind === 'environment' ? callRuntimeRpc( target, 'github.project.listLabelsBySlug', - { owner, repo, host: githubProjectHost(host) }, + { owner: selectedOwner, repo: selectedRepo, host: githubProjectHost(host) }, { timeoutMs: 30_000 } ) - : window.api.gh.listLabelsBySlug({ owner, repo, host: githubProjectHost(host) }) + : window.api.gh.listLabelsBySlug({ + owner: selectedOwner, + repo: selectedRepo, + host: githubProjectHost(host) + }) ).then((res) => { if (!res.ok) { throw new Error(res.error.message) } return res.labels }) - ) - .then((data) => { - if (activeKeyRef.current !== requestKey) { - return - } - setState({ data, loading: false, error: null }) - }) - .catch((err) => { - if (activeKeyRef.current !== requestKey) { - return - } - activeKeyRef.current = null - setState((s) => ({ - ...s, - loading: false, - error: err instanceof Error ? err.message : 'Failed to load labels' - })) - }) - }, [owner, repo, host, activeRuntimeEnvironmentId]) - - return state + }) } export function useRepoAssigneesBySlug( @@ -128,61 +65,30 @@ export function useRepoAssigneesBySlug( seedLogins?: string[], settings?: Pick | null, host?: string -): MetadataState { - const [state, setState] = useState>({ - data: [], - loading: false, - error: null - }) - const activeKeyRef = useRef(null) - // Why: seedLogins is a new array reference each parent render. Stabilize on - // the joined-string identity so the effect doesn't re-fire on every render - // — this is the assignee popover refetch-storm fix. +): MetadataListState { const seedKey = (seedLogins ?? []).slice().sort().join(',') - // Why: see useRepoLabelsBySlug — primitive env id keeps failure setState from - // re-arming the effect through a fresh settings object identity. const activeRuntimeEnvironmentId = settings?.activeRuntimeEnvironmentId ?? null + const target = getActiveRuntimeTarget({ activeRuntimeEnvironmentId }) + const selectedOwner = owner ?? '' + const selectedRepo = repo ?? '' + const repositoryKey = owner && repo ? githubRepoIdentityKey({ owner, repo, host }) : null + const cacheKey = repositoryKey + ? target.kind === 'environment' + ? `runtime:${target.environmentId}:${repositoryKey}#${seedKey}` + : `${repositoryKey}#${seedKey}` + : null + const args = { + owner: selectedOwner, + repo: selectedRepo, + host: githubProjectHost(host), + ...(seedKey ? { seedLogins: seedKey.split(',') } : {}) + } - useEffect(() => { - if (!owner || !repo) { - return - } - const target = getActiveRuntimeTarget({ activeRuntimeEnvironmentId }) - const repositoryKey = githubRepoIdentityKey({ owner, repo, host }) - const key = - target.kind === 'environment' - ? `runtime:${target.environmentId}:${repositoryKey}#${seedKey}` - : `${repositoryKey}#${seedKey}` - - const cached = getFreshMetadata(slugAssigneeStore, key) - if (cached) { - // Why: see useRepoLabelsBySlug — avoid cached no-op writes when only - // the settings object identity changed. - if (activeKeyRef.current !== key) { - setState({ data: cached.data, loading: false, error: null }) - } - activeKeyRef.current = key - return - } - - if (activeKeyRef.current === key) { - return - } - activeKeyRef.current = key - const requestKey = key - setState((s) => ({ - ...s, - data: s.data.length ? ([] as typeof s.data) : s.data, - loading: true, - error: null - })) - const args = { - owner, - repo, - host: githubProjectHost(host), - ...(seedKey ? { seedLogins: seedKey.split(',') } : {}) - } - loadMetadata(slugAssigneeStore, key, () => + return useMetadataListRequest({ + cacheKey, + store: slugAssigneeStore, + errorFallback: 'Failed to load assignees', + load: () => (target.kind === 'environment' ? callRuntimeRpc( target, @@ -197,25 +103,5 @@ export function useRepoAssigneesBySlug( } return res.users }) - ) - .then((data) => { - if (activeKeyRef.current !== requestKey) { - return - } - setState({ data, loading: false, error: null }) - }) - .catch((err) => { - if (activeKeyRef.current !== requestKey) { - return - } - activeKeyRef.current = null - setState((s) => ({ - ...s, - loading: false, - error: err instanceof Error ? err.message : 'Failed to load assignees' - })) - }) - }, [owner, repo, host, seedKey, activeRuntimeEnvironmentId]) - - return state + }) } diff --git a/src/renderer/src/hooks/useIssueMetadata.test.tsx b/src/renderer/src/hooks/useIssueMetadata.test.tsx index c0b79d220d6..c633d543daa 100644 --- a/src/renderer/src/hooks/useIssueMetadata.test.tsx +++ b/src/renderer/src/hooks/useIssueMetadata.test.tsx @@ -5,6 +5,7 @@ import { createRoot, type Root } from 'react-dom/client' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { clearLinearMetadataCache, + useRepoLabels, useTeamLabels, useTeamMembers, useTeamStates, @@ -17,6 +18,9 @@ const linearMocks = vi.hoisted(() => ({ linearTeamMembers: vi.fn() })) +const runtimeMocks = vi.hoisted(() => ({ callRuntimeRpc: vi.fn() })) +const githubMocks = vi.hoisted(() => ({ listLabels: vi.fn() })) + vi.mock('@/runtime/runtime-linear-client', () => ({ linearTeamStates: linearMocks.linearTeamStates, linearTeamLabels: linearMocks.linearTeamLabels, @@ -24,7 +28,7 @@ vi.mock('@/runtime/runtime-linear-client', () => ({ })) vi.mock('@/runtime/runtime-rpc-client', () => ({ - callRuntimeRpc: vi.fn(), + callRuntimeRpc: runtimeMocks.callRuntimeRpc, getActiveRuntimeTarget: (settings?: { activeRuntimeEnvironmentId?: string | null } | null) => settings?.activeRuntimeEnvironmentId ? { kind: 'environment', environmentId: settings.activeRuntimeEnvironmentId } @@ -33,6 +37,13 @@ vi.mock('@/runtime/runtime-rpc-client', () => ({ const roots: Root[] = [] +function installWindowApi(): void { + Object.defineProperty(window, 'api', { + configurable: true, + value: { gh: { listLabels: githubMocks.listLabels } } + }) +} + async function flushEffects(): Promise { await act(async () => { await Promise.resolve() @@ -50,12 +61,15 @@ function renderProbe(element: React.ReactNode): void { }) } -describe('useIssueMetadata Linear hooks', () => { +describe('useIssueMetadata hooks', () => { beforeEach(() => { clearLinearMetadataCache() linearMocks.linearTeamStates.mockReset() linearMocks.linearTeamLabels.mockReset() linearMocks.linearTeamMembers.mockReset() + runtimeMocks.callRuntimeRpc.mockReset() + githubMocks.listLabels.mockReset() + installWindowApi() }) afterEach(() => { @@ -65,6 +79,51 @@ describe('useIssueMetadata Linear hooks', () => { document.body.replaceChildren() }) + it('routes repo-id-only folder metadata through local IPC', async () => { + let labels: string[] = [] + githubMocks.listLabels.mockResolvedValue(['folder']) + + function LabelsProbe(): null { + labels = useRepoLabels(null, 'folder-repo-id').data + return null + } + + renderProbe() + await flushEffects() + + expect(labels).toEqual(['folder']) + expect(githubMocks.listLabels).toHaveBeenCalledExactlyOnceWith({ + repoPath: '', + repoId: 'folder-repo-id' + }) + expect(runtimeMocks.callRuntimeRpc).not.toHaveBeenCalled() + }) + + it('prefers an explicit remote environment and repo id', async () => { + let labels: string[] = [] + runtimeMocks.callRuntimeRpc.mockResolvedValue(['remote']) + + function LabelsProbe(): null { + labels = useRepoLabels('/local/repo', 'remote-repo-id', { + runtimeEnvironmentId: ' env-explicit ', + activeRuntimeEnvironmentId: 'env-active' + }).data + return null + } + + renderProbe() + await flushEffects() + + expect(labels).toEqual(['remote']) + expect(runtimeMocks.callRuntimeRpc).toHaveBeenCalledExactlyOnceWith( + { kind: 'environment', environmentId: 'env-explicit' }, + 'github.listLabels', + { repo: 'remote-repo-id' }, + { timeoutMs: 15_000 } + ) + expect(githubMocks.listLabels).not.toHaveBeenCalled() + }) + it('does not loop when cached team-state metadata is read with a fresh settings object', async () => { let renders = 0 let states: unknown[] = [] diff --git a/src/renderer/src/hooks/useIssueMetadata.ts b/src/renderer/src/hooks/useIssueMetadata.ts index b694c5491dc..8d8a44599ef 100644 --- a/src/renderer/src/hooks/useIssueMetadata.ts +++ b/src/renderer/src/hooks/useIssueMetadata.ts @@ -1,6 +1,3 @@ -/* eslint-disable max-lines -- Why: repo metadata hooks share TTL caches and -Linear/GitHub cache invalidation entrypoints used by the issue dialog. */ -/* oxlint-disable react-doctor/no-adjust-state-on-prop-change -- Why: issue metadata hooks clear stale rows and track loading while async provider cache requests are in flight. */ import { useEffect, useMemo, useRef, useState } from 'react' import { callRuntimeRpc, getActiveRuntimeTarget } from '@/runtime/runtime-rpc-client' import { @@ -24,20 +21,13 @@ import { loadMetadata, type MetadataRequestStore } from './metadata-request-cache' - -type MetadataState = { - data: T - loading: boolean - error: string | null -} +import { useMetadataListRequest, type MetadataListState } from './useMetadataListRequest' type GitHubMetadataOptions = { runtimeEnvironmentId?: string | null activeRuntimeEnvironmentId?: string | null } -// ─── GitHub ──────────────────────────────────────────────── - const ghLabelStore = createMetadataRequestStore() const ghAssigneeStore = createMetadataRequestStore() @@ -45,44 +35,22 @@ export function useRepoLabels( repoPath: string | null, repoId?: string | null, options?: GitHubMetadataOptions -): MetadataState { - const [state, setState] = useState>({ - data: [], - loading: false, - error: null - }) - const activeKeyRef = useRef(null) +): MetadataListState { + const runtimeEnvironmentId = + options?.runtimeEnvironmentId?.trim() || options?.activeRuntimeEnvironmentId?.trim() || null + const repoSelector = repoId ?? repoPath ?? '' + const cacheKey = + repoPath || repoId + ? runtimeEnvironmentId + ? `runtime:${runtimeEnvironmentId}:${repoSelector}` + : repoSelector + : null - useEffect(() => { - if (!repoPath && !repoId) { - return - } - const runtimeEnvironmentId = - options?.runtimeEnvironmentId?.trim() || options?.activeRuntimeEnvironmentId?.trim() || null - const repoSelector = repoId ?? repoPath ?? '' - // Why: SSH/runtime metadata must not reuse host-path cache entries; the same - // repo id may resolve through a different credential/runtime boundary. - const cacheKey = runtimeEnvironmentId - ? `runtime:${runtimeEnvironmentId}:${repoSelector}` - : repoSelector - const cached = getFreshMetadata(ghLabelStore, cacheKey) - if (cached) { - if (activeKeyRef.current !== cacheKey) { - setState({ data: cached.data, loading: false, error: null }) - activeKeyRef.current = cacheKey - } - return - } - - activeKeyRef.current = cacheKey - const requestKey = cacheKey - setState((s) => ({ - ...s, - data: s.data.length ? ([] as typeof s.data) : s.data, - loading: true, - error: null - })) - loadMetadata(ghLabelStore, cacheKey, () => + return useMetadataListRequest({ + cacheKey, + store: ghLabelStore, + errorFallback: 'Failed to load labels', + load: () => runtimeEnvironmentId ? callRuntimeRpc( { kind: 'environment', environmentId: runtimeEnvironmentId }, @@ -93,71 +61,29 @@ export function useRepoLabels( : window.api.gh .listLabels({ repoPath: repoPath ?? '', repoId: repoId ?? undefined }) .then((labels) => labels as string[]) - ) - .then((data) => { - if (activeKeyRef.current !== requestKey) { - return - } - setState({ data, loading: false, error: null }) - }) - .catch((err) => { - if (activeKeyRef.current !== requestKey) { - return - } - activeKeyRef.current = null - setState((s) => ({ - ...s, - loading: false, - error: err instanceof Error ? err.message : 'Failed to load labels' - })) - }) - }, [repoPath, repoId, options?.runtimeEnvironmentId, options?.activeRuntimeEnvironmentId]) - - return state + }) } export function useRepoAssignees( repoPath: string | null, repoId?: string | null, options?: GitHubMetadataOptions -): MetadataState { - const [state, setState] = useState>({ - data: [], - loading: false, - error: null - }) - const activeKeyRef = useRef(null) +): MetadataListState { + const runtimeEnvironmentId = + options?.runtimeEnvironmentId?.trim() || options?.activeRuntimeEnvironmentId?.trim() || null + const repoSelector = repoId ?? repoPath ?? '' + const cacheKey = + repoPath || repoId + ? runtimeEnvironmentId + ? `runtime:${runtimeEnvironmentId}:${repoSelector}` + : repoSelector + : null - useEffect(() => { - if (!repoPath && !repoId) { - return - } - const runtimeEnvironmentId = - options?.runtimeEnvironmentId?.trim() || options?.activeRuntimeEnvironmentId?.trim() || null - const repoSelector = repoId ?? repoPath ?? '' - // Why: SSH/runtime metadata must not reuse host-path cache entries; the same - // repo id may resolve through a different credential/runtime boundary. - const cacheKey = runtimeEnvironmentId - ? `runtime:${runtimeEnvironmentId}:${repoSelector}` - : repoSelector - const cached = getFreshMetadata(ghAssigneeStore, cacheKey) - if (cached) { - if (activeKeyRef.current !== cacheKey) { - setState({ data: cached.data, loading: false, error: null }) - activeKeyRef.current = cacheKey - } - return - } - - activeKeyRef.current = cacheKey - const requestKey = cacheKey - setState((s) => ({ - ...s, - data: s.data.length ? ([] as typeof s.data) : s.data, - loading: true, - error: null - })) - loadMetadata(ghAssigneeStore, cacheKey, () => + return useMetadataListRequest({ + cacheKey, + store: ghAssigneeStore, + errorFallback: 'Failed to load assignees', + load: () => runtimeEnvironmentId ? callRuntimeRpc( { kind: 'environment', environmentId: runtimeEnvironmentId }, @@ -168,31 +94,9 @@ export function useRepoAssignees( : window.api.gh .listAssignableUsers({ repoPath: repoPath ?? '', repoId: repoId ?? undefined }) .then((users) => users as GitHubAssignableUser[]) - ) - .then((data) => { - if (activeKeyRef.current !== requestKey) { - return - } - setState({ data, loading: false, error: null }) - }) - .catch((err) => { - if (activeKeyRef.current !== requestKey) { - return - } - activeKeyRef.current = null - setState((s) => ({ - ...s, - loading: false, - error: err instanceof Error ? err.message : 'Failed to load assignees' - })) - }) - }, [repoPath, repoId, options?.runtimeEnvironmentId, options?.activeRuntimeEnvironmentId]) - - return state + }) } -// ─── Linear ──────────────────────────────────────────────── - const linearStateStore = createMetadataRequestStore() const linearLabelStore = createMetadataRequestStore() const linearMemberStore = createMetadataRequestStore() @@ -221,205 +125,44 @@ export function useTeamStates( teamId: string | null, settings?: RuntimeLinearSettings, workspaceId?: string | null -): MetadataState { - const [state, setState] = useState>({ - data: [], - loading: false, - error: null +): MetadataListState { + const selectedTeamId = teamId ?? '' + return useMetadataListRequest({ + cacheKey: selectedTeamId ? linearMetadataCacheKey(selectedTeamId, settings, workspaceId) : null, + store: linearStateStore, + load: () => loadTeamStates(settings, selectedTeamId, workspaceId), + errorFallback: 'Failed to load states' }) - const activeKeyRef = useRef(null) - // Why: parents can pass a fresh settings object each render; keying the effect - // on the derived cache key keeps a failure's setState from re-arming the fetch - // in a render-paced loop. The ref carries the latest settings for the call. - const settingsRef = useRef(settings) - settingsRef.current = settings - const cacheKey = teamId ? linearMetadataCacheKey(teamId, settings, workspaceId) : null - - useEffect(() => { - if (!teamId || !cacheKey) { - return - } - - const cached = getFreshMetadata(linearStateStore, cacheKey) - if (cached) { - if (activeKeyRef.current !== cacheKey) { - setState({ data: cached.data, loading: false, error: null }) - activeKeyRef.current = cacheKey - } - return - } - - activeKeyRef.current = cacheKey - const requestKey = cacheKey - setState((s) => ({ - ...s, - data: s.data.length ? ([] as typeof s.data) : s.data, - loading: true, - error: null - })) - loadMetadata(linearStateStore, cacheKey, () => - linearTeamStates(settingsRef.current, teamId, workspaceId).then( - (states) => states as LinearWorkflowState[] - ) - ) - .then((data) => { - if (activeKeyRef.current !== requestKey) { - return - } - setState({ data, loading: false, error: null }) - }) - .catch((err) => { - if (activeKeyRef.current !== requestKey) { - return - } - activeKeyRef.current = null - setState((s) => ({ - ...s, - loading: false, - error: err instanceof Error ? err.message : 'Failed to load states' - })) - }) - }, [cacheKey, teamId, workspaceId]) - - return state } export function useTeamLabels( teamId: string | null, settings?: RuntimeLinearSettings, workspaceId?: string | null -): MetadataState { - const [state, setState] = useState>({ - data: [], - loading: false, - error: null +): MetadataListState { + const selectedTeamId = teamId ?? '' + return useMetadataListRequest({ + cacheKey: selectedTeamId ? linearMetadataCacheKey(selectedTeamId, settings, workspaceId) : null, + store: linearLabelStore, + load: () => loadTeamLabels(settings, selectedTeamId, workspaceId), + errorFallback: 'Failed to load labels' }) - const activeKeyRef = useRef(null) - // Why: see useTeamStates — cache-key deps + latest-settings ref stop the - // failure-setState render loop. - const settingsRef = useRef(settings) - settingsRef.current = settings - const cacheKey = teamId ? linearMetadataCacheKey(teamId, settings, workspaceId) : null - - useEffect(() => { - if (!teamId || !cacheKey) { - return - } - - const cached = getFreshMetadata(linearLabelStore, cacheKey) - if (cached) { - if (activeKeyRef.current !== cacheKey) { - setState({ data: cached.data, loading: false, error: null }) - activeKeyRef.current = cacheKey - } - return - } - - activeKeyRef.current = cacheKey - const requestKey = cacheKey - setState((s) => ({ - ...s, - data: s.data.length ? ([] as typeof s.data) : s.data, - loading: true, - error: null - })) - loadMetadata(linearLabelStore, cacheKey, () => - linearTeamLabels(settingsRef.current, teamId, workspaceId).then( - (labels) => labels as LinearLabel[] - ) - ) - .then((data) => { - if (activeKeyRef.current !== requestKey) { - return - } - setState({ data, loading: false, error: null }) - }) - .catch((err) => { - if (activeKeyRef.current !== requestKey) { - return - } - activeKeyRef.current = null - setState((s) => ({ - ...s, - loading: false, - error: err instanceof Error ? err.message : 'Failed to load labels' - })) - }) - }, [cacheKey, teamId, workspaceId]) - - return state } export function useTeamMembers( teamId: string | null, settings?: RuntimeLinearSettings, workspaceId?: string | null -): MetadataState { - const [state, setState] = useState>({ - data: [], - loading: false, - error: null +): MetadataListState { + const selectedTeamId = teamId ?? '' + return useMetadataListRequest({ + cacheKey: selectedTeamId ? linearMetadataCacheKey(selectedTeamId, settings, workspaceId) : null, + store: linearMemberStore, + load: () => loadTeamMembers(settings, selectedTeamId, workspaceId), + errorFallback: 'Failed to load members' }) - const activeKeyRef = useRef(null) - // Why: see useTeamStates — cache-key deps + latest-settings ref stop the - // failure-setState render loop. - const settingsRef = useRef(settings) - settingsRef.current = settings - const cacheKey = teamId ? linearMetadataCacheKey(teamId, settings, workspaceId) : null - - useEffect(() => { - if (!teamId || !cacheKey) { - return - } - - const cached = getFreshMetadata(linearMemberStore, cacheKey) - if (cached) { - if (activeKeyRef.current !== cacheKey) { - setState({ data: cached.data, loading: false, error: null }) - activeKeyRef.current = cacheKey - } - return - } - - activeKeyRef.current = cacheKey - const requestKey = cacheKey - setState((s) => ({ - ...s, - data: s.data.length ? ([] as typeof s.data) : s.data, - loading: true, - error: null - })) - loadMetadata(linearMemberStore, cacheKey, () => - linearTeamMembers(settingsRef.current, teamId, workspaceId).then( - (members) => members as LinearMember[] - ) - ) - .then((data) => { - if (activeKeyRef.current !== requestKey) { - return - } - setState({ data, loading: false, error: null }) - }) - .catch((err) => { - if (activeKeyRef.current !== requestKey) { - return - } - activeKeyRef.current = null - setState((s) => ({ - ...s, - loading: false, - error: err instanceof Error ? err.message : 'Failed to load members' - })) - }) - }, [cacheKey, teamId, workspaceId]) - - return state } -/** - * Load Linear team metadata for every selected team and union by id (#8739). - * Reuses the same per-team cache stores as the single-team hooks. - */ function useTeamsMetadataList( teamIds: readonly string[], settings: RuntimeLinearSettings | undefined, @@ -431,8 +174,8 @@ function useTeamsMetadataList( workspaceId: string | null | undefined ) => Promise, errorFallback: string -): MetadataState { - const [state, setState] = useState>({ +): MetadataListState { + const [state, setState] = useState>({ data: [], loading: false, error: null @@ -448,7 +191,6 @@ function useTeamsMetadataList( [teamIdsKey] ) - // Why: recompute each render (like useTeamStates cacheKey) so runtime target changes re-key. const requestKey = stableTeamIds.length === 0 ? null @@ -464,22 +206,12 @@ function useTeamsMetadataList( } activeKeyRef.current = requestKey - const capturedKey = requestKey - const teams = stableTeamIds - - // Fast path: every team still fresh in cache → union synchronously. - const cachedGroups: T[][] = [] - let allCached = true - for (const teamId of teams) { - const cacheKey = linearMetadataCacheKey(teamId, settingsRef.current, workspaceId) - const cached = getFreshMetadata(store, cacheKey) - if (!cached) { - allCached = false - break - } - cachedGroups.push(cached.data) - } - if (allCached) { + const cachedGroups = stableTeamIds.map( + (teamId) => + getFreshMetadata(store, linearMetadataCacheKey(teamId, settingsRef.current, workspaceId)) + ?.data + ) + if (cachedGroups.every((group): group is T[] => group !== undefined)) { setState({ data: unionLinearMetadataById(cachedGroups), loading: false, error: null }) return } @@ -492,7 +224,7 @@ function useTeamsMetadataList( })) void Promise.all( - teams.map((teamId) => { + stableTeamIds.map((teamId) => { const cacheKey = linearMetadataCacheKey(teamId, settingsRef.current, workspaceId) return loadMetadata(store, cacheKey, () => loadTeam(settingsRef.current, teamId, workspaceId) @@ -500,7 +232,7 @@ function useTeamsMetadataList( }) ) .then((groups) => { - if (activeKeyRef.current !== capturedKey) { + if (activeKeyRef.current !== requestKey) { return } setState({ @@ -510,7 +242,7 @@ function useTeamsMetadataList( }) }) .catch((err) => { - if (activeKeyRef.current !== capturedKey) { + if (activeKeyRef.current !== requestKey) { return } activeKeyRef.current = null @@ -546,12 +278,11 @@ const loadTeamMembers = ( ): Promise => linearTeamMembers(settings, teamId, workspaceId).then((members) => members as LinearMember[]) -/** Union of workflow states for every selected Linear team (multi-team filters). */ export function useTeamsStates( teamIds: readonly string[], settings?: RuntimeLinearSettings, workspaceId?: string | null -): MetadataState { +): MetadataListState { return useTeamsMetadataList( teamIds, settings, @@ -562,12 +293,11 @@ export function useTeamsStates( ) } -/** Union of labels for every selected Linear team (multi-team filters). */ export function useTeamsLabels( teamIds: readonly string[], settings?: RuntimeLinearSettings, workspaceId?: string | null -): MetadataState { +): MetadataListState { return useTeamsMetadataList( teamIds, settings, @@ -578,12 +308,11 @@ export function useTeamsLabels( ) } -/** Union of members for every selected Linear team (multi-team filters). */ export function useTeamsMembers( teamIds: readonly string[], settings?: RuntimeLinearSettings, workspaceId?: string | null -): MetadataState { +): MetadataListState { return useTeamsMetadataList( teamIds, settings, diff --git a/src/renderer/src/hooks/useMetadataListRequest.test.tsx b/src/renderer/src/hooks/useMetadataListRequest.test.tsx new file mode 100644 index 00000000000..3e0c6a68055 --- /dev/null +++ b/src/renderer/src/hooks/useMetadataListRequest.test.tsx @@ -0,0 +1,68 @@ +// @vitest-environment happy-dom + +import { act, cleanup, renderHook } from '@testing-library/react' +import { afterEach, describe, expect, it } from 'vitest' +import { createMetadataRequestStore } from './metadata-request-cache' +import { useMetadataListRequest } from './useMetadataListRequest' + +function deferred(): { promise: Promise; resolve: (value: T) => void } { + let resolve!: (value: T) => void + const promise = new Promise((settle) => { + resolve = settle + }) + return { promise, resolve } +} + +afterEach(cleanup) + +describe('useMetadataListRequest', () => { + it('treats only null as disabled', async () => { + const store = createMetadataRequestStore() + let loads = 0 + const view = renderHook(() => + useMetadataListRequest({ + cacheKey: '', + store, + errorFallback: 'Failed to load metadata', + load: async () => { + loads += 1 + return ['loaded'] + } + }) + ) + + await act(() => Promise.resolve()) + expect(loads).toBe(1) + expect(view.result.current.data).toEqual(['loaded']) + }) + + it('ignores stale responses and retains the result while disabled', async () => { + const store = createMetadataRequestStore() + const first = deferred() + const second = deferred() + const view = renderHook( + ({ cacheKey }: { cacheKey: string | null }) => + useMetadataListRequest({ + cacheKey, + store, + errorFallback: 'Failed to load metadata', + load: () => (cacheKey === 'first' ? first.promise : second.promise) + }), + { initialProps: { cacheKey: 'first' as string | null } } + ) + + view.rerender({ cacheKey: 'second' }) + await act(async () => { + second.resolve(['second']) + await second.promise + }) + expect(view.result.current.data).toEqual(['second']) + + view.rerender({ cacheKey: null }) + await act(async () => { + first.resolve(['first']) + await first.promise + }) + expect(view.result.current).toEqual({ data: ['second'], loading: false, error: null }) + }) +}) diff --git a/src/renderer/src/hooks/useMetadataListRequest.ts b/src/renderer/src/hooks/useMetadataListRequest.ts new file mode 100644 index 00000000000..186b620d6cd --- /dev/null +++ b/src/renderer/src/hooks/useMetadataListRequest.ts @@ -0,0 +1,72 @@ +import { useEffect, useEffectEvent, useRef, useState } from 'react' +import { getFreshMetadata, loadMetadata, type MetadataRequestStore } from './metadata-request-cache' + +export type MetadataListState = { + data: T[] + loading: boolean + error: string | null +} + +type MetadataListRequest = { + cacheKey: string | null + store: MetadataRequestStore + load: () => Promise + errorFallback: string +} + +export function useMetadataListRequest({ + cacheKey, + store, + load, + errorFallback +}: MetadataListRequest): MetadataListState { + const [state, setState] = useState>({ + data: [], + loading: false, + error: null + }) + const activeKeyRef = useRef(null) + const loadLatest = useEffectEvent(load) + + useEffect(() => { + if (cacheKey === null) { + return + } + + const cached = getFreshMetadata(store, cacheKey) + if (cached) { + if (activeKeyRef.current !== cacheKey) { + setState({ data: cached.data, loading: false, error: null }) + } + activeKeyRef.current = cacheKey + return + } + activeKeyRef.current = cacheKey + const requestKey = cacheKey + setState((current) => ({ + ...current, + data: current.data.length ? [] : current.data, + loading: true, + error: null + })) + void loadMetadata(store, cacheKey, () => loadLatest()) + .then((data) => { + if (activeKeyRef.current === requestKey) { + setState({ data, loading: false, error: null }) + } + }) + .catch((error: unknown) => { + if (activeKeyRef.current !== requestKey) { + return + } + activeKeyRef.current = null + setState((current) => ({ + ...current, + loading: false, + error: error instanceof Error ? error.message : errorFallback + })) + }) + }, [cacheKey, errorFallback, store]) + + return state +}