perf(file-explorer): stop rebuilding the whole visible tree twice per directory refresh (#18319)

* perf(file-explorer): stop rebuilding the whole visible tree twice per directory refresh

The per-directory loading flag moves out of `dirCache` into a sibling
`Set<string>`, so a `dirCache` identity change now means "children changed".
Every identity change re-ran `getFileExplorerIgnoredQueryRelativePaths` (full
recursive walk) and `createVisibleFileExplorerRowProjection` (full flatten, new
Map, new array identity cascading into virtual rows, selection, keyboard nav and
the name filter) over the whole visible tree — and half of those rebuilds
produced a byte-identical row set.

Also in this change:
- `refreshFileExplorerExpandedDirs` no longer pre-marks every expanded dir in
  `dirCache`; the 13 progressive commits stay.
- `flushBatch` paces its `fs.stat` fanout at 8 (was up to 5,000 concurrent onto
  libuv's 4-thread pool), matching parcel-watcher-event-delivery.ts.
- The editor external-watch loop bails before allocating a notification for a
  path no open file matches.
- `createCachedDirPathIndex` is built lazily, only when a direct
  `dirPath in cache` lookup misses.

* fix(file-explorer): keep the loading-dirs ref out of the render body

React Doctor's no-ref-current-in-render flagged the render-body mirror, and it
was right: a render React discards would still have mutated the ref. The ref is
now authoritative and written only from callbacks, with one updater that moves
the ref and the state together.

Side effect, in the safe direction: loadDir's in-flight guard now sees a mark the
moment it is made instead of one commit later, so a second non-forced read of a
directory already being read is deduped rather than started and then superseded.
Forced reads (refreshDir, refreshTree) bypass the guard and are unaffected.

Also moves the in-flight check out of decideExpandedDirLoad and into the
expansion effect that owns the fan-out, restoring the two-argument signature.
This clears the no-pass-data-to-parent warning the three-argument call had
dragged onto a changed line, and it keeps the pure staleness decision pure.
This commit is contained in:
Neil
2026-09-02 23:19:59 -07:00
committed by GitHub
parent ddb13a10f7
commit 94be54d16c
27 changed files with 619 additions and 160 deletions
@@ -132,6 +132,44 @@ describe('local filesystem watcher flush serialization', () => {
expect(sender.send).not.toHaveBeenCalled()
})
it('caps concurrent stats at eight for a full batch and keeps result order', async () => {
const eventCount = 5_000
const paths = Array.from({ length: eventCount }, (_, index) => `/repo/file-${index}.ts`)
let inFlight = 0
let peakInFlight = 0
statMock.mockImplementation(async (statPath: string) => {
inFlight++
peakInFlight = Math.max(peakInFlight, inFlight)
await Promise.resolve()
inFlight--
return { isDirectory: () => statPath.endsWith('-0.ts') }
})
const root = await createLocalWatcher('/repo', '/repo')
root.listeners.set(1, sender as never)
watcherCallback?.(
null,
paths.map((path) => ({ type: 'update' as const, path }))
)
vi.advanceTimersByTime(WATCH_BATCH_TRAILING_MS)
// Why a loop, not a fixed microtask count: 5,000 stats through 8 lanes take many turns.
for (let i = 0; i < eventCount * 4 && sender.send.mock.calls.length === 0; i++) {
await Promise.resolve()
}
expect(statMock).toHaveBeenCalledTimes(eventCount)
expect(peakInFlight).toBe(8)
expect(sender.send).toHaveBeenCalledTimes(1)
const { events } = sender.send.mock.calls[0][1] as FsChangedPayload
expect(events).toEqual(
paths.map((path) => ({
kind: 'update',
absolutePath: path,
isDirectory: path.endsWith('-0.ts')
}))
)
})
it('leaves an open debounce window to the armed timer instead of draining early', async () => {
const firstStat = deferred<{ isDirectory: () => boolean }>()
const secondStat = deferred<{ isDirectory: () => boolean }>()
@@ -17,6 +17,10 @@ import {
trackDetachedLocalUnsubscribe
} from './filesystem-watcher-listener-lifecycle'
import { createDebouncedBatch } from './filesystem-watcher-batch-control'
import { mapWithConcurrency } from '../../shared/map-with-concurrency'
// Why: matches the watcher subprocess budget in parcel-watcher-event-delivery.ts.
const DIRECTORY_STAT_CONCURRENCY = 8
// ── Event coalescing ─────────────────────────────────────────────────
// Why: keep the last event per path in a flush window; delete→create emits both (delete cleans the subtree, create refreshes the parent), create→delete is dropped (§4.4).
@@ -128,8 +132,12 @@ async function flushBatch(root: WatchedRoot): Promise<void> {
const coalesced = coalesceEvents(rawEvents)
const events: FsChangeEvent[] = await Promise.all(
coalesced.map(async (evt) => {
// Why: a full batch is up to MAX_BATCHED_WATCHER_EVENTS paths; unbounded stat() would swamp
// libuv's 4-thread pool, which also serves git reads and persistence writes.
const events: FsChangeEvent[] = await mapWithConcurrency(
coalesced,
DIRECTORY_STAT_CONCURRENCY,
async (evt) => {
// Why: a deleted path can't be stat'd; leave isDirectory undefined and let the renderer infer from dirCache.
const isDirectory = evt.type === 'delete' ? undefined : await tryStatIsDirectory(evt.path)
@@ -138,7 +146,7 @@ async function flushBatch(root: WatchedRoot): Promise<void> {
absolutePath: evt.path,
isDirectory
}
})
}
)
if (root.batch.cancelled || root.listeners.size === 0) {
@@ -59,7 +59,7 @@ export function FileExplorerFilesTreePane({
handleExplorerBackgroundContextMenuCapture,
handleExplorerBackgroundDoubleClick
}: FileExplorerFilesTreePaneProps): React.JSX.Element {
const { dirCache, rootCache, rootError } = tree
const { loadingDirPaths, rootCache, rootError } = tree
const { selectedPaths, preserveSelectionForContextMenu, copyPathsForNode } = selection
const {
scrollRef,
@@ -111,8 +111,8 @@ export function FileExplorerFilesTreePane({
// when the tree is empty, still loading, or showing a read error.
const isEmptyState = visibleRowCount === 0 && !inlineInput
const isNameFilterLoading = nameFilterSource?.relativePaths === null
const isLoading =
isEmptyState && (hasNameFilter ? isNameFilterLoading : (rootCache?.loading ?? true))
const isRootLoading = !rootCache || (!!worktreePath && loadingDirPaths.has(worktreePath))
const isLoading = isEmptyState && (hasNameFilter ? isNameFilterLoading : isRootLoading)
const treeError = hasNameFilter ? nameFilterFiles.loadError : rootError
const hasError = isEmptyState && !isLoading && !!treeError
const showTree = !isEmptyState
@@ -177,7 +177,7 @@ export function FileExplorerFilesTreePane({
ignoredByRelativePath={ignoredByRelativePath}
expanded={rowExpandedPaths}
canCollapseFolderSubtree={!hasNameFilter}
dirCache={dirCache}
loadingDirPaths={loadingDirPaths}
selectedPaths={selectedPaths}
activeFileId={activeFileId}
flashingPath={flashingPath}
@@ -36,7 +36,7 @@ describe('FileExplorerRow collapse folder action', () => {
statusByRelativePath: new Map(),
ignoredByRelativePath: new Set(),
expanded: new Set([directoryNode.path]),
dirCache: {},
loadingDirPaths: new Set<string>(),
selectedPaths: new Set(),
activeFileId: null,
flashingPath: null,
@@ -89,7 +89,7 @@ describe('FileExplorerRow collapse folder action', () => {
statusByRelativePath: new Map(),
ignoredByRelativePath: new Set(),
expanded: new Set([directoryNode.path]),
dirCache: {},
loadingDirPaths: new Set<string>(),
selectedPaths: new Set(),
activeFileId: null,
flashingPath: null,
@@ -141,7 +141,7 @@ describe('FileExplorerRow collapse folder action', () => {
statusByRelativePath: new Map(),
ignoredByRelativePath: new Set(),
expanded: new Set(),
dirCache: {},
loadingDirPaths: new Set<string>(),
selectedPaths: new Set(),
activeFileId: null,
flashingPath: null,
@@ -194,7 +194,7 @@ describe('FileExplorerRow collapse folder action', () => {
statusByRelativePath: new Map(),
ignoredByRelativePath: new Set(),
expanded: new Set([directoryNode.path]),
dirCache: {},
loadingDirPaths: new Set<string>(),
selectedPaths: new Set(),
activeFileId: null,
flashingPath: null,
@@ -247,7 +247,7 @@ describe('FileExplorerRow collapse folder action', () => {
statusByRelativePath: new Map(),
ignoredByRelativePath: new Set(),
expanded: new Set(),
dirCache: {},
loadingDirPaths: new Set<string>(),
selectedPaths: new Set(),
activeFileId: null,
flashingPath: null,
@@ -6,7 +6,7 @@ import type { GitFileStatus } from '../../../../shared/git-status-types'
import { FileExplorerRow } from './FileExplorerRow'
import { InlineInputRow, type InlineInput } from './file-explorer-inline-input-row'
import { shouldShowIgnoredDecoration, STATUS_COLORS } from './status-display'
import type { DirCache, TreeNode } from './file-explorer-types'
import type { TreeNode } from './file-explorer-types'
import type { FileExplorerRowProjection } from './file-explorer-row-projection'
import type { RuntimeFileOperationArgs } from '@/runtime/runtime-file-client'
@@ -22,7 +22,7 @@ type FileExplorerVirtualRowsProps = {
ignoredByRelativePath: Set<string>
expanded: Set<string>
canCollapseFolderSubtree?: boolean
dirCache: Record<string, DirCache>
loadingDirPaths: ReadonlySet<string>
selectedPaths: Set<string>
activeFileId: string | null
flashingPath: string | null
@@ -69,7 +69,7 @@ export function FileExplorerVirtualRows(props: FileExplorerVirtualRowsProps): Re
ignoredByRelativePath,
expanded,
canCollapseFolderSubtree = true,
dirCache,
loadingDirPaths,
selectedPaths,
activeFileId,
flashingPath,
@@ -171,7 +171,7 @@ export function FileExplorerVirtualRows(props: FileExplorerVirtualRowsProps): Re
<FileExplorerRow
node={n}
isExpanded={expanded.has(n.path)}
isLoading={n.isDirectory && Boolean(dirCache[n.path]?.loading)}
isLoading={n.isDirectory && loadingDirPaths.has(n.path)}
isSelected={selectedPaths.has(n.path) || activeFileId === n.path}
selectedPaths={selectedPaths}
isFlashing={flashingPath === n.path}
@@ -63,7 +63,7 @@ describe('FileExplorerVirtualRows add-as-project action', () => {
statusByRelativePath: new Map(),
ignoredByRelativePath: new Set(),
expanded: new Set([directoryNode.path]),
dirCache: {},
loadingDirPaths: new Set<string>(),
selectedPaths: new Set(),
activeFileId: null,
flashingPath: null,
@@ -0,0 +1,76 @@
import type { DirCache } from './file-explorer-types'
/**
* Directories with a directory read in flight.
*
* Why this is not a `dirCache` field: every `dirCache` identity change re-walks and re-flattens the
* whole visible tree (the ignored-path query plus the row projection). A read that starts and a read
* that lands would each pay for that, and the first one commits a byte-identical row set because the
* spinner is the only thing that changed. Keeping the flag in a sibling set lets `dirCache` change
* only when `children` do.
*/
export const EMPTY_FILE_EXPLORER_LOADING_DIRS: ReadonlySet<string> = new Set<string>()
/**
* Applies one mark/clear to the loading set.
*
* Why not `Dispatch<SetStateAction<…>>`: the owner keeps the set in a ref as well as in state, and
* the ref must be current the moment a mark is made — a refresh wave marks every expanded dir
* before the render that would refresh a mirrored copy.
*/
export type FileExplorerLoadingDirsUpdater = (
update: (prev: ReadonlySet<string>) => ReadonlySet<string>
) => void
/** Returns `prev` unchanged when every path is already marked, so subscribers do not re-render. */
export function markFileExplorerDirsLoading(
prev: ReadonlySet<string>,
dirPaths: readonly string[]
): ReadonlySet<string> {
if (dirPaths.every((dirPath) => prev.has(dirPath))) {
return prev
}
const next = new Set(prev)
for (const dirPath of dirPaths) {
next.add(dirPath)
}
return next
}
/** Returns `prev` unchanged when no path was marked, so subscribers do not re-render. */
export function clearFileExplorerDirsLoading(
prev: ReadonlySet<string>,
dirPaths: readonly string[]
): ReadonlySet<string> {
if (!dirPaths.some((dirPath) => prev.has(dirPath))) {
return prev
}
const next = new Set(prev)
for (const dirPath of dirPaths) {
next.delete(dirPath)
}
return next.size === 0 ? EMPTY_FILE_EXPLORER_LOADING_DIRS : next
}
/**
* Adds an empty listing for dirs a read is about to populate for the first time.
*
* Why: a `dirCache` key is what tells the watcher reconciler that a path is a directory the
* Explorer tracks. Without the placeholder, a create/delete arriving while the very first read of
* that dir is still in flight resolves to no cached dir and is dropped, leaving the listing stale.
* Returns `prev` unchanged once every dir is known, which is the common case.
*/
export function withPendingFileExplorerDirCacheEntries(
prev: Record<string, DirCache>,
dirPaths: readonly string[]
): Record<string, DirCache> {
const missing = dirPaths.filter((dirPath) => prev[dirPath] === undefined)
if (missing.length === 0) {
return prev
}
const next = { ...prev }
for (const dirPath of missing) {
next[dirPath] = { children: [] }
}
return next
}
@@ -64,7 +64,7 @@ function virtualRowsElement(nodes: TreeNode[]): React.JSX.Element {
statusByRelativePath: new Map(),
ignoredByRelativePath: new Set(),
expanded: new Set(),
dirCache: {},
loadingDirPaths: new Set<string>(),
selectedPaths: new Set(),
activeFileId: null,
flashingPath: null,
@@ -4,30 +4,49 @@ import type { DirEntry } from '../../../../shared/filesystem-entry-types'
import type { DirCache } from './file-explorer-types'
import { createFileExplorerDirLoadTracker } from './file-explorer-dir-load-tracker'
import { refreshFileExplorerExpandedDirs } from './file-explorer-expanded-dirs-refresh'
import type { FileExplorerLoadingDirsUpdater } from './file-explorer-dir-load-state'
type CacheUpdate = SetStateAction<Record<string, DirCache>>
function createLoadingDirPathsRecorder(): {
updateLoadingDirPaths: FileExplorerLoadingDirsUpdater
isLoading: (dirPath: string) => boolean
} {
let loadingDirPaths: ReadonlySet<string> = new Set<string>()
return {
updateLoadingDirPaths: (update) => {
loadingDirPaths = update(loadingDirPaths)
},
isLoading: (dirPath: string) => loadingDirPaths.has(dirPath)
}
}
function entry(name: string, isDirectory = false): DirEntry {
return { name, isDirectory, isSymlink: false }
}
describe('refreshFileExplorerExpandedDirs', () => {
it('reloads expanded directories with one loading cache commit and one result cache commit', async () => {
it('rebuilds the dirCache identity once per refresh of already-cached dirs', async () => {
let cache: Record<string, DirCache> = {
'/repo': {
children: [
{ name: 'old', path: '/repo/old', relativePath: 'old', isDirectory: false, depth: 0 }
],
loading: false
]
},
'/repo/src': { children: [], loading: false },
'/repo/docs': { children: [], loading: false }
'/repo/src': { children: [] },
'/repo/docs': { children: [] }
}
// Why identities, not calls: React skips the re-render (and the row-projection rebuild) when a
// setState produces the same value, so only a new identity costs a full tree walk.
const committedCaches: Record<string, DirCache>[] = []
const setDirCache = vi.fn((update: CacheUpdate) => {
cache = typeof update === 'function' ? update(cache) : update
committedCaches.push(cache)
const next = typeof update === 'function' ? update(cache) : update
if (next !== cache) {
committedCaches.push(next)
}
cache = next
})
const { updateLoadingDirPaths, isLoading } = createLoadingDirPathsRecorder()
const readDirectory = vi.fn(async (dirPath: string) => {
const entriesByPath: Record<string, DirEntry[]> = {
'/repo/src': [entry('index.ts')],
@@ -44,22 +63,19 @@ describe('refreshFileExplorerExpandedDirs', () => {
worktreePath: '/repo',
dirLoadTracker: createFileExplorerDirLoadTracker(),
setDirCache,
updateLoadingDirPaths,
readDirectory,
// A limit at or above the dir count keeps one result batch.
maxConcurrentReads: 16
})
expect(refreshed).toBe(true)
expect(setDirCache).toHaveBeenCalledTimes(2)
expect(committedCaches).toHaveLength(1)
expect(isLoading('/repo/src')).toBe(false)
expect(isLoading('/repo/docs')).toBe(false)
expect(committedCaches[0]).toMatchObject({
'/repo': { loading: false, children: [{ name: 'old' }] },
'/repo/src': { loading: true },
'/repo/docs': { loading: true }
})
expect(committedCaches[1]).toMatchObject({
'/repo': { loading: false, children: [{ name: 'old' }] },
'/repo': { children: [{ name: 'old' }] },
'/repo/src': {
loading: false,
children: [
{
name: 'index.ts',
@@ -71,7 +87,6 @@ describe('refreshFileExplorerExpandedDirs', () => {
]
},
'/repo/docs': {
loading: false,
children: [
{
name: 'guide.md',
@@ -89,14 +104,14 @@ describe('refreshFileExplorerExpandedDirs', () => {
it('drops a superseded directory result so a newer concurrent load is not clobbered', async () => {
const tracker = createFileExplorerDirLoadTracker()
let cache: Record<string, DirCache> = {
'/repo/src': { children: [], loading: false },
'/repo/docs': { children: [], loading: false }
'/repo/src': { children: [] },
'/repo/docs': { children: [] }
}
const setDirCache = vi.fn((update: CacheUpdate) => {
cache = typeof update === 'function' ? update(cache) : update
})
const { updateLoadingDirPaths } = createLoadingDirPathsRecorder()
const newerSrcCache: DirCache = {
loading: true,
children: [
{
name: 'fresh.ts',
@@ -130,6 +145,7 @@ describe('refreshFileExplorerExpandedDirs', () => {
worktreePath: '/repo',
dirLoadTracker: tracker,
setDirCache,
updateLoadingDirPaths,
readDirectory,
maxConcurrentReads: 16
})
@@ -140,21 +156,19 @@ describe('refreshFileExplorerExpandedDirs', () => {
// dropped from the batched commit instead of clobbering fresher data.
expect(cache['/repo/src']).toEqual(newerSrcCache)
// The still-current dir is committed normally.
expect(cache['/repo/docs']).toMatchObject({
loading: false,
children: [{ name: 'guide.md' }]
})
expect(cache['/repo/docs']).toMatchObject({ children: [{ name: 'guide.md' }] })
})
it('drops a result superseded after its read resolved but before the batch commit', async () => {
const tracker = createFileExplorerDirLoadTracker()
let cache: Record<string, DirCache> = {
'/repo/src': { children: [], loading: false },
'/repo/docs': { children: [], loading: false }
'/repo/src': { children: [] },
'/repo/docs': { children: [] }
}
const setDirCache = vi.fn((update: CacheUpdate) => {
cache = typeof update === 'function' ? update(cache) : update
})
const { updateLoadingDirPaths } = createLoadingDirPathsRecorder()
let releaseDocs!: () => void
const docsGate = new Promise<void>((resolve) => {
releaseDocs = resolve
@@ -175,6 +189,7 @@ describe('refreshFileExplorerExpandedDirs', () => {
worktreePath: '/repo',
dirLoadTracker: tracker,
setDirCache,
updateLoadingDirPaths,
readDirectory,
maxConcurrentReads: 16
})
@@ -187,7 +202,6 @@ describe('refreshFileExplorerExpandedDirs', () => {
// the window between its resolved read and the final batched commit.
tracker.begin('/repo/src')
const newerSrcCache: DirCache = {
loading: false,
children: [
{
name: 'fresh.ts',
@@ -206,10 +220,7 @@ describe('refreshFileExplorerExpandedDirs', () => {
expect(refreshed).toBe(false)
// The stale /repo/src read must not clobber the newer committed cache.
expect(cache['/repo/src']).toEqual(newerSrcCache)
expect(cache['/repo/docs']).toMatchObject({
loading: false,
children: [{ name: 'guide.md' }]
})
expect(cache['/repo/docs']).toMatchObject({ children: [{ name: 'guide.md' }] })
})
it('never exceeds maxConcurrentReads in flight and still commits every directory', async () => {
@@ -221,6 +232,7 @@ describe('refreshFileExplorerExpandedDirs', () => {
const setDirCache = vi.fn((update: CacheUpdate) => {
cache = typeof update === 'function' ? update(cache) : update
})
const { updateLoadingDirPaths, isLoading } = createLoadingDirPathsRecorder()
let inFlight = 0
let peakInFlight = 0
const readDirectory = vi.fn(async (dirPath: string) => {
@@ -239,6 +251,7 @@ describe('refreshFileExplorerExpandedDirs', () => {
worktreePath: '/repo',
dirLoadTracker: createFileExplorerDirLoadTracker(),
setDirCache,
updateLoadingDirPaths,
readDirectory,
maxConcurrentReads: 4
})
@@ -249,10 +262,8 @@ describe('refreshFileExplorerExpandedDirs', () => {
// One up-front loading write plus one result write per completed group of four.
expect(setDirCache).toHaveBeenCalledTimes(6)
for (const { dirPath } of dirs) {
expect(cache[dirPath]).toMatchObject({
loading: false,
children: [{ name: expect.any(String) }]
})
expect(cache[dirPath]).toMatchObject({ children: [{ name: expect.any(String) }] })
expect(isLoading(dirPath)).toBe(false)
}
})
@@ -265,6 +276,7 @@ describe('refreshFileExplorerExpandedDirs', () => {
const setDirCache = vi.fn((update: CacheUpdate) => {
cache = typeof update === 'function' ? update(cache) : update
})
const { updateLoadingDirPaths, isLoading } = createLoadingDirPathsRecorder()
let releaseInitialReads!: () => void
const initialReadsGate = new Promise<void>((resolve) => {
releaseInitialReads = resolve
@@ -281,22 +293,23 @@ describe('refreshFileExplorerExpandedDirs', () => {
worktreePath: '/repo',
dirLoadTracker: createFileExplorerDirLoadTracker(),
setDirCache,
updateLoadingDirPaths,
readDirectory,
maxConcurrentReads: 3
})
await Promise.resolve()
expect(cache['/repo/d0']).toMatchObject({ loading: true })
expect(isLoading('/repo/d0')).toBe(true)
// A queued dir must already advertise loading:true, or FileExplorer's
// auto-load effect fans out an unbounded loadDir for it on the next
// `expanded` change — the reads this cap exists to bound.
expect(cache['/repo/d6']).toMatchObject({ loading: true })
expect(isLoading('/repo/d6')).toBe(true)
expect(readDirectory).toHaveBeenCalledTimes(3)
releaseInitialReads()
await refreshPromise
expect(cache['/repo/d6']).toMatchObject({ loading: false })
expect(isLoading('/repo/d6')).toBe(false)
})
it('starts later reads as slots free without waiting for the slowest initial read', async () => {
@@ -308,6 +321,7 @@ describe('refreshFileExplorerExpandedDirs', () => {
const setDirCache = vi.fn((update: CacheUpdate) => {
cache = typeof update === 'function' ? update(cache) : update
})
const { updateLoadingDirPaths, isLoading } = createLoadingDirPathsRecorder()
let releaseSlowRead!: () => void
const slowRead = new Promise<void>((resolve) => {
releaseSlowRead = resolve
@@ -324,6 +338,7 @@ describe('refreshFileExplorerExpandedDirs', () => {
worktreePath: '/repo',
dirLoadTracker: createFileExplorerDirLoadTracker(),
setDirCache,
updateLoadingDirPaths,
readDirectory,
maxConcurrentReads: 2
})
@@ -336,12 +351,12 @@ describe('refreshFileExplorerExpandedDirs', () => {
'/repo/d3',
'/repo/d4'
])
expect(cache['/repo/d1']).toMatchObject({ loading: false })
expect(cache['/repo/d0']).toMatchObject({ loading: true })
expect(isLoading('/repo/d1')).toBe(false)
expect(isLoading('/repo/d0')).toBe(true)
releaseSlowRead()
await expect(refreshPromise).resolves.toBe(true)
expect(cache['/repo/d0']).toMatchObject({ loading: false })
expect(isLoading('/repo/d0')).toBe(false)
})
it('does not turn a commit callback failure into an empty directory result', async () => {
@@ -349,6 +364,7 @@ describe('refreshFileExplorerExpandedDirs', () => {
const setDirCache = vi.fn((update: CacheUpdate) => {
cache = typeof update === 'function' ? update(cache) : update
})
const { updateLoadingDirPaths } = createLoadingDirPathsRecorder()
const commitError = new Error('commit failed')
await expect(
@@ -357,6 +373,7 @@ describe('refreshFileExplorerExpandedDirs', () => {
worktreePath: '/repo',
dirLoadTracker: createFileExplorerDirLoadTracker(),
setDirCache,
updateLoadingDirPaths,
readDirectory: async () => ({
entries: [entry('index.ts')],
operationOwner: { kind: 'local' as const }
@@ -369,10 +386,7 @@ describe('refreshFileExplorerExpandedDirs', () => {
).rejects.toBe(commitError)
expect(setDirCache).toHaveBeenCalledTimes(2)
expect(cache['/repo/src']).toMatchObject({
loading: false,
children: [{ name: 'index.ts' }]
})
expect(cache['/repo/src']).toMatchObject({ children: [{ name: 'index.ts' }] })
})
it('still notifies the rest of a commit batch after one commit callback throws', async () => {
@@ -380,6 +394,7 @@ describe('refreshFileExplorerExpandedDirs', () => {
const setDirCache = vi.fn((update: CacheUpdate) => {
cache = typeof update === 'function' ? update(cache) : update
})
const { updateLoadingDirPaths } = createLoadingDirPathsRecorder()
const commitError = new Error('commit failed')
const onDirCommitted = vi.fn((dirPath: string) => {
if (dirPath === '/repo/a') {
@@ -396,6 +411,7 @@ describe('refreshFileExplorerExpandedDirs', () => {
worktreePath: '/repo',
dirLoadTracker: createFileExplorerDirLoadTracker(),
setDirCache,
updateLoadingDirPaths,
readDirectory: async () => ({
entries: [entry('index.ts')],
operationOwner: { kind: 'local' as const }
@@ -410,7 +426,7 @@ describe('refreshFileExplorerExpandedDirs', () => {
'/repo/a',
'/repo/b'
])
expect(cache['/repo/b']).toMatchObject({ loading: false, children: [{ name: 'index.ts' }] })
expect(cache['/repo/b']).toMatchObject({ children: [{ name: 'index.ts' }] })
})
it('stops later batches after a commit callback throws', async () => {
@@ -418,6 +434,7 @@ describe('refreshFileExplorerExpandedDirs', () => {
const setDirCache = vi.fn((update: CacheUpdate) => {
cache = typeof update === 'function' ? update(cache) : update
})
const { updateLoadingDirPaths, isLoading } = createLoadingDirPathsRecorder()
const commitError = new Error('commit failed')
const onDirCommitted = vi.fn((dirPath: string) => {
if (dirPath === '/repo/a') {
@@ -431,6 +448,7 @@ describe('refreshFileExplorerExpandedDirs', () => {
worktreePath: '/repo',
dirLoadTracker: createFileExplorerDirLoadTracker(),
setDirCache,
updateLoadingDirPaths,
readDirectory: async () => ({
entries: [entry('index.ts')],
operationOwner: { kind: 'local' as const }
@@ -447,10 +465,11 @@ describe('refreshFileExplorerExpandedDirs', () => {
'/repo/a',
'/repo/b'
])
// One up-front loading write plus the single failed batch's result write.
// One up-front placeholder write plus the single failed batch's result write.
expect(setDirCache).toHaveBeenCalledTimes(2)
expect(cache['/repo/c']).toMatchObject({ loading: true })
expect(cache['/repo/d']).toMatchObject({ loading: true })
// Why not still loading: no read was ever started for these, so a spinner would never clear.
expect(isLoading('/repo/c')).toBe(false)
expect(isLoading('/repo/d')).toBe(false)
})
it('drops a queued directory superseded while an earlier read is blocked', async () => {
@@ -459,6 +478,7 @@ describe('refreshFileExplorerExpandedDirs', () => {
const setDirCache = vi.fn((update: CacheUpdate) => {
cache = typeof update === 'function' ? update(cache) : update
})
const { updateLoadingDirPaths } = createLoadingDirPathsRecorder()
let releaseFirst!: () => void
const firstGate = new Promise<void>((resolve) => {
releaseFirst = resolve
@@ -478,6 +498,7 @@ describe('refreshFileExplorerExpandedDirs', () => {
worktreePath: '/repo',
dirLoadTracker: tracker,
setDirCache,
updateLoadingDirPaths,
readDirectory,
maxConcurrentReads: 1
})
@@ -485,14 +506,14 @@ describe('refreshFileExplorerExpandedDirs', () => {
// A watcher-driven refreshDir supersedes the queued dir before it starts reading.
tracker.begin('/repo/b')
const newerBCache: DirCache = { loading: false, children: [] }
const newerBCache: DirCache = { children: [] }
setDirCache((prev) => ({ ...prev, '/repo/b': newerBCache }))
releaseFirst()
const refreshed = await refreshPromise
expect(refreshed).toBe(false)
expect(cache['/repo/a']).toMatchObject({ loading: false, children: [{ name: 'x.ts' }] })
expect(cache['/repo/a']).toMatchObject({ children: [{ name: 'x.ts' }] })
// The queued task must neither read nor commit the superseded dir.
expect(readDirectory).toHaveBeenCalledTimes(1)
expect(cache['/repo/b']).toEqual(newerBCache)
@@ -6,6 +6,12 @@ import {
type FileExplorerDirectoryListing
} from './file-explorer-directory-listing'
import { forEachWithConcurrency } from '../../../../shared/map-with-concurrency'
import {
clearFileExplorerDirsLoading,
markFileExplorerDirsLoading,
withPendingFileExplorerDirCacheEntries,
type FileExplorerLoadingDirsUpdater
} from './file-explorer-dir-load-state'
export type RefreshFileExplorerTreeDir = {
dirPath: string
@@ -17,6 +23,7 @@ export type RefreshFileExplorerExpandedDirsParams = {
worktreePath: string
dirLoadTracker: FileExplorerDirLoadTracker
setDirCache: Dispatch<SetStateAction<Record<string, DirCache>>>
updateLoadingDirPaths: FileExplorerLoadingDirsUpdater
readDirectory: (dirPath: string) => Promise<FileExplorerDirectoryListing>
maxConcurrentReads: number
/** Called per dir whose fresh listing was committed, so callers can clear a staleness mark. */
@@ -28,6 +35,7 @@ export async function refreshFileExplorerExpandedDirs({
worktreePath,
dirLoadTracker,
setDirCache,
updateLoadingDirPaths,
readDirectory,
maxConcurrentReads,
onDirCommitted
@@ -55,19 +63,22 @@ export async function refreshFileExplorerExpandedDirs({
// workers itself — otherwise a later batch commits after the caller already saw this reject.
let stopped = false
const uniqueDirPaths = uniqueDirs.map((dir) => dir.dirPath)
// Why: mark every dir loading up front — FileExplorer's auto-load
// effect re-runs on any `expanded` change and fans out an unbounded loadDir per
// dir that is neither cached nor loading, which would defeat the concurrency cap.
setDirCache((prev) => {
const next = { ...prev }
for (const { dirPath } of uniqueDirs) {
next[dirPath] = {
children: prev[dirPath]?.children ?? [],
loading: true
}
// Why this no longer touches dirCache for known dirs: the pre-mark used to rebuild the whole
// visible tree once per refresh before a single fresh listing existed.
setDirCache((prev) => withPendingFileExplorerDirCacheEntries(prev, uniqueDirPaths))
updateLoadingDirPaths((prev) => markFileExplorerDirsLoading(prev, uniqueDirPaths))
// Why: only dirs this refresh still owns — a superseding load owns the flag for the rest.
const clearOwnedLoadingMarks = (dirPaths: readonly string[]): void => {
const owned = dirPaths.filter((dirPath) => dirLoadTracker.isCurrent(loadTokens.get(dirPath)!))
if (owned.length > 0) {
updateLoadingDirPaths((prev) => clearFileExplorerDirsLoading(prev, owned))
}
return next
})
}
const commitPendingResults = (): void => {
if (stopped) {
@@ -88,6 +99,7 @@ export async function refreshFileExplorerExpandedDirs({
}
return next
})
clearOwnedLoadingMarks(currentResults.map((result) => result.dirPath))
committedDirs += currentResults.length
// Why: the cache write above already landed for every result, so a throwing callback must not
// strand the rest of the batch with a staleness mark no later commit will clear.
@@ -119,41 +131,46 @@ export async function refreshFileExplorerExpandedDirs({
}
}
await forEachWithConcurrency(uniqueDirs, maxConcurrentReads, async ({ dirPath, depth }) => {
if (stopped) {
return
}
const loadToken = loadTokens.get(dirPath)!
// A superseding load owns this dir now; do not spend a round trip on a result we must drop.
if (!dirLoadTracker.isCurrent(loadToken)) {
settleRead()
return
}
let cache: DirCache | undefined
try {
const listing = await readDirectory(dirPath)
if (dirLoadTracker.isCurrent(loadToken)) {
cache = {
children: fileExplorerEntriesToTreeNodes(
listing.entries,
dirPath,
depth,
worktreePath,
listing.operationOwner
),
loading: false,
operationOwner: listing.operationOwner
try {
await forEachWithConcurrency(uniqueDirs, maxConcurrentReads, async ({ dirPath, depth }) => {
if (stopped) {
return
}
const loadToken = loadTokens.get(dirPath)!
// A superseding load owns this dir now; do not spend a round trip on a result we must drop.
if (!dirLoadTracker.isCurrent(loadToken)) {
settleRead()
return
}
let cache: DirCache | undefined
try {
const listing = await readDirectory(dirPath)
if (dirLoadTracker.isCurrent(loadToken)) {
cache = {
children: fileExplorerEntriesToTreeNodes(
listing.entries,
dirPath,
depth,
worktreePath,
listing.operationOwner
),
operationOwner: listing.operationOwner
}
}
} catch {
if (dirLoadTracker.isCurrent(loadToken)) {
cache = { children: [] }
}
}
} catch {
if (dirLoadTracker.isCurrent(loadToken)) {
cache = { children: [], loading: false }
}
settleRead(cache ? { dirPath, cache } : undefined)
})
if (settledSinceCommit > 0) {
commitPendingResults()
}
settleRead(cache ? { dirPath, cache } : undefined)
})
if (settledSinceCommit > 0) {
commitPendingResults()
} finally {
// Why: no dir this refresh still owns may keep a spinner once the wave ends, including the
// ones a failed commit or a superseded read left uncommitted.
clearOwnedLoadingMarks(uniqueDirPaths)
}
return committedDirs === uniqueDirs.length
@@ -34,20 +34,15 @@ describe('decideExpandedDirLoad', () => {
const children = [{ name: 'gone.ts', path: '/repo/src/gone.ts' } as TreeNode]
it('re-reads a cached dir whose listing the last full refresh skipped', () => {
expect(decideExpandedDirLoad({ children, loading: false }, true)).toBe('reload')
expect(decideExpandedDirLoad({ children }, true)).toBe('reload')
})
it('trusts a cached listing that is not stale', () => {
expect(decideExpandedDirLoad({ children, loading: false }, false)).toBe('skip')
expect(decideExpandedDirLoad({ children }, false)).toBe('skip')
})
it('reads a dir that has never been listed', () => {
expect(decideExpandedDirLoad(undefined, false)).toBe('load')
expect(decideExpandedDirLoad({ children: [], loading: false }, false)).toBe('load')
})
it('never stacks a read on one already in flight, stale or not', () => {
expect(decideExpandedDirLoad({ children, loading: true }, true)).toBe('skip')
expect(decideExpandedDirLoad({ children: [], loading: true }, false)).toBe('skip')
expect(decideExpandedDirLoad({ children: [] }, false)).toBe('load')
})
})
@@ -19,14 +19,16 @@ export function collectStaleDirCachePaths(
export type ExpandedDirLoadDecision = 'skip' | 'load' | 'reload'
/** What the expansion effect owes a newly expanded dir: nothing, a first read, or a forced re-read. */
/**
* What the expansion effect owes a newly expanded dir: nothing, a first read, or a forced re-read.
*
* Callers must skip a dir with a read already in flight before asking — that state lives in the
* loading set, not in `dirCache` (see file-explorer-dir-load-state.ts).
*/
export function decideExpandedDirLoad(
cached: DirCache | undefined,
stale: boolean
): ExpandedDirLoadDecision {
if (cached?.loading) {
return 'skip'
}
if (!cached?.children.length) {
return 'load'
}
@@ -17,9 +17,9 @@ export type TreeNode = {
operationOwner?: FileExplorerOperationOwner
}
/** Why no `loading` here: see file-explorer-loading-dirs.ts — identity changes re-walk the tree. */
export type DirCache = {
children: TreeNode[]
loading: boolean
operationOwner?: FileExplorerOperationOwner
}
@@ -12,7 +12,6 @@ function processRootEvent(root: string, event: FsChangeEvent): ReturnType<typeof
const refreshDir = vi.fn()
const rootCache: DirCache = {
children: [],
loading: false,
operationOwner: { kind: 'local' }
}
@@ -55,18 +55,21 @@ export function createCachedDirPathIndex(
/**
* Map an event path to the dirCache key that should be refreshed.
* Windows watchers often differ in drive-letter casing from the worktree key.
*
* Why the index is a thunk: the direct `dirPath in cache` hit answers nearly every lookup outside
* Windows casing drift, so building it eagerly costs one normalize per cached dir for nothing.
*/
export function resolveCachedDirPath(
cache: Record<string, { children: unknown }>,
dirPath: string,
worktreePath?: string,
cachePathIndex?: ReadonlyMap<string, string>
cachePathIndex?: () => ReadonlyMap<string, string>
): string | null {
if (dirPath in cache) {
return dirPath
}
const target = normalizeRuntimePathForComparison(dirPath)
const indexedPath = cachePathIndex?.get(target)
const indexedPath = cachePathIndex?.().get(target)
if (indexedPath) {
return indexedPath
}
@@ -14,7 +14,6 @@ function cacheWithChildren(paths: string[]): DirCache {
depth: 0,
operationOwner: { kind: 'local' }
})),
loading: false,
operationOwner: { kind: 'local' }
}
}
@@ -432,7 +431,9 @@ describe('processFileExplorerFsPayload update reconciliation', () => {
}
expect(setDirCache).toHaveBeenCalledOnce()
expect(keyVisits).toBe(entryCount * 2)
// One scan, in purgeDirCacheSubtrees. The casing-fallback index stays unbuilt because every
// lookup here hits `dirPath in cache` directly.
expect(keyVisits).toBe(entryCount)
expect(expandedPathReads).toBe(expandedPaths.length)
expect(remainingExpanded).toEqual(new Set())
})
@@ -68,7 +68,9 @@ export function processFileExplorerFsPayload(args: ProcessFileExplorerFsPayloadA
const dirsToRefresh = new Set<string>()
const childPathIndexes = new Map<string, Set<string>>()
const cachePathIndex = createCachedDirPathIndex(cache)
let cachedDirPathIndex: ReadonlyMap<string, string> | undefined
const cachePathIndex = (): ReadonlyMap<string, string> =>
(cachedDirPathIndex ??= createCachedDirPathIndex(cache))
const cachedDirsToPurge = new Set<string>()
const reconciledRenameSources = new Set<string>()
let needsFullRefresh = false
@@ -17,6 +17,7 @@ type UseFileExplorerRowScrollingParams = {
worktreePath: string | null
expanded: Set<string>
dirCache: Record<string, DirCache>
loadingDirPaths: ReadonlySet<string>
rootCache: DirCache | undefined
loadDir: (dirPath: string, depth: number, options?: { force?: boolean }) => Promise<boolean>
setSelectedPath: (path: string | null) => void
@@ -42,6 +43,7 @@ export function useFileExplorerRowScrolling({
worktreePath,
expanded,
dirCache,
loadingDirPaths,
rootCache,
loadDir,
setSelectedPath,
@@ -81,6 +83,7 @@ export function useFileExplorerRowScrolling({
clearPendingExplorerReveal,
expanded,
dirCache,
loadingDirPaths,
rootCache,
rowProjection,
loadDir,
@@ -11,6 +11,7 @@ type UseFileExplorerTreeLoadEffectsParams = {
visibleFilesWorktreePath: string | null
expanded: Set<string>
dirCache: Record<string, DirCache>
loadingDirPaths: ReadonlySet<string>
rootError: string | null
isDirStale: (dirPath: string) => boolean
loadDir: (dirPath: string, depth: number, options?: { force?: boolean }) => Promise<boolean>
@@ -24,6 +25,7 @@ export function useFileExplorerTreeLoadEffects({
visibleFilesWorktreePath,
expanded,
dirCache,
loadingDirPaths,
rootError,
isDirStale,
loadDir,
@@ -75,6 +77,11 @@ export function useFileExplorerTreeLoadEffects({
return
}
for (const dirPath of expanded) {
// Why first: a refresh wave marks every dir it owns before its first read lands, and without
// this the effect would fan out an unbounded loadDir per dir on the next `expanded` change.
if (loadingDirPaths.has(dirPath)) {
continue
}
// Why: a full refresh (watcher overflow) re-reads only root and the dirs expanded at the time,
// so a listing cached while collapsed is unverified — re-read it here instead of trusting it.
const decision = decideExpandedDirLoad(dirCache[dirPath], isDirStale(dirPath))
@@ -84,6 +84,7 @@ export function useFileExplorerTreePaneState({
const {
dirCache,
setDirCache,
loadingDirPaths,
rootCache,
rootError,
loadDir,
@@ -165,6 +166,7 @@ export function useFileExplorerTreePaneState({
visibleFilesWorktreePath,
expanded,
dirCache,
loadingDirPaths,
rootError,
isDirStale,
loadDir,
@@ -215,6 +217,7 @@ export function useFileExplorerTreePaneState({
worktreePath: visibleFilesWorktreePath,
expanded,
dirCache,
loadingDirPaths,
rootCache,
loadDir,
setSelectedPath: setSingleSelectedPath,
@@ -18,6 +18,7 @@ type UseFileExplorerRevealParams = {
clearPendingExplorerReveal: () => void
expanded: Set<string>
dirCache: Record<string, DirCache>
loadingDirPaths: ReadonlySet<string>
rootCache: DirCache | undefined
rowProjection: FileExplorerRowProjection
loadDir: (dirPath: string, depth: number, options?: { force?: boolean }) => Promise<boolean>
@@ -34,6 +35,7 @@ export function useFileExplorerReveal({
clearPendingExplorerReveal,
expanded,
dirCache,
loadingDirPaths,
rootCache,
rowProjection,
loadDir,
@@ -154,14 +156,15 @@ export function useFileExplorerReveal({
const missingAncestor = pendingRevealAncestorDirs.find(
(dirPath) => !rowProjection.hasPath(dirPath)
)
const rootStillLoading = !rootCache || loadingDirPaths.has(worktreePath)
const parentDirStillLoading =
parentDirPath === worktreePath
? (rootCache?.loading ?? true)
: (parentDirCache?.loading ?? true)
? rootStillLoading
: !parentDirCache || loadingDirPaths.has(parentDirPath)
const parentDirKnown = parentDirPath === worktreePath ? !!rootCache : !!parentDirCache
if (
(rootCache?.loading ?? true) ||
rootStillLoading ||
missingExpandedAncestor ||
missingAncestor ||
parentDirStillLoading ||
@@ -210,6 +213,7 @@ export function useFileExplorerReveal({
clearPendingExplorerReveal,
dirCache,
expanded,
loadingDirPaths,
pendingExplorerReveal,
pendingRevealAncestorDirs,
rowProjection,
@@ -0,0 +1,248 @@
// @vitest-environment happy-dom
import { act, cleanup, renderHook } from '@testing-library/react'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { useAppStore } from '@/store'
import type { AppState } from '@/store/types'
import type { DirEntry } from '../../../../shared/filesystem-entry-types'
import { FileExplorerRow } from './FileExplorerRow'
import { FileExplorerVirtualRows } from './FileExplorerVirtualRows'
import { createFileExplorerRowProjection } from './file-explorer-row-projection'
import { directoryNode } from './file-explorer-tree-node-test-fixtures'
import { visit, type ReactElementLike } from './file-explorer-element-tree-test-harness'
import { useFileExplorerTreeLoadEffects } from './use-file-explorer-tree-load-effects'
import { useFileExplorerTree } from './useFileExplorerTree'
import { useFileExplorerVisibleRowProjection } from './useFileExplorerVisibleRowProjection'
const readDirectoryMock = vi.hoisted(() => vi.fn())
vi.mock('./file-explorer-directory-listing', async (importOriginal) => ({
...(await importOriginal<Record<string, unknown>>()),
readFileExplorerDirectory: readDirectoryMock
}))
vi.mock('./file-explorer-operation-owner', async (importOriginal) => ({
...(await importOriginal<Record<string, unknown>>()),
getFileExplorerOperationOwner: () => ({ kind: 'local' as const })
}))
vi.mock('@/runtime/runtime-git-client', () => ({
getRuntimeGitIgnoredPaths: vi.fn().mockResolvedValue([])
}))
const initialAppState = useAppStore.getInitialState()
const WORKTREE_PATH = '/repo'
const SRC_DIR = '/repo/src'
function entry(name: string, isDirectory = false): DirEntry {
return { name, isDirectory } as DirEntry
}
function listing(...entries: DirEntry[]) {
return { entries, operationOwner: { kind: 'local' as const } }
}
function useTreeWithProjection(expanded: Set<string>) {
const tree = useFileExplorerTree(WORKTREE_PATH, expanded, 'wt-1')
const projection = useFileExplorerVisibleRowProjection(
'wt-1',
WORKTREE_PATH,
tree.dirCache,
expanded,
false,
true,
null
)
return { tree, rowProjection: projection.rowProjection }
}
/** Counts how many times the memoized visible-row projection produced a new value. */
function renderTreeWithProjectionRebuildCounter(expanded: Set<string>): {
result: { current: ReturnType<typeof useTreeWithProjection> }
rebuilds: () => number
} {
const seen = new Set<unknown>()
const hook = renderHook(() => {
const value = useTreeWithProjection(expanded)
seen.add(value.rowProjection)
return value
})
return { result: hook.result, rebuilds: () => seen.size }
}
/** Holds the next directory read open so the loading commit lands in its own render. */
function gateNextRead(): { resolve: (value: ReturnType<typeof listing>) => void } {
let resolve!: (value: ReturnType<typeof listing>) => void
const gate = new Promise<ReturnType<typeof listing>>((nextResolve) => {
resolve = nextResolve
})
readDirectoryMock.mockImplementationOnce(() => gate)
return { resolve }
}
function findFileExplorerRow(node: unknown): ReactElementLike {
let found: ReactElementLike | null = null
visit(node, (candidate) => {
if (candidate.type === FileExplorerRow) {
found = candidate
}
})
if (!found) {
throw new Error('file explorer row not found')
}
return found
}
describe('file explorer directory refresh churn', () => {
beforeEach(() => {
readDirectoryMock.mockReset().mockResolvedValue(listing())
useAppStore.setState(initialAppState, true)
useAppStore.setState({
settings: { activeRuntimeEnvironmentId: null } as AppState['settings']
})
})
afterEach(() => {
cleanup()
useAppStore.setState(initialAppState, true)
})
it('rebuilds the visible row projection once per touched-directory refresh', async () => {
const expanded = new Set([SRC_DIR])
readDirectoryMock.mockResolvedValue(listing(entry('index.ts')))
const { result, rebuilds } = renderTreeWithProjectionRebuildCounter(expanded)
await act(async () => {
await result.current.tree.loadDir(WORKTREE_PATH, -1, { force: true })
})
await act(async () => {
await result.current.tree.loadDir(SRC_DIR, 0, { force: true })
})
const rebuildsBeforeRefresh = rebuilds()
// One watcher-driven refresh of a directory that is already cached, with the read gated so the
// loading state is committed and painted before the listing lands.
const gatedRead = gateNextRead()
let pendingRefresh!: Promise<void>
await act(async () => {
pendingRefresh = result.current.tree.refreshDir(SRC_DIR)
await Promise.resolve()
})
expect(result.current.tree.loadingDirPaths.has(SRC_DIR)).toBe(true)
// Why zero here: marking the dir loading used to commit a second dirCache identity carrying a
// byte-identical row set, so every refresh paid for two full tree walks and flattens.
expect(rebuilds()).toBe(rebuildsBeforeRefresh)
await act(async () => {
gatedRead.resolve(listing(entry('index.ts')))
await pendingRefresh
})
expect(rebuilds() - rebuildsBeforeRefresh).toBe(1)
})
it('keeps the directory marked loading for the whole of a slow read', async () => {
const expanded = new Set([SRC_DIR])
const gatedRead = gateNextRead()
const { result, rebuilds } = renderTreeWithProjectionRebuildCounter(expanded)
let pendingLoad!: Promise<boolean>
await act(async () => {
pendingLoad = result.current.tree.loadDir(SRC_DIR, 0)
await Promise.resolve()
})
expect(result.current.tree.loadingDirPaths.has(SRC_DIR)).toBe(true)
const rebuildsWhileLoading = rebuilds()
await act(async () => {
gatedRead.resolve(listing(entry('index.ts')))
await pendingLoad
})
expect(result.current.tree.loadingDirPaths.has(SRC_DIR)).toBe(false)
expect(result.current.tree.dirCache[SRC_DIR].children).toHaveLength(1)
// The spinner rendered without the projection being rebuilt for it.
expect(rebuilds()).toBe(rebuildsWhileLoading + 1)
})
it('does not stack a second read on an expanded dir the loading set already owns', () => {
const loadDir = vi.fn().mockResolvedValue(true)
const params = {
visibleFilesWorktreePath: WORKTREE_PATH,
expanded: new Set([SRC_DIR]),
dirCache: {},
loadingDirPaths: new Set([SRC_DIR]),
rootError: null,
isDirStale: () => false,
loadDir,
resetAndLoad: vi.fn(),
resetSelection: vi.fn(),
setNameFilterQuery: vi.fn()
}
const hook = renderHook((props: typeof params) => useFileExplorerTreeLoadEffects(props), {
initialProps: params
})
// Why this matters: a refresh wave marks every dir it owns before its first read lands, and the
// effect re-runs on any `expanded` change — without the guard it fans out an unbounded loadDir.
expect(loadDir).not.toHaveBeenCalled()
// The effect re-runs on `expanded` identity; by then the wave's read has landed and cleared.
hook.rerender({
...params,
expanded: new Set([SRC_DIR]),
loadingDirPaths: new Set<string>()
})
expect(loadDir).toHaveBeenCalledTimes(1)
expect(loadDir).toHaveBeenCalledWith(SRC_DIR, 0, undefined)
})
it('renders the folder spinner from the loading dir set', () => {
const rowProps = {
virtualizer: {
getTotalSize: () => 26,
getVirtualItems: () => [{ index: 0, key: 'src', start: 0 }],
measureElement: vi.fn()
} as never,
inlineInputIndex: -1,
rowProjection: createFileExplorerRowProjection([directoryNode]),
inlineInput: null,
handleInlineSubmit: vi.fn(),
dismissInlineInput: vi.fn(),
folderStatusByRelativePath: new Map(),
statusByRelativePath: new Map(),
ignoredByRelativePath: new Set<string>(),
expanded: new Set([directoryNode.path]),
selectedPaths: new Set<string>(),
activeFileId: null,
flashingPath: null,
deleteShortcutLabel: 'Del',
onClick: vi.fn(),
onDoubleClick: vi.fn(),
onViewFile: vi.fn(),
onContextMenuSelect: vi.fn(),
onCopyPaths: vi.fn(),
onStartNew: vi.fn(),
onStartRename: vi.fn(),
onDuplicate: vi.fn(),
onAddFolderAsProject: vi.fn(),
canAddFolderAsProject: () => false,
onOpenInTerminal: vi.fn(),
onRequestDelete: vi.fn(),
onCollapseFolderSubtree: vi.fn(),
onFindInFolder: vi.fn(),
onMoveDrop: vi.fn(),
onDragTargetChange: vi.fn(),
onDragSourceChange: vi.fn(),
onDragExpandDir: vi.fn(),
onNativeDragTargetChange: vi.fn(),
onNativeDragExpandDir: vi.fn(),
dropTargetDir: null,
dragSourcePath: null,
nativeDropTargetDir: null
}
const loading = FileExplorerVirtualRows({
...rowProps,
loadingDirPaths: new Set([directoryNode.path])
})
const idle = FileExplorerVirtualRows({ ...rowProps, loadingDirPaths: new Set<string>() })
expect(findFileExplorerRow(loading).props.isLoading).toBe(true)
expect(findFileExplorerRow(idle).props.isLoading).toBe(false)
})
})
@@ -16,10 +16,18 @@ import {
import { refreshFileExplorerExpandedDirs } from './file-explorer-expanded-dirs-refresh'
import { collectStaleDirCachePaths } from './file-explorer-stale-dir-cache'
import { fileExplorerRefreshConcurrency } from './file-explorer-refresh-concurrency'
import {
clearFileExplorerDirsLoading,
EMPTY_FILE_EXPLORER_LOADING_DIRS,
markFileExplorerDirsLoading,
withPendingFileExplorerDirCacheEntries
} from './file-explorer-dir-load-state'
type UseFileExplorerTreeResult = {
dirCache: Record<string, DirCache>
setDirCache: Dispatch<SetStateAction<Record<string, DirCache>>>
/** Dirs with a read in flight — kept out of dirCache so the row projection does not rebuild. */
loadingDirPaths: ReadonlySet<string>
rootCache: DirCache | undefined
rootError: string | null
loadDir: (
@@ -42,9 +50,27 @@ export function useFileExplorerTree(
activeWorktreeId?: string | null
): UseFileExplorerTreeResult {
const [dirCache, setDirCache] = useState<Record<string, DirCache>>({})
const [loadingDirPaths, setLoadingDirPaths] = useState<ReadonlySet<string>>(
EMPTY_FILE_EXPLORER_LOADING_DIRS
)
const [rootError, setRootError] = useState<string | null>(null)
const dirCacheRef = useRef(dirCache)
dirCacheRef.current = dirCache
// Why the ref is authoritative rather than a render mirror: writing it during render is unsafe
// (React may discard that render), and a mirror would leave loadDir's in-flight guard reading a
// set one commit stale — long enough for a second read of the same dir to slip through.
const loadingDirPathsRef = useRef<ReadonlySet<string>>(EMPTY_FILE_EXPLORER_LOADING_DIRS)
const updateLoadingDirPaths = useCallback(
(update: (prev: ReadonlySet<string>) => ReadonlySet<string>) => {
const next = update(loadingDirPathsRef.current)
if (next === loadingDirPathsRef.current) {
return
}
loadingDirPathsRef.current = next
setLoadingDirPaths(next)
},
[]
)
const dirLoadTrackerRef = useRef<ReturnType<typeof createFileExplorerDirLoadTracker>>(undefined!)
dirLoadTrackerRef.current ??= createFileExplorerDirLoadTracker()
// Why: a ref, not state — the expansion effect must read the mark set by a refresh that landed
@@ -60,25 +86,23 @@ export function useFileExplorerTree(
options?: { force?: boolean; failOnError?: boolean }
) => {
const cache = dirCacheRef.current
if (!options?.force && (cache[dirPath]?.children.length > 0 || cache[dirPath]?.loading)) {
if (
!options?.force &&
(cache[dirPath]?.children.length > 0 || loadingDirPathsRef.current.has(dirPath))
) {
return true
}
const loadToken = dirLoadTrackerRef.current.begin(dirPath)
// Why: this read starts after the refresh that marked the dir, so its result is current.
staleDirsRef.current.delete(dirPath)
// Why: when force-reloading a directory (e.g. after a file is created,
// duplicated, or deleted), keep the previous children visible while the
// fresh listing loads. Clearing to [] would momentarily shrink the
// visible projection and make the virtualizer jump to the top.
setDirCache((prev) => ({
...prev,
[dirPath]: {
children: prev[dirPath]?.children ?? [],
loading: true
}
}))
// Why: an already-cached dir keeps its children visible for the whole read — clearing to []
// would momentarily shrink the visible projection and jump the virtualizer to the top.
setDirCache((prev) => withPendingFileExplorerDirCacheEntries(prev, [dirPath]))
updateLoadingDirPaths((prev) => markFileExplorerDirsLoading(prev, [dirPath]))
try {
const listing = await readFileExplorerDirectory(activeWorktreeId, worktreePath, dirPath)
// Why: only the current owner may clear the flag — a superseded read clearing it would
// drop the spinner while the load that replaced it is still in flight.
if (!dirLoadTrackerRef.current.isCurrent(loadToken)) {
return false
}
@@ -94,8 +118,9 @@ export function useFileExplorerTree(
)
setDirCache((prev) => ({
...prev,
[dirPath]: { children, loading: false, operationOwner: listing.operationOwner }
[dirPath]: { children, operationOwner: listing.operationOwner }
}))
updateLoadingDirPaths((prev) => clearFileExplorerDirsLoading(prev, [dirPath]))
return true
} catch (error) {
if (!dirLoadTrackerRef.current.isCurrent(loadToken)) {
@@ -109,11 +134,12 @@ export function useFileExplorerTree(
setRootError(error instanceof Error ? error.message : String(error))
rootReadFailedRef.current = true
}
setDirCache((prev) => ({ ...prev, [dirPath]: { children: [], loading: false } }))
setDirCache((prev) => ({ ...prev, [dirPath]: { children: [] } }))
updateLoadingDirPaths((prev) => clearFileExplorerDirsLoading(prev, [dirPath]))
return !options?.failOnError
}
},
[activeWorktreeId, worktreePath]
[activeWorktreeId, updateLoadingDirPaths, worktreePath]
)
const markPathAsDirectory = useCallback((path: string) => {
@@ -206,6 +232,7 @@ export function useFileExplorerTree(
worktreePath,
dirLoadTracker: dirLoadTrackerRef.current,
setDirCache,
updateLoadingDirPaths,
readDirectory: (dirPath) =>
readFileExplorerDirectory(activeWorktreeId, worktreePath, dirPath),
maxConcurrentReads: fileExplorerRefreshConcurrency(
@@ -214,7 +241,7 @@ export function useFileExplorerTree(
onDirCommitted: (dirPath) => staleDirsRef.current.delete(dirPath)
})
return allDirsCommitted ? 'refreshed' : 'superseded'
}, [activeWorktreeId, expanded, loadDir, worktreePath])
}, [activeWorktreeId, expanded, loadDir, updateLoadingDirPaths, worktreePath])
const refreshDir = useCallback(
async (dirPath: string) => {
@@ -240,15 +267,17 @@ export function useFileExplorerTree(
dirLoadTrackerRef.current.reset()
staleDirsRef.current.clear()
setDirCache({})
updateLoadingDirPaths(() => EMPTY_FILE_EXPLORER_LOADING_DIRS)
setRootError(null)
if (worktreePath) {
void loadDir(worktreePath, -1, { force: true })
}
}, [worktreePath, loadDir])
}, [worktreePath, loadDir, updateLoadingDirPaths])
return {
dirCache,
setDirCache,
loadingDirPaths,
rootCache,
rootError,
loadDir,
@@ -22,7 +22,8 @@ function useProjection(query: string) {
})
}
function treeDirCache(loading: boolean) {
/** A fresh object per call: a wave-batched refresh commits a new dirCache identity per wave. */
function treeDirCache() {
return {
'/repo': {
children: [
@@ -33,13 +34,12 @@ function treeDirCache(loading: boolean) {
isDirectory: true,
depth: 0
}
],
loading
]
}
}
}
function useTreeProjection(dirCache = treeDirCache(false)) {
function useTreeProjection(dirCache = treeDirCache()) {
return useFileExplorerVisibleRowProjection(
'worktree-1',
'/repo',
@@ -105,12 +105,12 @@ describe('file explorer ignored-path query debounce', () => {
// ignored query is an uncancellable remote git check-ignore over the whole
// visible tree, so identical contents must not re-issue it.
const hook = renderHook(({ dirCache }) => useTreeProjection(dirCache), {
initialProps: { dirCache: treeDirCache(false) }
initialProps: { dirCache: treeDirCache() }
})
expect(getRuntimeGitIgnoredPathsMock).toHaveBeenCalledTimes(1)
hook.rerender({ dirCache: treeDirCache(true) })
hook.rerender({ dirCache: treeDirCache(false) })
hook.rerender({ dirCache: treeDirCache() })
hook.rerender({ dirCache: treeDirCache() })
expect(getRuntimeGitIgnoredPathsMock).toHaveBeenCalledTimes(1)
})
@@ -25,7 +25,7 @@ function row(relativePath: string, isDirectory = false, depth?: number): TreeNod
function cache(childrenByPath: Record<string, TreeNode[]>): Record<string, DirCache> {
const dirCache: Record<string, DirCache> = {}
for (const [path, children] of Object.entries(childrenByPath)) {
dirCache[path] = { children, loading: false }
dirCache[path] = { children }
}
return dirCache
}
@@ -74,7 +74,7 @@ describe('useFileExplorerWatch pending refreshes', () => {
useFileExplorerWatch({
worktreePath: visiblePath,
activeWorktreeId: 'wt-1',
dirCache: { '/repo': { children: [], loading: false } },
dirCache: { '/repo': { children: [] } },
setDirCache: vi.fn(),
expanded: new Set(),
setSelectedPath: vi.fn(),
@@ -164,6 +164,11 @@ export function buildEditorExternalWatchEventHandler(
for (const change of batchPaths.changes) {
const matching = batchPaths.matchingOpenFiles(change)
// Why: most watched paths match no open file, and the notification below is only ever read
// past this point — building it first allocates (and dictionary-modes) it for nothing.
if (matching.length === 0 && !batchPaths.hasCombinedDiffConsumer) {
continue
}
const notification: EditorExternalWatchNotification = {
worktreeId: target.worktreeId,
worktreePath: target.worktreePath,
@@ -177,9 +182,7 @@ export function buildEditorExternalWatchEventHandler(
}
})
if (matching.length === 0) {
if (batchPaths.hasCombinedDiffConsumer) {
scheduleDebouncedEditorExternalReload(notification)
}
scheduleDebouncedEditorExternalReload(notification)
continue
}
const dirtyMatches = matching.filter((file) => file.isDirty)