mirror of
https://github.com/stablyai/orca.git
synced 2026-09-30 00:03:15 +00:00
perf: reset github checks tab state during render (#4287)
This commit is contained in:
@@ -85,6 +85,14 @@ import {
|
||||
import type { DiffSection } from '@/components/editor/diff-section-types'
|
||||
import type { CombinedDiffFileTreeEntry } from '@/components/editor/combined-diff-file-tree-model'
|
||||
import { CHECK_COLOR, CHECK_ICON } from '@/components/right-sidebar/checks-panel-content'
|
||||
import {
|
||||
createGitHubChecksTabState,
|
||||
resolveGitHubChecksTabState,
|
||||
toggleGitHubChecksTabExpandedKey,
|
||||
updateGitHubChecksTabDetails,
|
||||
updateGitHubChecksTabLocalChecks,
|
||||
type CheckDetailsLoadState
|
||||
} from '@/components/github-checks-tab-state'
|
||||
import {
|
||||
filterPRCommentsByAudience,
|
||||
getPRCommentAudienceCounts,
|
||||
@@ -151,7 +159,6 @@ import type {
|
||||
GitBranchChangeEntry,
|
||||
GitDiffResult,
|
||||
PRCheckDetail,
|
||||
PRCheckRunDetails,
|
||||
PRComment,
|
||||
TuiAgent
|
||||
} from '../../../shared/types'
|
||||
@@ -3271,12 +3278,6 @@ function pickDefaultAgent(
|
||||
return AGENT_CATALOG.find((entry) => enabledAgents.includes(entry.id))?.id ?? null
|
||||
}
|
||||
|
||||
type CheckDetailsLoadState = {
|
||||
loading: boolean
|
||||
details: PRCheckRunDetails | null
|
||||
error: string | null
|
||||
}
|
||||
|
||||
function getCheckDetailsKey(check: PRCheckDetail): string {
|
||||
return String(check.checkRunId ?? check.workflowRunId ?? check.url ?? check.name)
|
||||
}
|
||||
@@ -3316,15 +3317,18 @@ function ChecksTab({
|
||||
variant?: 'compact' | 'page'
|
||||
onChecksUpdated: (checks: PRCheckDetail[]) => void
|
||||
}): React.JSX.Element {
|
||||
const [localChecks, setLocalChecks] = useState<PRCheckDetail[] | null>(null)
|
||||
const [refreshing, setRefreshing] = useState(false)
|
||||
const [rerunning, setRerunning] = useState(false)
|
||||
const [fixingChecks, setFixingChecks] = useState(false)
|
||||
const [expandedCheckKey, setExpandedCheckKey] = useState<string | null>(null)
|
||||
const [detailsByCheckKey, setDetailsByCheckKey] = useState<Record<string, CheckDetailsLoadState>>(
|
||||
{}
|
||||
)
|
||||
const [checksState, setChecksState] = useState(() => createGitHubChecksTabState(checks))
|
||||
const mountedRef = useMountedRef()
|
||||
const resolvedChecksState = resolveGitHubChecksTabState(checksState, checks)
|
||||
if (resolvedChecksState !== checksState) {
|
||||
// Why: parent check refreshes replace the source list; clear local refresh
|
||||
// and inline detail state before stale rows/details can paint.
|
||||
setChecksState(resolvedChecksState)
|
||||
}
|
||||
const { localChecks, expandedCheckKey, detailsByCheckKey } = resolvedChecksState
|
||||
const list = useMemo(() => localChecks ?? checks ?? [], [checks, localChecks])
|
||||
const prRepo = useMemo(() => parseOwnerRepoFromItemUrl(item.url), [item.url])
|
||||
const sorted = [...list].sort(
|
||||
@@ -3353,12 +3357,6 @@ function ChecksTab({
|
||||
: 'text-muted-foreground'
|
||||
const canFixBrokenChecks = Boolean((repoId ?? item.repoId) && failedChecks.length > 0)
|
||||
|
||||
useEffect(() => {
|
||||
setLocalChecks(null)
|
||||
setExpandedCheckKey(null)
|
||||
setDetailsByCheckKey({})
|
||||
}, [checks])
|
||||
|
||||
const handleRefresh = useCallback(async (): Promise<PRCheckDetail[] | null> => {
|
||||
if (!repoPath) {
|
||||
toast.error('Unable to refresh checks without a repository path.')
|
||||
@@ -3373,7 +3371,7 @@ function ChecksTab({
|
||||
headSha,
|
||||
noCache: true
|
||||
})) as PRCheckDetail[]
|
||||
setLocalChecks(nextChecks)
|
||||
setChecksState((current) => updateGitHubChecksTabLocalChecks(current, nextChecks))
|
||||
onChecksUpdated(nextChecks)
|
||||
return nextChecks
|
||||
} catch (err) {
|
||||
@@ -3493,7 +3491,7 @@ function ChecksTab({
|
||||
const handleToggleCheckDetails = useCallback(
|
||||
(check: PRCheckDetail): void => {
|
||||
const key = getCheckDetailsKey(check)
|
||||
setExpandedCheckKey((current) => (current === key ? null : key))
|
||||
setChecksState((current) => toggleGitHubChecksTabExpandedKey(current, key))
|
||||
if (
|
||||
!repoPath ||
|
||||
detailsByCheckKey[key] ||
|
||||
@@ -3501,10 +3499,9 @@ function ChecksTab({
|
||||
) {
|
||||
return
|
||||
}
|
||||
setDetailsByCheckKey((current) => ({
|
||||
...current,
|
||||
[key]: { loading: true, details: null, error: null }
|
||||
}))
|
||||
setChecksState((current) =>
|
||||
updateGitHubChecksTabDetails(current, key, { loading: true, details: null, error: null })
|
||||
)
|
||||
void window.api.gh
|
||||
.prCheckDetails({
|
||||
repoPath,
|
||||
@@ -3519,27 +3516,25 @@ function ChecksTab({
|
||||
if (!mountedRef.current) {
|
||||
return
|
||||
}
|
||||
setDetailsByCheckKey((current) => ({
|
||||
...current,
|
||||
[key]: {
|
||||
setChecksState((current) =>
|
||||
updateGitHubChecksTabDetails(current, key, {
|
||||
loading: false,
|
||||
details,
|
||||
error: details ? null : 'No inline details are available for this check.'
|
||||
}
|
||||
}))
|
||||
})
|
||||
)
|
||||
})
|
||||
.catch((err) => {
|
||||
if (!mountedRef.current) {
|
||||
return
|
||||
}
|
||||
setDetailsByCheckKey((current) => ({
|
||||
...current,
|
||||
[key]: {
|
||||
setChecksState((current) =>
|
||||
updateGitHubChecksTabDetails(current, key, {
|
||||
loading: false,
|
||||
details: null,
|
||||
error: err instanceof Error ? err.message : 'Failed to load check details.'
|
||||
}
|
||||
}))
|
||||
})
|
||||
)
|
||||
})
|
||||
},
|
||||
[detailsByCheckKey, mountedRef, prRepo, repoId, repoPath]
|
||||
|
||||
@@ -84,6 +84,14 @@ import {
|
||||
import type { DiffSection } from '@/components/editor/diff-section-types'
|
||||
import type { CombinedDiffFileTreeEntry } from '@/components/editor/combined-diff-file-tree-model'
|
||||
import { CHECK_COLOR, CHECK_ICON } from '@/components/right-sidebar/checks-panel-content'
|
||||
import {
|
||||
createGitHubChecksTabState,
|
||||
resolveGitHubChecksTabState,
|
||||
toggleGitHubChecksTabExpandedKey,
|
||||
updateGitHubChecksTabDetails,
|
||||
updateGitHubChecksTabLocalChecks,
|
||||
type CheckDetailsLoadState
|
||||
} from '@/components/github-checks-tab-state'
|
||||
import {
|
||||
filterPRCommentsByAudience,
|
||||
getPRCommentAudienceCounts,
|
||||
@@ -147,7 +155,6 @@ import type {
|
||||
GitBranchChangeEntry,
|
||||
GitDiffResult,
|
||||
PRCheckDetail,
|
||||
PRCheckRunDetails,
|
||||
PRComment,
|
||||
TuiAgent
|
||||
} from '../../../shared/types'
|
||||
@@ -3525,12 +3532,6 @@ function pickDefaultAgent(
|
||||
return AGENT_CATALOG.find((entry) => enabledAgents.includes(entry.id))?.id ?? null
|
||||
}
|
||||
|
||||
type CheckDetailsLoadState = {
|
||||
loading: boolean
|
||||
details: PRCheckRunDetails | null
|
||||
error: string | null
|
||||
}
|
||||
|
||||
function getCheckDetailsKey(check: PRCheckDetail): string {
|
||||
return String(check.checkRunId ?? check.workflowRunId ?? check.url ?? check.name)
|
||||
}
|
||||
@@ -3570,15 +3571,18 @@ function ChecksTab({
|
||||
variant?: 'compact' | 'page'
|
||||
onChecksUpdated: (checks: PRCheckDetail[]) => void
|
||||
}): React.JSX.Element {
|
||||
const [localChecks, setLocalChecks] = useState<PRCheckDetail[] | null>(null)
|
||||
const [refreshing, setRefreshing] = useState(false)
|
||||
const [rerunning, setRerunning] = useState(false)
|
||||
const [fixingChecks, setFixingChecks] = useState(false)
|
||||
const [expandedCheckKey, setExpandedCheckKey] = useState<string | null>(null)
|
||||
const [detailsByCheckKey, setDetailsByCheckKey] = useState<Record<string, CheckDetailsLoadState>>(
|
||||
{}
|
||||
)
|
||||
const [checksState, setChecksState] = useState(() => createGitHubChecksTabState(checks))
|
||||
const mountedRef = useMountedRef()
|
||||
const resolvedChecksState = resolveGitHubChecksTabState(checksState, checks)
|
||||
if (resolvedChecksState !== checksState) {
|
||||
// Why: parent check refreshes replace the source list; clear local refresh
|
||||
// and inline detail state before stale rows/details can paint.
|
||||
setChecksState(resolvedChecksState)
|
||||
}
|
||||
const { localChecks, expandedCheckKey, detailsByCheckKey } = resolvedChecksState
|
||||
const list = useMemo(() => localChecks ?? checks ?? [], [checks, localChecks])
|
||||
const prRepo = useMemo(() => parseOwnerRepoFromItemUrl(item.url), [item.url])
|
||||
const sorted = [...list].sort(
|
||||
@@ -3607,12 +3611,6 @@ function ChecksTab({
|
||||
: 'text-muted-foreground'
|
||||
const canFixBrokenChecks = Boolean((repoId ?? item.repoId) && failedChecks.length > 0)
|
||||
|
||||
useEffect(() => {
|
||||
setLocalChecks(null)
|
||||
setExpandedCheckKey(null)
|
||||
setDetailsByCheckKey({})
|
||||
}, [checks])
|
||||
|
||||
const handleRefresh = useCallback(async (): Promise<PRCheckDetail[] | null> => {
|
||||
if (!repoPath) {
|
||||
toast.error('Unable to refresh checks without a repository path.')
|
||||
@@ -3627,7 +3625,7 @@ function ChecksTab({
|
||||
headSha,
|
||||
noCache: true
|
||||
})) as PRCheckDetail[]
|
||||
setLocalChecks(nextChecks)
|
||||
setChecksState((current) => updateGitHubChecksTabLocalChecks(current, nextChecks))
|
||||
onChecksUpdated(nextChecks)
|
||||
return nextChecks
|
||||
} catch (err) {
|
||||
@@ -3747,7 +3745,7 @@ function ChecksTab({
|
||||
const handleToggleCheckDetails = useCallback(
|
||||
(check: PRCheckDetail): void => {
|
||||
const key = getCheckDetailsKey(check)
|
||||
setExpandedCheckKey((current) => (current === key ? null : key))
|
||||
setChecksState((current) => toggleGitHubChecksTabExpandedKey(current, key))
|
||||
if (
|
||||
!repoPath ||
|
||||
detailsByCheckKey[key] ||
|
||||
@@ -3755,10 +3753,9 @@ function ChecksTab({
|
||||
) {
|
||||
return
|
||||
}
|
||||
setDetailsByCheckKey((current) => ({
|
||||
...current,
|
||||
[key]: { loading: true, details: null, error: null }
|
||||
}))
|
||||
setChecksState((current) =>
|
||||
updateGitHubChecksTabDetails(current, key, { loading: true, details: null, error: null })
|
||||
)
|
||||
void window.api.gh
|
||||
.prCheckDetails({
|
||||
repoPath,
|
||||
@@ -3773,27 +3770,25 @@ function ChecksTab({
|
||||
if (!mountedRef.current) {
|
||||
return
|
||||
}
|
||||
setDetailsByCheckKey((current) => ({
|
||||
...current,
|
||||
[key]: {
|
||||
setChecksState((current) =>
|
||||
updateGitHubChecksTabDetails(current, key, {
|
||||
loading: false,
|
||||
details,
|
||||
error: details ? null : 'No inline details are available for this check.'
|
||||
}
|
||||
}))
|
||||
})
|
||||
)
|
||||
})
|
||||
.catch((err) => {
|
||||
if (!mountedRef.current) {
|
||||
return
|
||||
}
|
||||
setDetailsByCheckKey((current) => ({
|
||||
...current,
|
||||
[key]: {
|
||||
setChecksState((current) =>
|
||||
updateGitHubChecksTabDetails(current, key, {
|
||||
loading: false,
|
||||
details: null,
|
||||
error: err instanceof Error ? err.message : 'Failed to load check details.'
|
||||
}
|
||||
}))
|
||||
})
|
||||
)
|
||||
})
|
||||
},
|
||||
[detailsByCheckKey, mountedRef, prRepo, repoId, repoPath]
|
||||
|
||||
@@ -0,0 +1,63 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import type { PRCheckDetail } from '../../../shared/types'
|
||||
import {
|
||||
createGitHubChecksTabState,
|
||||
resolveGitHubChecksTabState,
|
||||
toggleGitHubChecksTabExpandedKey,
|
||||
updateGitHubChecksTabDetails,
|
||||
updateGitHubChecksTabLocalChecks
|
||||
} from './github-checks-tab-state'
|
||||
|
||||
const check = (name: string): PRCheckDetail => ({
|
||||
name,
|
||||
status: 'completed',
|
||||
conclusion: 'success',
|
||||
url: null
|
||||
})
|
||||
|
||||
describe('github checks tab state', () => {
|
||||
it('preserves local check state while the source checks reference is unchanged', () => {
|
||||
const sourceChecks = [check('unit')]
|
||||
const state = updateGitHubChecksTabLocalChecks(createGitHubChecksTabState(sourceChecks), [
|
||||
check('refreshed')
|
||||
])
|
||||
|
||||
expect(resolveGitHubChecksTabState(state, sourceChecks)).toBe(state)
|
||||
})
|
||||
|
||||
it('resets local checks and expanded details when source checks change', () => {
|
||||
const oldSource = [check('old')]
|
||||
const nextSource = [check('next')]
|
||||
const stateWithDetails = updateGitHubChecksTabDetails(
|
||||
toggleGitHubChecksTabExpandedKey(
|
||||
updateGitHubChecksTabLocalChecks(createGitHubChecksTabState(oldSource), [check('local')]),
|
||||
'unit'
|
||||
),
|
||||
'unit',
|
||||
{ loading: true, details: null, error: null }
|
||||
)
|
||||
|
||||
expect(resolveGitHubChecksTabState(stateWithDetails, nextSource)).toEqual({
|
||||
sourceChecks: nextSource,
|
||||
localChecks: null,
|
||||
expandedCheckKey: null,
|
||||
detailsByCheckKey: {}
|
||||
})
|
||||
})
|
||||
|
||||
it('toggles expanded check keys without discarding loaded details', () => {
|
||||
const sourceChecks = [check('unit')]
|
||||
const state = updateGitHubChecksTabDetails(createGitHubChecksTabState(sourceChecks), 'unit', {
|
||||
loading: false,
|
||||
details: null,
|
||||
error: 'No details'
|
||||
})
|
||||
|
||||
const expanded = toggleGitHubChecksTabExpandedKey(state, 'unit')
|
||||
const collapsed = toggleGitHubChecksTabExpandedKey(expanded, 'unit')
|
||||
|
||||
expect(expanded.expandedCheckKey).toBe('unit')
|
||||
expect(collapsed.expandedCheckKey).toBeNull()
|
||||
expect(collapsed.detailsByCheckKey).toBe(state.detailsByCheckKey)
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,66 @@
|
||||
import type { PRCheckDetail, PRCheckRunDetails } from '../../../shared/types'
|
||||
|
||||
export type CheckDetailsLoadState = {
|
||||
loading: boolean
|
||||
details: PRCheckRunDetails | null
|
||||
error: string | null
|
||||
}
|
||||
|
||||
export type GitHubChecksTabState = {
|
||||
sourceChecks: GitHubChecksSource
|
||||
localChecks: PRCheckDetail[] | null
|
||||
expandedCheckKey: string | null
|
||||
detailsByCheckKey: Record<string, CheckDetailsLoadState>
|
||||
}
|
||||
|
||||
type GitHubChecksSource = readonly PRCheckDetail[] | null | undefined
|
||||
|
||||
export function createGitHubChecksTabState(sourceChecks: GitHubChecksSource): GitHubChecksTabState {
|
||||
return {
|
||||
sourceChecks,
|
||||
localChecks: null,
|
||||
expandedCheckKey: null,
|
||||
detailsByCheckKey: {}
|
||||
}
|
||||
}
|
||||
|
||||
export function resolveGitHubChecksTabState(
|
||||
state: GitHubChecksTabState,
|
||||
sourceChecks: GitHubChecksSource
|
||||
): GitHubChecksTabState {
|
||||
return state.sourceChecks === sourceChecks ? state : createGitHubChecksTabState(sourceChecks)
|
||||
}
|
||||
|
||||
export function updateGitHubChecksTabLocalChecks(
|
||||
state: GitHubChecksTabState,
|
||||
localChecks: PRCheckDetail[]
|
||||
): GitHubChecksTabState {
|
||||
return {
|
||||
...state,
|
||||
localChecks
|
||||
}
|
||||
}
|
||||
|
||||
export function toggleGitHubChecksTabExpandedKey(
|
||||
state: GitHubChecksTabState,
|
||||
key: string
|
||||
): GitHubChecksTabState {
|
||||
return {
|
||||
...state,
|
||||
expandedCheckKey: state.expandedCheckKey === key ? null : key
|
||||
}
|
||||
}
|
||||
|
||||
export function updateGitHubChecksTabDetails(
|
||||
state: GitHubChecksTabState,
|
||||
key: string,
|
||||
details: CheckDetailsLoadState
|
||||
): GitHubChecksTabState {
|
||||
return {
|
||||
...state,
|
||||
detailsByCheckKey: {
|
||||
...state.detailsByCheckKey,
|
||||
[key]: details
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user