mirror of
https://github.com/stablyai/orca.git
synced 2026-10-01 00:02:10 +00:00
perf(source-control): sort branch entries before filtering, gate projections by view mode
Two dead-work fixes in the Source Control file projection. 1. filterAndSortSourceControlPathEntries copied and re-sorted the uncapped branch entry list with Intl.Collator on every keystroke. Sort once on branchEntries, filter after: Array#filter preserves order and compareFileNames is a total order, so filter(sort(x)) === sort(filter(x)). 2. The tree projection was built in list mode and the list projection in tree mode, then discarded. Gate each memo on sourceControlViewMode and return a shared empty projection, matching the combined-diff file tree precedent.
This commit is contained in:
@@ -1,7 +1,6 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import {
|
||||
SOURCE_CONTROL_FILE_FILTER_QUERY_MAX_BYTES,
|
||||
filterAndSortSourceControlPathEntries,
|
||||
filterSourceControlGroupedPathEntries,
|
||||
filterSourceControlPathEntries,
|
||||
getSourceControlFileFilterState,
|
||||
@@ -10,26 +9,6 @@ import {
|
||||
} from './source-control/listing/file-filter'
|
||||
|
||||
describe('source-control-file-filter', () => {
|
||||
it('naturally orders committed branch rows without mutating store input', () => {
|
||||
const entries = [
|
||||
{ path: 'migrations/100.sql' },
|
||||
{ path: 'migrations/9.sql' },
|
||||
{ path: 'migrations/99.sql' }
|
||||
]
|
||||
|
||||
expect(
|
||||
filterAndSortSourceControlPathEntries(entries, {
|
||||
normalizedFilter: '',
|
||||
tooLarge: false
|
||||
}).map((entry) => entry.path)
|
||||
).toEqual(['migrations/9.sql', 'migrations/99.sql', 'migrations/100.sql'])
|
||||
expect(entries.map((entry) => entry.path)).toEqual([
|
||||
'migrations/100.sql',
|
||||
'migrations/9.sql',
|
||||
'migrations/99.sql'
|
||||
])
|
||||
})
|
||||
|
||||
it('normalizes bounded queries and filters entries by path', () => {
|
||||
const filter = getSourceControlFileFilterState(' SRC/button ')
|
||||
|
||||
|
||||
@@ -1,5 +1,4 @@
|
||||
import { isClipboardTextByteLengthOverLimit } from '../../../../../../shared/clipboard-text'
|
||||
import { compareFileNames } from '../../../../../../shared/file-name-sort'
|
||||
|
||||
export const SOURCE_CONTROL_FILE_FILTER_QUERY_MAX_BYTES = 2 * 1024
|
||||
|
||||
@@ -49,15 +48,6 @@ export function filterSourceControlPathEntries<T extends SourceControlPathEntry>
|
||||
return entries.filter((entry) => entry.path.toLowerCase().includes(filter.normalizedFilter))
|
||||
}
|
||||
|
||||
export function filterAndSortSourceControlPathEntries<T extends SourceControlPathEntry>(
|
||||
entries: T[],
|
||||
filter: SourceControlFileFilterState
|
||||
): T[] {
|
||||
return [...filterSourceControlPathEntries(entries, filter)].sort((a, b) =>
|
||||
compareFileNames(a.path, b.path)
|
||||
)
|
||||
}
|
||||
|
||||
export function filterSourceControlGroupedPathEntries<T extends SourceControlPathEntry>(
|
||||
grouped: SourceControlGroupedPathEntries<T>,
|
||||
filter: SourceControlFileFilterState
|
||||
|
||||
+275
@@ -0,0 +1,275 @@
|
||||
// @vitest-environment happy-dom
|
||||
|
||||
import { renderHook } from '@testing-library/react'
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { GitBranchChangeEntry } from '../../../../../../shared/git-diff-compare-types'
|
||||
import type { GitStatusEntry } from '../../../../../../shared/git-status-types'
|
||||
import type { SourceControlViewMode } from '../../../../../../shared/ui-chrome-types'
|
||||
import type * as FileNameSortModule from '../../../../../../shared/file-name-sort'
|
||||
import type * as SourceControlTreeModule from '../../source-control-tree'
|
||||
import type * as SubmoduleExpansionModule from './submodule-expansion'
|
||||
|
||||
const counters = vi.hoisted(() => ({
|
||||
compareFileNames: 0,
|
||||
buildGitStatusSourceControlTree: 0,
|
||||
buildSourceControlTree: 0,
|
||||
flattenSourceControlTree: 0,
|
||||
injectExpandedSubmoduleRows: 0,
|
||||
injectExpandedSubmoduleEntries: 0
|
||||
}))
|
||||
|
||||
vi.mock('../../../../../../shared/file-name-sort', async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof FileNameSortModule>()
|
||||
return {
|
||||
...actual,
|
||||
compareFileNames: (a: string, b: string) => {
|
||||
counters.compareFileNames += 1
|
||||
return actual.compareFileNames(a, b)
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
vi.mock('../../source-control-tree', async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof SourceControlTreeModule>()
|
||||
return {
|
||||
...actual,
|
||||
buildGitStatusSourceControlTree: (
|
||||
...args: Parameters<typeof actual.buildGitStatusSourceControlTree>
|
||||
) => {
|
||||
counters.buildGitStatusSourceControlTree += 1
|
||||
return actual.buildGitStatusSourceControlTree(...args)
|
||||
},
|
||||
buildSourceControlTree: ((...args: unknown[]) => {
|
||||
counters.buildSourceControlTree += 1
|
||||
return (actual.buildSourceControlTree as (...a: unknown[]) => unknown)(...args)
|
||||
}) as typeof actual.buildSourceControlTree,
|
||||
flattenSourceControlTree: ((...args: unknown[]) => {
|
||||
counters.flattenSourceControlTree += 1
|
||||
return (actual.flattenSourceControlTree as (...a: unknown[]) => unknown)(...args)
|
||||
}) as typeof actual.flattenSourceControlTree
|
||||
}
|
||||
})
|
||||
|
||||
vi.mock('./submodule-expansion', async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof SubmoduleExpansionModule>()
|
||||
return {
|
||||
...actual,
|
||||
injectExpandedSubmoduleRows: ((...args: unknown[]) => {
|
||||
counters.injectExpandedSubmoduleRows += 1
|
||||
return (actual.injectExpandedSubmoduleRows as (...a: unknown[]) => unknown)(...args)
|
||||
}) as typeof actual.injectExpandedSubmoduleRows,
|
||||
injectExpandedSubmoduleEntries: ((...args: unknown[]) => {
|
||||
counters.injectExpandedSubmoduleEntries += 1
|
||||
return (actual.injectExpandedSubmoduleEntries as (...a: unknown[]) => unknown)(...args)
|
||||
}) as typeof actual.injectExpandedSubmoduleEntries
|
||||
}
|
||||
})
|
||||
|
||||
const { compareFileNames } = await import('../../../../../../shared/file-name-sort')
|
||||
const { getSourceControlFileFilterState, filterSourceControlPathEntries } =
|
||||
await import('./file-filter')
|
||||
const { useSourceControlFileProjection } = await import('./use-file-projection')
|
||||
|
||||
const NO_ENTRIES: GitStatusEntry[] = []
|
||||
const NO_COLLAPSED_TREE_DIRS = new Set<string>()
|
||||
const NO_EXPANDED_SUBMODULES = new Set<string>()
|
||||
const NO_COLLAPSED_SECTIONS = new Set<string>()
|
||||
const NO_SUBMODULE_STATUS = {}
|
||||
const GROUP_ORDER = ['unstaged', 'staged', 'untracked'] as const
|
||||
|
||||
type ProjectionProps = {
|
||||
entries: GitStatusEntry[]
|
||||
branchEntries: GitBranchChangeEntry[]
|
||||
filterQuery: string
|
||||
sourceControlViewMode: SourceControlViewMode
|
||||
}
|
||||
|
||||
function renderProjection(initialProps: ProjectionProps) {
|
||||
return renderHook(
|
||||
(props: ProjectionProps) =>
|
||||
useSourceControlFileProjection({
|
||||
entries: props.entries,
|
||||
branchEntries: props.branchEntries,
|
||||
filterQuery: props.filterQuery,
|
||||
sourceControlGroupOrder: GROUP_ORDER,
|
||||
activeWorktreeId: 'wt-1',
|
||||
worktreePath: '/repo',
|
||||
isFolder: false,
|
||||
collapsedTreeDirs: NO_COLLAPSED_TREE_DIRS,
|
||||
expandedSubmoduleKeys: NO_EXPANDED_SUBMODULES,
|
||||
submoduleStatusByKey: NO_SUBMODULE_STATUS,
|
||||
sourceControlViewMode: props.sourceControlViewMode,
|
||||
collapsedSections: NO_COLLAPSED_SECTIONS
|
||||
}),
|
||||
{ initialProps }
|
||||
)
|
||||
}
|
||||
|
||||
function makeBranchEntries(count: number): GitBranchChangeEntry[] {
|
||||
return Array.from({ length: count }, (_, index) => ({
|
||||
path: `src/area-${index % 7}/nested/deep-${index % 13}/file-${index}.ts`,
|
||||
status: 'modified' as const
|
||||
}))
|
||||
}
|
||||
|
||||
/** Numeric collation, case variants, unicode, collator ties, and an exact duplicate path. */
|
||||
const ORDERING_FIXTURE: GitBranchChangeEntry[] = [
|
||||
{ path: 'migrations/100.sql', status: 'modified' },
|
||||
{ path: 'migrations/9.sql', status: 'modified' },
|
||||
{ path: 'migrations/99.sql', status: 'added' },
|
||||
{ path: 'migrations/02.sql', status: 'modified' },
|
||||
{ path: 'migrations/2.sql', status: 'deleted' },
|
||||
{ path: 'src/Button.tsx', status: 'modified' },
|
||||
{ path: 'src/button.tsx', status: 'added' },
|
||||
{ path: 'src/éclair.ts', status: 'modified' },
|
||||
{ path: 'src/eclair.ts', status: 'deleted' },
|
||||
{ path: 'src/Éclair.ts', status: 'modified' },
|
||||
{ path: 'src/日本語.ts', status: 'added' },
|
||||
{ path: 'src/dup.ts', status: 'modified' },
|
||||
{ path: 'src/dup.ts', status: 'added' }
|
||||
]
|
||||
|
||||
/** The pre-change implementation: filter, then copy-and-sort. */
|
||||
function legacyFilterThenSort(
|
||||
entries: GitBranchChangeEntry[],
|
||||
filterQuery: string
|
||||
): GitBranchChangeEntry[] {
|
||||
const state = getSourceControlFileFilterState(filterQuery)
|
||||
return [...filterSourceControlPathEntries(entries, state)].sort((a, b) =>
|
||||
compareFileNames(a.path, b.path)
|
||||
)
|
||||
}
|
||||
|
||||
function makeStatusEntries(count: number): GitStatusEntry[] {
|
||||
return Array.from({ length: count }, (_, index) => ({
|
||||
path: `src/area-${index % 7}/nested/deep-${index % 13}/file-${index}.ts`,
|
||||
status: 'modified' as const,
|
||||
area: (['unstaged', 'staged', 'untracked'] as const)[index % 3]
|
||||
}))
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
for (const key of Object.keys(counters) as (keyof typeof counters)[]) {
|
||||
counters[key] = 0
|
||||
}
|
||||
})
|
||||
|
||||
describe('useSourceControlFileProjection branch entry ordering', () => {
|
||||
it('sorts committed branch entries once across many filter changes', () => {
|
||||
const branchEntries = makeBranchEntries(400)
|
||||
const { rerender } = renderProjection({
|
||||
entries: NO_ENTRIES,
|
||||
branchEntries,
|
||||
filterQuery: '',
|
||||
sourceControlViewMode: 'list'
|
||||
})
|
||||
|
||||
const comparesForInitialSort = counters.compareFileNames
|
||||
expect(comparesForInitialSort).toBeGreaterThan(0)
|
||||
|
||||
for (const filterQuery of ['f', 'fi', 'fil', 'file', 'file-', 'file-1']) {
|
||||
rerender({ entries: NO_ENTRIES, branchEntries, filterQuery, sourceControlViewMode: 'list' })
|
||||
}
|
||||
|
||||
expect(counters.compareFileNames).toBe(comparesForInitialSort)
|
||||
})
|
||||
|
||||
it('produces the same order as the previous filter-then-sort for every filter', () => {
|
||||
const { result, rerender } = renderProjection({
|
||||
entries: NO_ENTRIES,
|
||||
branchEntries: ORDERING_FIXTURE,
|
||||
filterQuery: '',
|
||||
sourceControlViewMode: 'list'
|
||||
})
|
||||
|
||||
for (const filterQuery of ['', 'src', 'MIGRATIONS', 'é', '9', 'dup', 'no-match']) {
|
||||
rerender({
|
||||
entries: NO_ENTRIES,
|
||||
branchEntries: ORDERING_FIXTURE,
|
||||
filterQuery,
|
||||
sourceControlViewMode: 'list'
|
||||
})
|
||||
expect(result.current.filteredBranchEntries).toEqual(
|
||||
legacyFilterThenSort(ORDERING_FIXTURE, filterQuery)
|
||||
)
|
||||
}
|
||||
})
|
||||
|
||||
it('does not mutate the store-owned branch entry array', () => {
|
||||
const branchEntries = [...ORDERING_FIXTURE]
|
||||
renderProjection({
|
||||
entries: NO_ENTRIES,
|
||||
branchEntries,
|
||||
filterQuery: '',
|
||||
sourceControlViewMode: 'list'
|
||||
})
|
||||
|
||||
expect(branchEntries).toEqual(ORDERING_FIXTURE)
|
||||
})
|
||||
})
|
||||
|
||||
describe('useSourceControlFileProjection view-mode gating', () => {
|
||||
const entries = makeStatusEntries(120)
|
||||
const branchEntries = makeBranchEntries(120)
|
||||
|
||||
it('builds no tree projection in list mode', () => {
|
||||
const { result } = renderProjection({
|
||||
entries,
|
||||
branchEntries,
|
||||
filterQuery: '',
|
||||
sourceControlViewMode: 'list'
|
||||
})
|
||||
|
||||
expect(counters.buildGitStatusSourceControlTree).toBe(0)
|
||||
expect(counters.buildSourceControlTree).toBe(0)
|
||||
expect(counters.flattenSourceControlTree).toBe(0)
|
||||
expect(counters.injectExpandedSubmoduleRows).toBe(0)
|
||||
expect(counters.injectExpandedSubmoduleEntries).toBeGreaterThan(0)
|
||||
expect(result.current.visibleTreeRowsBySection).toEqual({})
|
||||
expect(result.current.visibleBranchTreeRows).toEqual([])
|
||||
})
|
||||
|
||||
it('builds no list projection in tree mode', () => {
|
||||
const { result } = renderProjection({
|
||||
entries,
|
||||
branchEntries,
|
||||
filterQuery: '',
|
||||
sourceControlViewMode: 'tree'
|
||||
})
|
||||
|
||||
expect(counters.injectExpandedSubmoduleEntries).toBe(0)
|
||||
expect(counters.buildGitStatusSourceControlTree).toBeGreaterThan(0)
|
||||
expect(counters.buildSourceControlTree).toBeGreaterThan(0)
|
||||
expect(result.current.visibleListRowsBySection).toEqual({})
|
||||
})
|
||||
|
||||
it('has the other mode fully projected on the first render after a switch', () => {
|
||||
const { result, rerender } = renderProjection({
|
||||
entries,
|
||||
branchEntries,
|
||||
filterQuery: '',
|
||||
sourceControlViewMode: 'list'
|
||||
})
|
||||
const listRows = result.current.visibleListRowsBySection
|
||||
const listSelectionCount = result.current.visibleSelectionEntries.length
|
||||
expect(listSelectionCount).toBe(entries.length)
|
||||
|
||||
rerender({ entries, branchEntries, filterQuery: '', sourceControlViewMode: 'tree' })
|
||||
|
||||
expect(result.current.visibleListRowsBySection).toEqual({})
|
||||
expect(result.current.visibleBranchTreeRows.length).toBeGreaterThan(0)
|
||||
expect(
|
||||
Object.values(result.current.visibleTreeRowsBySection).reduce(
|
||||
(total, rows) => total + rows.length,
|
||||
0
|
||||
)
|
||||
).toBeGreaterThan(0)
|
||||
expect(result.current.visibleSelectionEntries.length).toBe(listSelectionCount)
|
||||
|
||||
rerender({ entries, branchEntries, filterQuery: '', sourceControlViewMode: 'list' })
|
||||
|
||||
expect(result.current.visibleTreeRowsBySection).toEqual({})
|
||||
expect(result.current.visibleBranchTreeRows).toEqual([])
|
||||
expect(result.current.visibleListRowsBySection).toEqual(listRows)
|
||||
})
|
||||
})
|
||||
+45
-9
@@ -2,10 +2,11 @@ import { useMemo } from 'react'
|
||||
import type { GitBranchChangeEntry } from '../../../../../../shared/git-diff-compare-types'
|
||||
import type { GitStatusEntry } from '../../../../../../shared/git-status-types'
|
||||
import type { SourceControlViewMode } from '../../../../../../shared/ui-chrome-types'
|
||||
import { compareFileNames } from '../../../../../../shared/file-name-sort'
|
||||
import { compareGitStatusEntries } from '../../source-control-status-sort'
|
||||
import {
|
||||
filterAndSortSourceControlPathEntries,
|
||||
filterSourceControlGroupedPathEntries,
|
||||
filterSourceControlPathEntries,
|
||||
getSourceControlFileFilterState,
|
||||
type SourceControlFileFilterState
|
||||
} from './file-filter'
|
||||
@@ -60,6 +61,19 @@ export type SourceControlFileProjection = {
|
||||
visibleSelectionEntries: FlatEntry[]
|
||||
}
|
||||
|
||||
// Why: only one view mode is ever rendered, so building the other mode's projection is pure dead
|
||||
// work (precedent: the combined-diff file tree short-circuits the same way while collapsed).
|
||||
const EMPTY_TREE_ROOTS_BY_SECTION: Readonly<
|
||||
Partial<Record<SourceControlDisplaySectionId, GitStatusSourceControlTreeNode[]>>
|
||||
> = Object.freeze({})
|
||||
const EMPTY_TREE_ROWS_BY_SECTION: Readonly<
|
||||
Partial<Record<SourceControlDisplaySectionId, RenderableSourceControlNode[]>>
|
||||
> = Object.freeze({})
|
||||
const EMPTY_LIST_ROWS_BY_SECTION: Readonly<
|
||||
Partial<Record<SourceControlDisplaySectionId, RenderableSubmoduleListItem[]>>
|
||||
> = Object.freeze({})
|
||||
const EMPTY_BRANCH_TREE_NODES: SourceControlTreeNode<GitBranchChangeEntry, 'branch'>[] = []
|
||||
|
||||
export function useSourceControlFileProjection({
|
||||
entries,
|
||||
branchEntries,
|
||||
@@ -127,12 +141,21 @@ export function useSourceControlFileProjection({
|
||||
[unfilteredDisplaySections]
|
||||
)
|
||||
|
||||
// Why: sorting before filtering keeps the collator off the keystroke path; Array#filter preserves
|
||||
// order and compareFileNames is a total order, so filter(sort(x)) === sort(filter(x)).
|
||||
const sortedBranchEntries = useMemo(
|
||||
() => [...branchEntries].sort((a, b) => compareFileNames(a.path, b.path)),
|
||||
[branchEntries]
|
||||
)
|
||||
const filteredBranchEntries = useMemo(
|
||||
() => filterAndSortSourceControlPathEntries(branchEntries, fileFilterState),
|
||||
[branchEntries, fileFilterState]
|
||||
() => filterSourceControlPathEntries(sortedBranchEntries, fileFilterState),
|
||||
[fileFilterState, sortedBranchEntries]
|
||||
)
|
||||
|
||||
const treeRootsBySection = useMemo(() => {
|
||||
if (sourceControlViewMode !== 'tree') {
|
||||
return EMPTY_TREE_ROOTS_BY_SECTION
|
||||
}
|
||||
const roots: Partial<Record<SourceControlDisplaySectionId, GitStatusSourceControlTreeNode[]>> =
|
||||
{}
|
||||
for (const section of displaySections) {
|
||||
@@ -148,9 +171,12 @@ export function useSourceControlFileProjection({
|
||||
: sectionRoots
|
||||
}
|
||||
return roots
|
||||
}, [displaySections])
|
||||
}, [displaySections, sourceControlViewMode])
|
||||
|
||||
const visibleTreeRowsBySection = useMemo(() => {
|
||||
if (sourceControlViewMode !== 'tree') {
|
||||
return EMPTY_TREE_ROWS_BY_SECTION
|
||||
}
|
||||
const rows: Partial<Record<SourceControlDisplaySectionId, RenderableSourceControlNode[]>> = {}
|
||||
for (const section of displaySections) {
|
||||
rows[section.id] = injectExpandedSubmoduleRows(
|
||||
@@ -167,11 +193,15 @@ export function useSourceControlFileProjection({
|
||||
displaySections,
|
||||
treeRootsBySection,
|
||||
expandedSubmoduleKeys,
|
||||
sourceControlViewMode,
|
||||
submoduleStatusByKey
|
||||
])
|
||||
|
||||
// List view needs the same lazy submodule expansion as tree view, spliced into the flat entry list.
|
||||
const visibleListRowsBySection = useMemo(() => {
|
||||
if (sourceControlViewMode !== 'list') {
|
||||
return EMPTY_LIST_ROWS_BY_SECTION
|
||||
}
|
||||
const rows: Partial<Record<SourceControlDisplaySectionId, RenderableSubmoduleListItem[]>> = {}
|
||||
for (const section of displaySections) {
|
||||
rows[section.id] = injectExpandedSubmoduleEntries(
|
||||
@@ -183,15 +213,21 @@ export function useSourceControlFileProjection({
|
||||
)
|
||||
}
|
||||
return rows
|
||||
}, [displaySections, expandedSubmoduleKeys, submoduleStatusByKey])
|
||||
}, [displaySections, expandedSubmoduleKeys, sourceControlViewMode, submoduleStatusByKey])
|
||||
|
||||
const branchTreeRoots = useMemo(
|
||||
() => compactSourceControlTree(buildSourceControlTree('branch', filteredBranchEntries)),
|
||||
[filteredBranchEntries]
|
||||
() =>
|
||||
sourceControlViewMode === 'tree'
|
||||
? compactSourceControlTree(buildSourceControlTree('branch', filteredBranchEntries))
|
||||
: EMPTY_BRANCH_TREE_NODES,
|
||||
[filteredBranchEntries, sourceControlViewMode]
|
||||
)
|
||||
const visibleBranchTreeRows = useMemo(
|
||||
() => flattenSourceControlTree(branchTreeRoots, collapsedTreeDirs),
|
||||
[branchTreeRoots, collapsedTreeDirs]
|
||||
() =>
|
||||
sourceControlViewMode === 'tree'
|
||||
? flattenSourceControlTree(branchTreeRoots, collapsedTreeDirs)
|
||||
: EMPTY_BRANCH_TREE_NODES,
|
||||
[branchTreeRoots, collapsedTreeDirs, sourceControlViewMode]
|
||||
)
|
||||
|
||||
const visibleSelectionEntries = useMemo(() => {
|
||||
|
||||
Reference in New Issue
Block a user