fix(files): preserve tree expand state after mutations (#1773)

Stop remounting the file tree on every refresh so expanded folders and
scroll position survive delete, move, rename, and save. Relist open
directories in place and drop stale collapsed caches.
This commit is contained in:
Anso
2026-08-05 08:52:50 -04:00
committed by GitHub
parent 63a3594671
commit 9b481212f6
4 changed files with 320 additions and 22 deletions
+62 -17
View File
@@ -16,8 +16,10 @@ import { cn } from '@/lib/utils';
interface FileTreeProps {
/** Loads directory contents at `relPath` (use '' for the tree root). */
loadDir: (relPath: string) => Promise<FileEntry[]>;
/** Stable identity for the source. Changing it remounts the tree. */
/** Stable identity for the source. Parents remount via React `key` when this
* changes so expand state does not leak across sources. */
sourceKey: string;
/** Soft refresh: relist root and expanded dirs in place; keep expand state. */
refreshKey?: number;
selectedPath: string;
onSelectFile: (relPath: string, entry: FileEntry) => void;
@@ -125,37 +127,80 @@ export function FileTree({
}
const sourceKeyRef = useRef(sourceKey);
const loadDirRef = useRef(loadDir);
// Read at merge time so a concurrent expand/collapse is not overwritten by the snapshot.
const expandedDirsRef = useRef(expandedDirs);
expandedDirsRef.current = expandedDirs;
// Always keep the latest loader in a ref so callers can pass a fresh
// function each render without re-triggering the root fetch effect below.
// function each render without re-triggering the refresh effect below.
loadDirRef.current = loadDir;
useEffect(() => {
sourceKeyRef.current = sourceKey;
let cancelled = false;
const expandedSnapshot = [...expandedDirs];
loadDirRef.current('')
.then((entries) => {
if (!cancelled) {
setRootEntries(entries);
setRootLoading(false);
void (async () => {
try {
const entries = await loadDirRef.current('');
if (cancelled) return;
setRootEntries(entries);
setError(null);
setRootLoading(false);
} catch (err: unknown) {
if (cancelled) return;
const msg = err instanceof Error ? err.message : 'Failed to load files.';
setError(msg);
setRootLoading(false);
toast.error('Failed to load files.');
return;
}
const results = await Promise.all(expandedSnapshot.map(async (path) => {
try {
const entries = await loadDirRef.current(path);
return { path, ok: true as const, entries };
} catch {
// Folder is gone or unlistable after a mutation. Prune below; do not
// toast (the user often just deleted or moved this folder).
return { path, ok: false as const };
}
})
.catch((err: unknown) => {
if (!cancelled) {
const msg = err instanceof Error ? err.message : 'Failed to load files.';
setError(msg);
setRootLoading(false);
toast.error('Failed to load files.');
}));
if (cancelled) return;
const failed = results.filter((r) => !r.ok).map((r) => r.path);
function inFailedTree(path: string): boolean {
return failed.some((p) => path === p || path.startsWith(`${p}/`));
}
if (failed.length > 0) {
setExpandedDirs((current) => {
const next = new Set([...current].filter((key) => !inFailedTree(key)));
return next.size === current.size ? current : next;
});
}
setDirContents((prev) => {
const fetched = new Map<string, FileEntry[]>();
for (const r of results) {
if (r.ok) fetched.set(r.path, r.entries);
}
const next = new Map<string, FileEntry[]>();
for (const path of expandedDirsRef.current) {
if (inFailedTree(path)) continue;
const entries = fetched.get(path) ?? prev.get(path);
if (entries) next.set(path, entries);
}
return next;
});
})();
return () => {
cancelled = true;
};
// loadDir is intentionally read through loadDirRef to avoid refetch on
// identity-only changes from the parent (StackFileExplorer rebuilds the
// arrow on every render).
// Do not depend on expandedDirs (snapshot at start; expand/collapse must
// not refetch). loadDir is read via loadDirRef so identity-only parent
// changes do not refetch.
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [sourceKey, refreshKey]);
@@ -565,7 +565,7 @@ export function StackFileExplorer({
)}
<div className="flex-1 min-h-0 overflow-hidden">
<FileTree
key={`${stackName}:${selectedRootId}:${refreshKey}`}
key={`${stackName}:${selectedRootId}`}
sourceKey={`${stackName}:${selectedRootId}`}
loadDir={(p) => listStackDirectory(stackName, p, selectedRootId)}
refreshKey={refreshKey}
@@ -1,9 +1,10 @@
/**
* Coverage for FileTree.
*
* Locks the expand/collapse behavior: root directory loaded on mount,
* subdirectory fetched on first expand, collapsed on second click, and
* re-expanded from cache (no second fetch) on third click.
* Locks expand/collapse: root loaded on mount, subdirectory fetched on first
* expand, collapsed on second click, re-expanded from cache on third click
* when no refresh ran. Also locks soft refresh (`refreshKey`): expand state
* survives, collapsed caches drop, failed trees (and descendants) prune.
*/
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import { render, screen, waitFor, fireEvent } from '@testing-library/react';
@@ -639,3 +640,255 @@ describe('FileTree layout', () => {
expect(screen.getByText('Rename')).toBeInTheDocument();
});
});
// ── soft refresh: expand + listings survive refreshKey without remount ───────
describe('FileTree soft refresh', () => {
const DEEP_ENTRIES: FileEntry[] = [makeFile('nested.ts')];
const SRC_WITH_DEEP: FileEntry[] = [makeDir('deep'), makeFile('index.ts')];
it('keeps expanded dirs open and re-lists them when refreshKey changes', async () => {
mockLoadDir.mockImplementation(async (relPath: string) => {
if (relPath === '') return ROOT_ENTRIES;
if (relPath === 'src') return SRC_ENTRIES;
throw new Error(`unexpected path ${relPath}`);
});
const user = userEvent.setup();
const { rerender } = render(<FileTree {...defaultProps()} refreshKey={0} />);
await screen.findByText('src');
await user.click(screen.getByText('src'));
await screen.findByText('index.ts');
mockLoadDir.mockClear();
rerender(<FileTree {...defaultProps()} refreshKey={1} />);
await waitFor(() => {
expect(mockLoadDir).toHaveBeenCalledWith('');
expect(mockLoadDir).toHaveBeenCalledWith('src');
});
expect(await screen.findByText('index.ts')).toBeInTheDocument();
expect(screen.getByText('app.ts')).toBeInTheDocument();
});
it('updates an open folder listing in place on soft refresh', async () => {
mockLoadDir.mockImplementation(async (relPath: string) => {
if (relPath === '') return ROOT_ENTRIES;
if (relPath === 'src') return SRC_ENTRIES;
throw new Error(`unexpected path ${relPath}`);
});
const user = userEvent.setup();
const { rerender } = render(<FileTree {...defaultProps()} refreshKey={0} />);
await screen.findByText('src');
await user.click(screen.getByText('src'));
await screen.findByText('index.ts');
mockLoadDir.mockImplementation(async (relPath: string) => {
if (relPath === '') return ROOT_ENTRIES;
if (relPath === 'src') return [makeFile('index.ts'), makeFile('new.ts')];
throw new Error(`unexpected path ${relPath}`);
});
rerender(<FileTree {...defaultProps()} refreshKey={1} />);
expect(await screen.findByText('new.ts')).toBeInTheDocument();
expect(screen.getByText('index.ts')).toBeInTheDocument();
expect(screen.queryByText('app.ts')).not.toBeInTheDocument();
});
it('prunes descendant expand state when only the parent re-list fails', async () => {
mockLoadDir.mockImplementation(async (relPath: string) => {
if (relPath === '') return ROOT_ENTRIES;
if (relPath === 'src') return SRC_WITH_DEEP;
if (relPath === 'src/deep') return DEEP_ENTRIES;
throw new Error(`unexpected path ${relPath}`);
});
const user = userEvent.setup();
const { rerender } = render(<FileTree {...defaultProps()} refreshKey={0} />);
await screen.findByText('src');
await user.click(screen.getByText('src'));
await screen.findByText('deep');
await user.click(screen.getByText('deep'));
await screen.findByText('nested.ts');
mockLoadDir.mockImplementation(async (relPath: string) => {
if (relPath === '') return ROOT_ENTRIES;
if (relPath === 'src') throw new Error('Not found');
if (relPath === 'src/deep') return DEEP_ENTRIES;
throw new Error(`unexpected path ${relPath}`);
});
rerender(<FileTree {...defaultProps()} refreshKey={1} />);
await waitFor(() => expect(screen.queryByText('nested.ts')).not.toBeInTheDocument());
expect(screen.getByText('src')).toBeInTheDocument();
expect(screen.queryByText('deep')).not.toBeInTheDocument();
mockLoadDir.mockClear();
mockLoadDir.mockImplementation(async (relPath: string) => {
if (relPath === '') return ROOT_ENTRIES;
if (relPath === 'src') return SRC_WITH_DEEP;
if (relPath === 'src/deep') return [makeFile('fresh.ts')];
throw new Error(`unexpected path ${relPath}`);
});
await user.click(screen.getByText('src'));
await screen.findByText('deep');
// Prune must drop descendant expand state: deep is visible but collapsed.
expect(screen.queryByText('nested.ts')).not.toBeInTheDocument();
expect(screen.queryByText('fresh.ts')).not.toBeInTheDocument();
await user.click(screen.getByText('deep'));
// Re-expand must refetch, not reuse the pre-prune listing.
expect(await screen.findByText('fresh.ts')).toBeInTheDocument();
});
it('keeps nested expand state and updates deep listings on successful refresh', async () => {
mockLoadDir.mockImplementation(async (relPath: string) => {
if (relPath === '') return ROOT_ENTRIES;
if (relPath === 'src') return SRC_WITH_DEEP;
if (relPath === 'src/deep') return DEEP_ENTRIES;
throw new Error(`unexpected path ${relPath}`);
});
const user = userEvent.setup();
const { rerender } = render(<FileTree {...defaultProps()} refreshKey={0} />);
await screen.findByText('src');
await user.click(screen.getByText('src'));
await screen.findByText('deep');
await user.click(screen.getByText('deep'));
await screen.findByText('nested.ts');
mockLoadDir.mockClear();
mockLoadDir.mockImplementation(async (relPath: string) => {
if (relPath === '') return ROOT_ENTRIES;
if (relPath === 'src') return SRC_WITH_DEEP;
if (relPath === 'src/deep') return [makeFile('nested.ts'), makeFile('extra.ts')];
throw new Error(`unexpected path ${relPath}`);
});
rerender(<FileTree {...defaultProps()} refreshKey={1} />);
await waitFor(() => {
expect(mockLoadDir).toHaveBeenCalledWith('');
expect(mockLoadDir).toHaveBeenCalledWith('src');
expect(mockLoadDir).toHaveBeenCalledWith('src/deep');
});
expect(await screen.findByText('extra.ts')).toBeInTheDocument();
expect(screen.getByText('nested.ts')).toBeInTheDocument();
});
it('drops a collapsed sibling cache while another folder stays open', async () => {
const rootWithLib: FileEntry[] = [makeDir('src'), makeDir('lib'), makeFile('README.md')];
mockLoadDir.mockImplementation(async (relPath: string) => {
if (relPath === '') return rootWithLib;
if (relPath === 'src') return SRC_ENTRIES;
if (relPath === 'lib') return [makeFile('util.ts')];
throw new Error(`unexpected path ${relPath}`);
});
const user = userEvent.setup();
const { rerender } = render(<FileTree {...defaultProps()} refreshKey={0} />);
await screen.findByText('src');
await user.click(screen.getByText('src'));
await screen.findByText('index.ts');
await user.click(screen.getByText('lib'));
await screen.findByText('util.ts');
await user.click(screen.getByText('src'));
await waitFor(() => expect(screen.queryByText('index.ts')).not.toBeInTheDocument());
mockLoadDir.mockClear();
mockLoadDir.mockImplementation(async (relPath: string) => {
if (relPath === '') return rootWithLib;
if (relPath === 'src') return [makeFile('index.ts'), makeFile('added.ts')];
if (relPath === 'lib') return [makeFile('util.ts'), makeFile('extra.ts')];
throw new Error(`unexpected path ${relPath}`);
});
rerender(<FileTree {...defaultProps()} refreshKey={1} />);
await waitFor(() => {
expect(mockLoadDir).toHaveBeenCalledWith('');
expect(mockLoadDir).toHaveBeenCalledWith('lib');
});
expect(mockLoadDir).not.toHaveBeenCalledWith('src');
expect(await screen.findByText('extra.ts')).toBeInTheDocument();
await user.click(screen.getByText('src'));
expect(await screen.findByText('added.ts')).toBeInTheDocument();
expect(mockLoadDir).toHaveBeenCalledWith('src');
});
it('keeps a sibling expanded when another open folder fails to re-list', async () => {
const rootWithLib: FileEntry[] = [makeDir('src'), makeDir('lib'), makeFile('README.md')];
mockLoadDir.mockImplementation(async (relPath: string) => {
if (relPath === '') return rootWithLib;
if (relPath === 'src') return SRC_ENTRIES;
if (relPath === 'lib') return [makeFile('util.ts')];
throw new Error(`unexpected path ${relPath}`);
});
const user = userEvent.setup();
const { rerender } = render(<FileTree {...defaultProps()} refreshKey={0} />);
await screen.findByText('src');
await user.click(screen.getByText('src'));
await screen.findByText('index.ts');
await user.click(screen.getByText('lib'));
await screen.findByText('util.ts');
mockLoadDir.mockImplementation(async (relPath: string) => {
if (relPath === '') return rootWithLib;
if (relPath === 'src') throw new Error('Not found');
if (relPath === 'lib') return [makeFile('util.ts'), makeFile('kept.ts')];
throw new Error(`unexpected path ${relPath}`);
});
rerender(<FileTree {...defaultProps()} refreshKey={1} />);
expect(await screen.findByText('kept.ts')).toBeInTheDocument();
expect(screen.getByText('util.ts')).toBeInTheDocument();
await waitFor(() => expect(screen.queryByText('index.ts')).not.toBeInTheDocument());
expect(screen.getByText('src')).toBeInTheDocument();
});
it('invalidates collapsed dir cache so re-expand refetches after refresh', async () => {
mockLoadDir.mockImplementation(async (relPath: string) => {
if (relPath === '') return ROOT_ENTRIES;
if (relPath === 'src') return SRC_ENTRIES;
throw new Error(`unexpected path ${relPath}`);
});
const user = userEvent.setup();
const { rerender } = render(<FileTree {...defaultProps()} refreshKey={0} />);
await screen.findByText('src');
await user.click(screen.getByText('src'));
await screen.findByText('index.ts');
await user.click(screen.getByText('src'));
await waitFor(() => expect(screen.queryByText('index.ts')).not.toBeInTheDocument());
mockLoadDir.mockImplementation(async (relPath: string) => {
if (relPath === '') return ROOT_ENTRIES;
if (relPath === 'src') return [makeFile('index.ts'), makeFile('added.ts')];
throw new Error(`unexpected path ${relPath}`);
});
mockLoadDir.mockClear();
rerender(<FileTree {...defaultProps()} refreshKey={1} />);
await waitFor(() => expect(mockLoadDir).toHaveBeenCalledWith(''));
expect(mockLoadDir).not.toHaveBeenCalledWith('src');
await user.click(screen.getByText('src'));
expect(await screen.findByText('added.ts')).toBeInTheDocument();
expect(mockLoadDir).toHaveBeenCalledWith('src');
});
it('clears a root-load error when a later soft refresh succeeds', async () => {
mockLoadDir.mockRejectedValue(new Error('Network error'));
const { rerender } = render(<FileTree {...defaultProps()} refreshKey={0} />);
expect(await screen.findByText('Network error')).toBeInTheDocument();
mockLoadDir.mockImplementation(async (relPath: string) => {
if (relPath === '') return ROOT_ENTRIES;
throw new Error(`unexpected path ${relPath}`);
});
rerender(<FileTree {...defaultProps()} refreshKey={1} />);
expect(await screen.findByText('src')).toBeInTheDocument();
expect(screen.getByText('README.md')).toBeInTheDocument();
expect(screen.queryByText('Network error')).not.toBeInTheDocument();
});
});
@@ -93,7 +93,7 @@ export function VolumeBrowserSheet({ volumeName, onClose }: VolumeBrowserSheetPr
<div className="grid grid-cols-[260px_1fr] gap-3 px-6 py-5 flex-1 min-h-0 max-md:grid-cols-1 max-md:grid-rows-[40%_1fr] max-md:px-4">
<div className="rounded-md border border-card-border bg-card overflow-hidden">
<FileTree
key={`${volumeName}:${refreshKey}`}
key={volumeName}
sourceKey={volumeName}
loadDir={loadDir}
refreshKey={refreshKey}