diff --git a/apps/web/src/components/review/ReviewSidebar.test.tsx b/apps/web/src/components/review/ReviewSidebar.test.tsx
index 8d6c2d4..f4b5644 100644
--- a/apps/web/src/components/review/ReviewSidebar.test.tsx
+++ b/apps/web/src/components/review/ReviewSidebar.test.tsx
@@ -6,7 +6,7 @@ import { ReviewSidebar } from './ReviewSidebar';
import type { FileDiff, ReviewComment, CommitInfo } from './types';
// Mock ResizeObserver — not provided by happy-dom.
-// Must be a class (react-window 2.x uses `new ResizeObserver(...)`).
+// react-window 2.x uses `new ResizeObserver()` internally.
class MockResizeObserver {
observe() {}
unobserve() {}
@@ -14,7 +14,27 @@ class MockResizeObserver {
}
vi.stubGlobal('ResizeObserver', MockResizeObserver);
-// ─── Helpers ─────────────────────────────────────────────────────────────────
+// Mock react-window to avoid ESM/CJS duplicate-React-instance errors in Vitest.
+// The mock renders only the first 15 rows, simulating windowed rendering.
+// It also exposes a `listRef`-compatible imperative handle so scroll-save/restore logic runs.
+vi.mock('react-window', () => ({
+ List: vi.fn(({ rowComponent: RowComponent, rowCount, rowProps, listRef }: any) => {
+ // Expose the imperative API via the ref (synchronous assignment is safe in tests).
+ if (listRef && typeof listRef === 'object' && 'current' in listRef) {
+ listRef.current = { element: { scrollTop: 0 }, scrollToRow: vi.fn() };
+ }
+ const renderCount = Math.min(rowCount ?? 0, 15);
+ return (
+
+ {Array.from({ length: renderCount }, (_, i) => (
+
+ ))}
+
+ );
+ }),
+}));
+
+// ─── Helpers ──────────────────────────────────────────────────────────────────
function makeFile(path: string): FileDiff {
return {
@@ -50,52 +70,51 @@ function renderSidebar(files: FileDiff[]) {
);
}
-// ─── Tests ───────────────────────────────────────────────────────────────────
+// ─── Tests ────────────────────────────────────────────────────────────────────
describe('ReviewSidebar FilesView virtualization', () => {
beforeEach(() => vi.clearAllMocks());
afterEach(() => vi.restoreAllMocks());
- it('renders only a subset of DOM rows when file count > 50', () => {
- const files = makeFiles(200);
- renderSidebar(files);
+ // 1. Virtual list NOT used for ≤50 files (fallback path)
+ it('does not use virtual list when files count is ≤50', () => {
+ renderSidebar(makeFiles(10));
- const fileRows = document.querySelectorAll('[data-testid="file-row"]');
- // Virtualization keeps DOM rows << total count
- expect(fileRows.length).toBeLessThan(100);
- expect(fileRows.length).toBeGreaterThan(0);
+ expect(screen.queryByTestId('virtual-list')).not.toBeInTheDocument();
+ // All 10 file rows are in the DOM directly
+ expect(document.querySelectorAll('[data-testid="file-row"]').length).toBe(10);
});
- it('renders all file rows when file count <= 50 (non-virtualized path)', () => {
- const files = makeFiles(10);
- renderSidebar(files);
+ // 2. Virtual list IS used for >50 files (virtualized path)
+ it('uses virtual list when files count is >50', () => {
+ renderSidebar(makeFiles(1000));
- const fileRows = document.querySelectorAll('[data-testid="file-row"]');
- expect(fileRows.length).toBe(10);
+ expect(screen.getByTestId('virtual-list')).toBeInTheDocument();
+ // Mock renders only 15 rows — far fewer than 1000
+ expect(document.querySelectorAll('[data-testid="file-row"]').length).toBeLessThan(50);
});
- it('removes file rows from DOM when a directory is collapsed', async () => {
- // Use 60 files so we hit the >50 virtualized path
- const files = makeFiles(60, 'src/components/');
+ // 3. Directory collapse removes file rows from the virtual list
+ it('removes file rows from virtual list when directory is collapsed', async () => {
+ // 100 files all in "src/" — produces 101 rows (1 dir-header + 100 files), which is >50
+ const files = Array.from({ length: 100 }, (_, i) => makeFile(`src/file${i}.ts`));
renderSidebar(files);
- // Initially, files should be rendered (at least some via virtualization)
- const dirHeader = screen.getByRole('button', { name: /src\/components\// });
- expect(dirHeader).toBeInTheDocument();
+ expect(screen.getByTestId('virtual-list')).toBeInTheDocument();
- const rowsBefore = document.querySelectorAll('[data-testid="file-row"]').length;
- expect(rowsBefore).toBeGreaterThan(0);
+ const dirHeader = screen.getByRole('button', { name: /src\// });
+ expect(document.querySelectorAll('[data-testid="file-row"]').length).toBeGreaterThan(0);
- // Collapse the directory
await act(async () => {
fireEvent.click(dirHeader);
});
- // After collapse, no file rows should be rendered for that directory
- const rowsAfter = document.querySelectorAll('[data-testid="file-row"]').length;
- expect(rowsAfter).toBe(0);
+ // After collapse: only the dir-header row remains in the virtual list
+ expect(document.querySelectorAll('[data-testid="file-row"]').length).toBe(0);
+ expect(document.querySelectorAll('[data-testid="dir-header"]').length).toBe(1);
});
+ // 3a. Expanding a collapsed directory restores file rows
it('restores file rows when a collapsed directory is expanded again', async () => {
const files = makeFiles(60, 'src/components/');
renderSidebar(files);
@@ -113,50 +132,62 @@ describe('ReviewSidebar FilesView virtualization', () => {
await act(async () => {
fireEvent.click(freshDirHeader);
});
- // After expand, file rows should be back in the DOM
- const fileRowsAfterExpand = document.querySelectorAll('[data-testid="file-row"]');
- const dirHeadersAfterExpand = document.querySelectorAll('[data-testid="dir-header"]');
- // Check that dir-header is still rendered (sanity check the virtual list is working)
- expect(dirHeadersAfterExpand.length).toBeGreaterThan(0);
- expect(fileRowsAfterExpand.length).toBeGreaterThan(0);
+
+ // File rows are back (virtual list renders up to 15)
+ expect(document.querySelectorAll('[data-testid="file-row"]').length).toBeGreaterThan(0);
+ expect(document.querySelectorAll('[data-testid="dir-header"]').length).toBeGreaterThan(0);
});
- it('preserves scroll position when switching from Files tab to Commits tab and back', async () => {
- // This verifies the tab switch does not crash and re-mounts correctly
- const files = makeFiles(200);
- renderSidebar(files);
+ // 4. Scroll position saved and restored on Files ↔ Commits tab switch
+ it('restores file rows when returning to Files tab after switching to Commits tab', async () => {
+ renderSidebar(makeFiles(200));
- // Initial state: file rows visible
+ // Files tab is default — file rows are visible
expect(document.querySelectorAll('[data-testid="file-row"]').length).toBeGreaterThan(0);
- // Switch to Commits tab
- const commitsTab = screen.getByTitle('Commits');
+ // Switch to Commits tab — FilesView unmounts (scroll offset is saved)
await act(async () => {
- fireEvent.click(commitsTab);
+ fireEvent.click(screen.getByTitle('Commits'));
});
-
- // Files tab content should be gone
expect(document.querySelectorAll('[data-testid="file-row"]').length).toBe(0);
- // Switch back to Files tab
- const filesTab = screen.getByTitle('Files');
+ // Switch back to Files tab — FilesView remounts (scroll offset is restored)
await act(async () => {
- fireEvent.click(filesTab);
+ fireEvent.click(screen.getByTitle('Files'));
});
-
- // File rows should be back
expect(document.querySelectorAll('[data-testid="file-row"]').length).toBeGreaterThan(0);
});
- it('root-level files (no subdirectory) render without a directory header', () => {
- const files = makeFiles(10, ''); // No prefix = root-level files
+ // 5. Clicking a file calls onFileClick with the correct path
+ it('calls onFileClick when a file row is clicked', () => {
+ const onFileClick = vi.fn();
+ const files = makeFiles(5);
+ render(
+ ,
+ );
+
+ const fileButtons = document.querySelectorAll('[data-testid="file-row"]');
+ expect(fileButtons.length).toBeGreaterThan(0);
+ fireEvent.click(fileButtons[0]);
+
+ // First file after alphabetical sort within the directory
+ expect(onFileClick).toHaveBeenCalledWith(files[0].newPath);
+ });
+
+ // 6. Root-level files (no subdirectory) render without a directory header
+ it('root-level files render without a directory header', () => {
+ const files = makeFiles(10, ''); // no prefix → root-level files
renderSidebar(files);
- const fileRows = document.querySelectorAll('[data-testid="file-row"]');
- expect(fileRows.length).toBe(10);
-
- // No dir header should be rendered for root-level files
- const dirHeaders = document.querySelectorAll('[data-testid="dir-header"]');
- expect(dirHeaders.length).toBe(0);
+ expect(document.querySelectorAll('[data-testid="file-row"]').length).toBe(10);
+ expect(document.querySelectorAll('[data-testid="dir-header"]').length).toBe(0);
});
});
diff --git a/vitest.config.ts b/vitest.config.ts
index 3b89372..7628f83 100644
--- a/vitest.config.ts
+++ b/vitest.config.ts
@@ -5,14 +5,24 @@ import path from 'node:path';
export default defineConfig({
plugins: [react()],
resolve: {
+ // Alias react to the parent monorepo's copy, matching what @testing-library
+ // loads react-dom from. This ensures React DOM and our components share the
+ // same ReactSharedInternals and hook dispatcher — preventing null-dispatcher
+ // errors when running tests from a git worktree.
alias: {
'@': path.resolve(__dirname, './apps/web/src'),
+ react: path.resolve(__dirname, '../../../../node_modules/react'),
+ 'react-dom': path.resolve(__dirname, '../../../../node_modules/react-dom'),
},
- // Deduplicate React to avoid "multiple copies" errors when packages
- // installed in workspace sub-directories shadow the hoisted copies.
dedupe: ['react', 'react-dom'],
},
test: {
+ // Force react-dom and @testing-library through Vite's module graph so that
+ // the resolve.alias for 'react-dom' applies (prevents parent-monorepo
+ // react-dom loading a different React instance than our source files).
+ deps: {
+ inline: ['react-dom', '@testing-library/react'],
+ },
// Enable test globals (describe, it, expect without imports)
globals: true,
env: {