diff --git a/apps/code/src/renderer/features/code-review/components/ReviewShell.test.tsx b/apps/code/src/renderer/features/code-review/components/ReviewShell.test.tsx index beb911c00..0a9d3930a 100644 --- a/apps/code/src/renderer/features/code-review/components/ReviewShell.test.tsx +++ b/apps/code/src/renderer/features/code-review/components/ReviewShell.test.tsx @@ -1,11 +1,19 @@ -import { render } from "@testing-library/react"; +import { render, renderHook } from "@testing-library/react"; +import { useEffect, useState } from "react"; import { describe, expect, it, vi } from "vitest"; vi.mock("@renderer/features/code-review/stores/reviewNavigationStore", () => ({ useReviewNavigationStore: vi.fn(), })); vi.mock("@features/code-editor/stores/diffViewerStore", () => ({ - useDiffViewerStore: vi.fn(), + useDiffViewerStore: vi.fn((selector) => + selector({ + viewMode: "unified", + wordWrap: true, + loadFullFiles: false, + wordDiffs: false, + }), + ), })); vi.mock("@features/task-detail/components/ChangesPanel", () => ({ ChangesPanel: () => null, @@ -14,7 +22,7 @@ vi.mock("@features/git-interaction/utils/diffStats", () => ({ computeDiffStats: () => ({ linesAdded: 0, linesRemoved: 0 }), })); vi.mock("@stores/themeStore", () => ({ - useThemeStore: vi.fn(() => ({ isDarkMode: false })), + useThemeStore: vi.fn((selector) => selector({ isDarkMode: false })), })); vi.mock("@pierre/diffs/react", () => ({ WorkerPoolContextProvider: ({ children }: { children: React.ReactNode }) => @@ -32,7 +40,11 @@ vi.mock("@features/sessions/service/service", () => ({ getSessionService: vi.fn(), })); -import { DeferredDiffPlaceholder, DiffFileHeader } from "./ReviewShell"; +import { + DeferredDiffPlaceholder, + DiffFileHeader, + useReviewState, +} from "./ReviewShell"; type FileDiffMetadata = import("@pierre/diffs/react").FileDiffMetadata; @@ -75,6 +87,29 @@ function renderHeader(path: string) { return { diff, deferred }; } +describe("useReviewState", () => { + it("does not loop when changedFiles is recreated during render", () => { + function useUnstableChangedFiles() { + const [_tick, setTick] = useState(0); + useEffect(() => setTick(1), []); + + return useReviewState( + [ + { + path: "dist/bundle.js", + status: "modified", + linesAdded: 600, + linesRemoved: 0, + }, + ], + ["dist/bundle.js"], + ); + } + + expect(() => renderHook(() => useUnstableChangedFiles())).not.toThrow(); + }); +}); + describe.each([ ["DiffFileHeader", "diff" as const], ["DeferredDiffPlaceholder", "deferred" as const], diff --git a/apps/code/src/renderer/features/code-review/components/ReviewShell.tsx b/apps/code/src/renderer/features/code-review/components/ReviewShell.tsx index ae0f1b05f..147554429 100644 --- a/apps/code/src/renderer/features/code-review/components/ReviewShell.tsx +++ b/apps/code/src/renderer/features/code-review/components/ReviewShell.tsx @@ -81,6 +81,13 @@ const AUTO_COLLAPSE_PATTERNS = [ export type DeferredReason = "deleted" | "large" | "generated" | "unavailable"; +function getDeferredPathsKey(paths: Map): string { + return Array.from(paths.entries()) + .sort(([left], [right]) => left.localeCompare(right)) + .map(([path, reason]) => `${path}:${reason}`) + .join("\n"); +} + export function computeAutoDeferred( files: { path: string; @@ -158,11 +165,17 @@ function useCollapseState( () => new Set(), ); - const [lastDeferred, setLastDeferred] = useState(deferredPaths); - if (deferredPaths !== lastDeferred) { - setLastDeferred(deferredPaths); + const deferredPathsKey = useMemo( + () => getDeferredPathsKey(deferredPaths), + [deferredPaths], + ); + + const lastDeferredPathsKey = useRef(deferredPathsKey); + useEffect(() => { + if (deferredPathsKey === lastDeferredPathsKey.current) return; + lastDeferredPathsKey.current = deferredPathsKey; setRevealedFiles(new Set()); - } + }, [deferredPathsKey]); const revealFile = useCallback((filePath: string) => { setRevealedFiles((prev) => new Set(prev).add(filePath));