mirror of
https://github.com/stablyai/orca.git
synced 2026-10-03 08:02:12 +00:00
perf(diff): defer large diffs until user loads them
Prevents UI freeze when opening files with very large diffs by deferring render until the user explicitly loads them.
This commit is contained in:
@@ -9,12 +9,20 @@ import {
|
||||
COMBINED_DIFF_FILE_TREE_QUERY_MAX_BYTES,
|
||||
getCombinedDiffBranchEntriesInTreeOrder,
|
||||
getFilteredCombinedDiffFileTreeEntries,
|
||||
isCombinedDiffFileTreeQueryTooLarge
|
||||
isCombinedDiffFileTreeQueryTooLarge,
|
||||
isCombinedDiffSectionViewed
|
||||
} from './combined-diff-file-tree-model'
|
||||
import type { GitBranchChangeEntry } from '../../../../shared/git-diff-compare-types'
|
||||
import type { GitStatusEntry } from '../../../../shared/git-status-types'
|
||||
|
||||
describe('CombinedDiffFileTree navigation mapping', () => {
|
||||
it('does not count deferred sections as viewed', () => {
|
||||
expect(isCombinedDiffSectionViewed({ loading: false, loadOnDemand: true })).toBe(false)
|
||||
expect(isCombinedDiffSectionViewed({ loading: false, loadOnDemand: false })).toBe(true)
|
||||
expect(isCombinedDiffSectionViewed({ loading: false })).toBe(true)
|
||||
expect(isCombinedDiffSectionViewed({ loading: true, loadOnDemand: false })).toBe(false)
|
||||
})
|
||||
|
||||
it('disambiguates uncommitted entries with the same path by area', () => {
|
||||
const staged: GitStatusEntry = { path: 'src/App.tsx', status: 'modified', area: 'staged' }
|
||||
const unstaged: GitStatusEntry = { path: 'src/App.tsx', status: 'modified', area: 'unstaged' }
|
||||
|
||||
@@ -28,7 +28,8 @@ export {
|
||||
createCombinedDiffSectionIndexMap,
|
||||
getCombinedDiffFileTreeNavigationIndex,
|
||||
getCombinedDiffFileTreeSectionKey,
|
||||
handleCombinedDiffFileTreeNavigation
|
||||
handleCombinedDiffFileTreeNavigation,
|
||||
isCombinedDiffSectionViewed
|
||||
} from './combined-diff-file-tree-model'
|
||||
|
||||
const UNCOMMITTED_AREA_ORDER: readonly GitStagingArea[] = ['unstaged', 'staged', 'untracked']
|
||||
|
||||
@@ -52,7 +52,8 @@ import { DiffNotesSendMenu } from './DiffNotesSendMenu'
|
||||
import {
|
||||
CombinedDiffFileTree,
|
||||
createCombinedDiffSectionIndexMap,
|
||||
handleCombinedDiffFileTreeNavigation
|
||||
handleCombinedDiffFileTreeNavigation,
|
||||
isCombinedDiffSectionViewed
|
||||
} from './CombinedDiffFileTree'
|
||||
import { getCombinedDiffFileTreeSectionKey } from './combined-diff-file-tree-model'
|
||||
import {
|
||||
@@ -310,6 +311,13 @@ export default function CombinedDiffViewer({
|
||||
const deferredLoadRequestsRef = useRef<Set<number>>(new Set())
|
||||
const sectionsRef = useRef<DiffSection[]>([])
|
||||
const generationRef = useRef(0)
|
||||
// Why: status polling can replace the entries array without changing its
|
||||
// metadata; don't reinitialize and cancel an in-flight explicit diff load.
|
||||
const initializedEntryStateRef = useRef<{
|
||||
viewStateKey: string
|
||||
entrySignature: string
|
||||
hasUncommittedEntriesSnapshot: boolean
|
||||
} | null>(null)
|
||||
// Why: per-section reload token, so a sibling's reload can't discard this section's in-flight load.
|
||||
const sectionLoadTokensRef = useRef<Map<number, number>>(new Map())
|
||||
const renderedIndicesRef = useRef<Set<number>>(new Set())
|
||||
@@ -520,6 +528,19 @@ export default function CombinedDiffViewer({
|
||||
|
||||
// Why: tab/worktree switches unmount this viewer; cache by pane key so remount restores sections+scroll before repaint.
|
||||
useLayoutEffect(() => {
|
||||
const initializedEntryState = initializedEntryStateRef.current
|
||||
if (
|
||||
initializedEntryState?.viewStateKey === viewStateKey &&
|
||||
initializedEntryState.entrySignature === entrySignature &&
|
||||
initializedEntryState.hasUncommittedEntriesSnapshot === hasUncommittedEntriesSnapshot
|
||||
) {
|
||||
return
|
||||
}
|
||||
initializedEntryStateRef.current = {
|
||||
viewStateKey,
|
||||
entrySignature,
|
||||
hasUncommittedEntriesSnapshot
|
||||
}
|
||||
deferredLoadRequestsRef.current.clear()
|
||||
const cached = combinedDiffViewStateCache.get(viewStateKey)
|
||||
const canRestoreSnapshotSectionsByKey =
|
||||
@@ -1131,7 +1152,12 @@ export default function CombinedDiffViewer({
|
||||
setActiveTreeSectionState({ entrySignature, key: null })
|
||||
}
|
||||
const viewedSectionKeys = React.useMemo(
|
||||
() => new Set(sections.filter((section) => !section.loading).map((section) => section.key)),
|
||||
() =>
|
||||
new Set(
|
||||
sections
|
||||
.filter((section) => isCombinedDiffSectionViewed(section))
|
||||
.map((section) => section.key)
|
||||
),
|
||||
[sections]
|
||||
)
|
||||
const handleTreeNavigate = useCallback(
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { basename } from '@/lib/path'
|
||||
import type { GitBranchChangeEntry } from '../../../../shared/git-diff-compare-types'
|
||||
import type { GitStatusEntry } from '../../../../shared/git-status-types'
|
||||
import type { DiffSection } from './diff-section-types'
|
||||
import { isClipboardTextByteLengthOverLimit } from '../../../../shared/clipboard-text'
|
||||
import {
|
||||
buildSourceControlTree,
|
||||
@@ -38,6 +39,12 @@ export function createCombinedDiffSectionIndexMap(
|
||||
return new Map(sections.map((section, index) => [section.key, index]))
|
||||
}
|
||||
|
||||
export function isCombinedDiffSectionViewed(
|
||||
section: Pick<DiffSection, 'loading' | 'loadOnDemand'>
|
||||
): boolean {
|
||||
return !section.loading && section.loadOnDemand !== true
|
||||
}
|
||||
|
||||
export function getCombinedDiffFileTreeNavigationIndex({
|
||||
mode,
|
||||
entry,
|
||||
|
||||
@@ -26,4 +26,16 @@ describe('combined diff on-demand loading', () => {
|
||||
it('automatically loads diffs when line counts are unavailable', () => {
|
||||
expect(shouldLoadCombinedDiffOnDemand({ added: undefined, removed: undefined })).toBe(false)
|
||||
})
|
||||
|
||||
it('defers untracked diffs when only additions are reported', () => {
|
||||
expect(shouldLoadCombinedDiffOnDemand({ added: MAX_AUTOMATIC_DIFF_CHANGED_LINES + 1 })).toBe(
|
||||
true
|
||||
)
|
||||
})
|
||||
|
||||
it('defers diffs when only removals are reported', () => {
|
||||
expect(shouldLoadCombinedDiffOnDemand({ removed: MAX_AUTOMATIC_DIFF_CHANGED_LINES + 1 })).toBe(
|
||||
true
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -7,8 +7,10 @@ export function shouldLoadCombinedDiffOnDemand({
|
||||
added?: number
|
||||
removed?: number
|
||||
}): boolean {
|
||||
if (added === undefined || removed === undefined) {
|
||||
// Untracked text files report additions only; both fields are absent when
|
||||
// line counts are unavailable (for example, binary or oversized files).
|
||||
if (added === undefined && removed === undefined) {
|
||||
return false
|
||||
}
|
||||
return added + removed > MAX_AUTOMATIC_DIFF_CHANGED_LINES
|
||||
return (added ?? 0) + (removed ?? 0) > MAX_AUTOMATIC_DIFF_CHANGED_LINES
|
||||
}
|
||||
|
||||
@@ -14869,6 +14869,10 @@
|
||||
"directoryAccessRequestMultiple": "This preview wants to read files in {{folders}}.",
|
||||
"allowDirectories": "Allow {{count}} folders",
|
||||
"editAddressControl": "Edit address"
|
||||
},
|
||||
"LargeDiffLoadPrompt": {
|
||||
"a0af0198aa": "Large diffs are not rendered by default.",
|
||||
"f7fa7a40d0": "Load diff"
|
||||
}
|
||||
},
|
||||
"diff": {
|
||||
|
||||
@@ -87,14 +87,23 @@ test.describe('Large diff freeze repro', () => {
|
||||
if (!store) {
|
||||
throw new Error('window.__store is not available')
|
||||
}
|
||||
const status = await window.api.git.status({ worktreePath: repoPath })
|
||||
store.getState().setGitStatus(wId, status)
|
||||
const entry = status.entries.find(
|
||||
let status = await window.api.git.status({ worktreePath: repoPath })
|
||||
let entry = status.entries.find(
|
||||
(candidate) => candidate.path === relativePath && candidate.area === 'unstaged'
|
||||
)
|
||||
// Why: the app may still be settling the just-added worktree's first status read.
|
||||
const statusDeadline = performance.now() + 5_000
|
||||
while (!entry && performance.now() < statusDeadline) {
|
||||
await new Promise((resolve) => window.setTimeout(resolve, 100))
|
||||
status = await window.api.git.status({ worktreePath: repoPath })
|
||||
entry = status.entries.find(
|
||||
(candidate) => candidate.path === relativePath && candidate.area === 'unstaged'
|
||||
)
|
||||
}
|
||||
if (!entry) {
|
||||
throw new Error(`large diff status entry not found: ${relativePath}`)
|
||||
}
|
||||
store.getState().setGitStatus(wId, status)
|
||||
store.getState().openAllDiffs(wId, repoPath, undefined, 'unstaged', [entry])
|
||||
},
|
||||
{ wId: worktreeId, repoPath: fixture.repoPath, relativePath: fixture.relativePath }
|
||||
|
||||
Reference in New Issue
Block a user