feat: enhance EditorFileWatcher and FileSearchService for improved performance and event handling
- Introduced debouncing in EditorFileWatcher to aggregate rapid file system events, reducing unnecessary callbacks. - Enhanced FileSearchService to parallelize file checks and improve efficiency in collecting file paths from directories. - Updated EditorFileTree and ProjectEditorOverlay components to optimize state management and reduce re-renders using Zustand's shallow comparison. - Added tests for EditorFileWatcher to ensure proper event handling and debouncing functionality.
This commit is contained in:
parent
509f7e82ab
commit
533f9b9e06
6 changed files with 112 additions and 61 deletions
|
|
@ -35,6 +35,10 @@ const MAX_DEPTH = 20;
|
|||
export class EditorFileWatcher {
|
||||
private watcher: FSWatcher | null = null;
|
||||
private projectRoot: string | null = null;
|
||||
private pendingEvents = new Map<string, EditorFileChangeEvent['type']>();
|
||||
private flushTimer: ReturnType<typeof setTimeout> | null = null;
|
||||
private onChangeCallback: ((event: EditorFileChangeEvent) => void) | null = null;
|
||||
private readonly DEBOUNCE_MS = 150;
|
||||
|
||||
/**
|
||||
* Start watching a project directory.
|
||||
|
|
@ -53,13 +57,17 @@ export class EditorFileWatcher {
|
|||
depth: MAX_DEPTH,
|
||||
});
|
||||
|
||||
this.onChangeCallback = onChange;
|
||||
|
||||
const emitSafe = (type: EditorFileChangeEvent['type'], filePath: string): void => {
|
||||
// SEC-2: validate path is within project root before sending to renderer
|
||||
if (!isPathWithinRoot(filePath, projectRoot)) {
|
||||
log.warn('Watcher event outside project root, ignoring:', filePath);
|
||||
return;
|
||||
}
|
||||
onChange({ type, path: filePath });
|
||||
// Aggregate rapid events — only the last event type per path is kept
|
||||
this.pendingEvents.set(filePath, type);
|
||||
this.scheduleFlush();
|
||||
};
|
||||
|
||||
this.watcher.on('change', (p) => emitSafe('change', p));
|
||||
|
|
@ -75,6 +83,12 @@ export class EditorFileWatcher {
|
|||
* Stop watching. Safe to call multiple times.
|
||||
*/
|
||||
stop(): void {
|
||||
if (this.flushTimer) {
|
||||
clearTimeout(this.flushTimer);
|
||||
this.flushTimer = null;
|
||||
}
|
||||
this.pendingEvents.clear();
|
||||
this.onChangeCallback = null;
|
||||
if (this.watcher) {
|
||||
log.info('Stopping file watcher');
|
||||
void this.watcher.close();
|
||||
|
|
@ -83,6 +97,23 @@ export class EditorFileWatcher {
|
|||
this.projectRoot = null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Flush pending events — debounced to aggregate rapid FS changes
|
||||
* (e.g. git checkout, bulk format). Fires once after 150ms of quiet.
|
||||
*/
|
||||
private scheduleFlush(): void {
|
||||
if (this.flushTimer) return;
|
||||
this.flushTimer = setTimeout(() => {
|
||||
this.flushTimer = null;
|
||||
const events = new Map(this.pendingEvents);
|
||||
this.pendingEvents.clear();
|
||||
if (!this.onChangeCallback) return;
|
||||
for (const [filePath, type] of events) {
|
||||
this.onChangeCallback({ type, path: filePath });
|
||||
}
|
||||
}, this.DEBOUNCE_MS);
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether the watcher is currently active.
|
||||
*/
|
||||
|
|
|
|||
|
|
@ -203,8 +203,11 @@ export class FileSearchService {
|
|||
return a.name.localeCompare(b.name);
|
||||
});
|
||||
|
||||
const candidates: string[] = [];
|
||||
const subdirs: string[] = [];
|
||||
|
||||
for (const entry of sorted) {
|
||||
if (signal?.aborted || files.length >= MAX_FILES) break;
|
||||
if (signal?.aborted) break;
|
||||
|
||||
const fullPath = path.join(dirPath, entry.name);
|
||||
|
||||
|
|
@ -216,28 +219,40 @@ export class FileSearchService {
|
|||
|
||||
if (entry.isDirectory()) {
|
||||
if (IGNORED_DIRS.has(entry.name) || entry.name.startsWith('.')) continue;
|
||||
await this.collectFiles(projectRoot, fullPath, files, signal);
|
||||
subdirs.push(fullPath);
|
||||
} else if (entry.isFile()) {
|
||||
if (IGNORED_FILES.has(entry.name)) continue;
|
||||
|
||||
// Skip files > 1MB
|
||||
try {
|
||||
const stat = await fs.stat(fullPath);
|
||||
if (stat.size > MAX_FILE_SIZE) continue;
|
||||
} catch {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Skip binary files (quick check via first 512 bytes)
|
||||
try {
|
||||
if (await isBinaryFile(fullPath)) continue;
|
||||
} catch {
|
||||
continue;
|
||||
}
|
||||
|
||||
files.push(fullPath);
|
||||
candidates.push(fullPath);
|
||||
}
|
||||
}
|
||||
|
||||
// Parallel stat + binary check (batched by 20 for I/O concurrency)
|
||||
const CHECK_CONCURRENCY = 20;
|
||||
for (let i = 0; i < candidates.length; i += CHECK_CONCURRENCY) {
|
||||
if (signal?.aborted || files.length >= MAX_FILES) break;
|
||||
const batch = candidates.slice(i, i + CHECK_CONCURRENCY);
|
||||
const results = await Promise.all(
|
||||
batch.map(async (fp) => {
|
||||
try {
|
||||
const stat = await fs.stat(fp);
|
||||
if (stat.size > MAX_FILE_SIZE) return null;
|
||||
if (await isBinaryFile(fp)) return null;
|
||||
return fp;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
})
|
||||
);
|
||||
for (const fp of results) {
|
||||
if (fp && files.length < MAX_FILES) files.push(fp);
|
||||
}
|
||||
}
|
||||
|
||||
// Recurse into subdirectories
|
||||
for (const subdir of subdirs) {
|
||||
if (signal?.aborted || files.length >= MAX_FILES) break;
|
||||
await this.collectFiles(projectRoot, subdir, files, signal);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -79,14 +79,6 @@ export class GitStatusService {
|
|||
}
|
||||
|
||||
try {
|
||||
// Check if it's a git repo first
|
||||
const isRepo = await this.isGitRepo();
|
||||
if (!isRepo) {
|
||||
const result: GitStatusResult = { files: [], isGitRepo: false, branch: null };
|
||||
this.setCacheResult(result);
|
||||
return result;
|
||||
}
|
||||
|
||||
const statusResult = await this.git.status();
|
||||
const files = mapStatusResult(statusResult);
|
||||
const branch = statusResult.current ?? null;
|
||||
|
|
@ -96,18 +88,10 @@ export class GitStatusService {
|
|||
return result;
|
||||
} catch (error) {
|
||||
log.error('Failed to get git status:', error);
|
||||
// Graceful degradation: return empty non-repo result
|
||||
return { files: [], isGitRepo: false, branch: null };
|
||||
}
|
||||
}
|
||||
|
||||
private async isGitRepo(): Promise<boolean> {
|
||||
if (!this.git) return false;
|
||||
try {
|
||||
await this.git.revparse(['--is-inside-work-tree']);
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
// Graceful degradation: cache negative result to avoid repeated git calls
|
||||
const result: GitStatusResult = { files: [], isGitRepo: false, branch: null };
|
||||
this.setCacheResult(result);
|
||||
return result;
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -29,6 +29,7 @@ import { useStore } from '@renderer/store';
|
|||
import { sortTreeNodes } from '@renderer/utils/fileTreeBuilder';
|
||||
import { useVirtualizer } from '@tanstack/react-virtual';
|
||||
import { ChevronDown, ChevronRight, Folder, FolderOpen, Lock } from 'lucide-react';
|
||||
import { useShallow } from 'zustand/react/shallow';
|
||||
|
||||
import { EditorContextMenu } from './EditorContextMenu';
|
||||
import { FileIcon } from './FileIcon';
|
||||
|
|
@ -77,20 +78,27 @@ export const EditorFileTree = ({
|
|||
selectedFilePath,
|
||||
onFileSelect,
|
||||
}: EditorFileTreeProps): React.ReactElement => {
|
||||
const fileTree = useStore((s) => s.editorFileTree);
|
||||
const expandedDirs = useStore((s) => s.editorExpandedDirs);
|
||||
// Data selectors — grouped with useShallow to prevent unnecessary re-renders
|
||||
const { fileTree, expandedDirs, loading, error, gitFiles, projectPath } = useStore(
|
||||
useShallow((s) => ({
|
||||
fileTree: s.editorFileTree,
|
||||
expandedDirs: s.editorExpandedDirs,
|
||||
loading: s.editorFileTreeLoading,
|
||||
error: s.editorFileTreeError,
|
||||
gitFiles: s.editorGitFiles,
|
||||
projectPath: s.editorProjectPath,
|
||||
}))
|
||||
);
|
||||
|
||||
// Actions — stable references in Zustand, no grouping needed
|
||||
const expandDirectory = useStore((s) => s.expandDirectory);
|
||||
const collapseDirectory = useStore((s) => s.collapseDirectory);
|
||||
const loading = useStore((s) => s.editorFileTreeLoading);
|
||||
const error = useStore((s) => s.editorFileTreeError);
|
||||
const createFileInTree = useStore((s) => s.createFileInTree);
|
||||
const createDirInTree = useStore((s) => s.createDirInTree);
|
||||
const deleteFileFromTree = useStore((s) => s.deleteFileFromTree);
|
||||
const moveFileInTree = useStore((s) => s.moveFileInTree);
|
||||
const renameFileInTree = useStore((s) => s.renameFileInTree);
|
||||
const openFile = useStore((s) => s.openFile);
|
||||
const gitFiles = useStore((s) => s.editorGitFiles);
|
||||
const projectPath = useStore((s) => s.editorProjectPath);
|
||||
|
||||
const [newItemState, setNewItemState] = useState<NewItemState | null>(null);
|
||||
const [renamingPath, setRenamingPath] = useState<string | null>(null);
|
||||
|
|
@ -473,7 +481,7 @@ export const EditorFileTree = ({
|
|||
<DraggableTreeItem
|
||||
item={item}
|
||||
activeNodePath={activeNodePath}
|
||||
gitStatusMap={gitStatusMap}
|
||||
gitStatus={gitStatusMap.get(item.node.fullPath)}
|
||||
dropTargetPath={dropTargetPath}
|
||||
isDragActive={!!draggedItem}
|
||||
onClick={handleNodeClick}
|
||||
|
|
@ -564,7 +572,7 @@ RootDropZone.displayName = 'RootDropZone';
|
|||
interface DraggableTreeItemProps {
|
||||
item: FlatTreeItem;
|
||||
activeNodePath: string | null;
|
||||
gitStatusMap: Map<string, GitFileStatusType>;
|
||||
gitStatus?: GitFileStatusType;
|
||||
dropTargetPath: string | null;
|
||||
isDragActive: boolean;
|
||||
onClick: (node: TreeNode<FileTreeEntry>) => void;
|
||||
|
|
@ -578,7 +586,7 @@ const DraggableTreeItem = React.memo(
|
|||
({
|
||||
item,
|
||||
activeNodePath,
|
||||
gitStatusMap,
|
||||
gitStatus,
|
||||
dropTargetPath,
|
||||
isDragActive,
|
||||
onClick,
|
||||
|
|
@ -689,9 +697,7 @@ const DraggableTreeItem = React.memo(
|
|||
) : (
|
||||
<span className="truncate">{node.name}</span>
|
||||
)}
|
||||
{!isRenaming && node.data && gitStatusMap.has(node.data.path) && (
|
||||
<GitStatusBadge status={gitStatusMap.get(node.data.path)!} />
|
||||
)}
|
||||
{!isRenaming && gitStatus && <GitStatusBadge status={gitStatus} />}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -31,6 +31,7 @@ import {
|
|||
RotateCcw,
|
||||
X,
|
||||
} from 'lucide-react';
|
||||
import { useShallow } from 'zustand/react/shallow';
|
||||
|
||||
import { CodeMirrorEditor } from './CodeMirrorEditor';
|
||||
import { EditorBinaryState } from './EditorBinaryState';
|
||||
|
|
@ -73,23 +74,29 @@ export const ProjectEditorOverlay = ({
|
|||
onClose,
|
||||
onEditorAction,
|
||||
}: ProjectEditorOverlayProps): React.ReactElement => {
|
||||
// Data selectors — grouped with useShallow to prevent unnecessary re-renders
|
||||
const { activeTabId, openTabs, modifiedFiles, saveErrors, externalChanges, conflictFile } =
|
||||
useStore(
|
||||
useShallow((s) => ({
|
||||
activeTabId: s.editorActiveTabId,
|
||||
openTabs: s.editorOpenTabs,
|
||||
modifiedFiles: s.editorModifiedFiles,
|
||||
saveErrors: s.editorSaveError,
|
||||
externalChanges: s.editorExternalChanges,
|
||||
conflictFile: s.editorConflictFile,
|
||||
}))
|
||||
);
|
||||
|
||||
// Actions — stable references in Zustand, no grouping needed
|
||||
const openEditor = useStore((s) => s.openEditor);
|
||||
const closeEditor = useStore((s) => s.closeEditor);
|
||||
const openFile = useStore((s) => s.openFile);
|
||||
const closeEditorTab = useStore((s) => s.closeEditorTab);
|
||||
const saveFile = useStore((s) => s.saveFile);
|
||||
const activeTabId = useStore((s) => s.editorActiveTabId);
|
||||
const openTabs = useStore((s) => s.editorOpenTabs);
|
||||
const modifiedFiles = useStore((s) => s.editorModifiedFiles);
|
||||
const saveErrors = useStore((s) => s.editorSaveError);
|
||||
const hasUnsavedChanges = useStore((s) => s.hasUnsavedChanges);
|
||||
const saveAllFiles = useStore((s) => s.saveAllFiles);
|
||||
const discardChanges = useStore((s) => s.discardChanges);
|
||||
|
||||
// Iter-5: git, watcher, conflict
|
||||
const externalChanges = useStore((s) => s.editorExternalChanges);
|
||||
const clearExternalChange = useStore((s) => s.clearExternalChange);
|
||||
const conflictFile = useStore((s) => s.editorConflictFile);
|
||||
const forceOverwrite = useStore((s) => s.forceOverwrite);
|
||||
const resolveConflict = useStore((s) => s.resolveConflict);
|
||||
const setFileMtime = useStore((s) => s.setFileMtime);
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@
|
|||
* Tests for EditorFileWatcher — start/stop, event filtering, path security.
|
||||
*/
|
||||
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
// Mock chokidar
|
||||
const mockOn = vi.fn().mockReturnThis();
|
||||
|
|
@ -43,11 +43,16 @@ describe('EditorFileWatcher', () => {
|
|||
let watcher: EditorFileWatcher;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
vi.resetAllMocks();
|
||||
mockOn.mockReturnThis();
|
||||
watcher = new EditorFileWatcher();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
describe('start', () => {
|
||||
it('creates chokidar watcher with correct options', () => {
|
||||
const onChange = vi.fn();
|
||||
|
|
@ -79,6 +84,7 @@ describe('EditorFileWatcher', () => {
|
|||
// Simulate chokidar 'change' event
|
||||
const changeHandler = mockOn.mock.calls.find((c) => c[0] === 'change')?.[1];
|
||||
changeHandler?.('/Users/test/project/src/index.ts');
|
||||
vi.advanceTimersByTime(150);
|
||||
|
||||
expect(onChange).toHaveBeenCalledWith({
|
||||
type: 'change',
|
||||
|
|
@ -92,6 +98,7 @@ describe('EditorFileWatcher', () => {
|
|||
|
||||
const addHandler = mockOn.mock.calls.find((c) => c[0] === 'add')?.[1];
|
||||
addHandler?.('/Users/test/project/new-file.ts');
|
||||
vi.advanceTimersByTime(150);
|
||||
|
||||
expect(onChange).toHaveBeenCalledWith({
|
||||
type: 'create',
|
||||
|
|
@ -105,6 +112,7 @@ describe('EditorFileWatcher', () => {
|
|||
|
||||
const unlinkHandler = mockOn.mock.calls.find((c) => c[0] === 'unlink')?.[1];
|
||||
unlinkHandler?.('/Users/test/project/old-file.ts');
|
||||
vi.advanceTimersByTime(150);
|
||||
|
||||
expect(onChange).toHaveBeenCalledWith({
|
||||
type: 'delete',
|
||||
|
|
|
|||
Loading…
Reference in a new issue