From 47beb01ff0adc066c6f945d1d1c6dc10f94a6ec2 Mon Sep 17 00:00:00 2001 From: Nihesh Rachakonda Date: Sun, 18 Oct 2026 21:38:53 +0530 Subject: [PATCH] fix(ui): stop reloads from retaining every previous review currentReviewVerticalStop was rebuilt whenever the measured stops changed, which happens one render after a reload swaps the files. That render's scope still held the resolveCommentRequest built for the new files, whose scope held the previous currentReviewVerticalStop, and so on back to the first load. In --watch every reload kept the old DiffFiles and row plans alive. Read the stops through a ref so the callback stays stable and the chain never forms. --- .changeset/release-reloaded-review-renders.md | 5 +++ .../src/ui/hooks/useTerminalReview.test.tsx | 31 +++++++++++++++++++ .../hunk/src/ui/hooks/useTerminalReview.ts | 13 ++++++-- 3 files changed, 46 insertions(+), 3 deletions(-) create mode 100644 .changeset/release-reloaded-review-renders.md diff --git a/.changeset/release-reloaded-review-renders.md b/.changeset/release-reloaded-review-renders.md new file mode 100644 index 000000000..49a32e3ea --- /dev/null +++ b/.changeset/release-reloaded-review-renders.md @@ -0,0 +1,5 @@ +--- +"hunkdiff": patch +--- + +Fix a memory leak in `--watch` and other live reloads where every reload kept the previous review's files and render state alive, so long-running sessions grew without bound. diff --git a/packages/hunk/src/ui/hooks/useTerminalReview.test.tsx b/packages/hunk/src/ui/hooks/useTerminalReview.test.tsx index f9e31cfd3..a1f60b990 100644 --- a/packages/hunk/src/ui/hooks/useTerminalReview.test.tsx +++ b/packages/hunk/src/ui/hooks/useTerminalReview.test.tsx @@ -514,6 +514,37 @@ describe("useTerminalReview", () => { } }); + test("releases files from earlier reloads once later reloads replace them", async () => { + const { controllerRef, setFilesRef, setup } = await renderTerminalReview([createTwoHunkFile()]); + const reloadedFiles: WeakRef[] = []; + + try { + await flush(setup); + + // Each reload replaces the files first and the measured stops one render later, which is the + // staggered update order that let memoized callbacks chain every earlier render together. + for (let reload = 0; reload < 12; reload += 1) { + const file = reload % 2 === 0 ? createSingleHunkFile() : createTwoHunkFile(); + reloadedFiles.push(new WeakRef(file)); + await act(async () => { + expectValue(setFilesRef.current)([file]); + }); + await flush(setup); + } + + expect(expectValue(controllerRef.current).visibleFiles).toHaveLength(1); + Bun.gc(true); + const retainedEarlyReloads = reloadedFiles + .slice(0, -4) + .filter((reference) => reference.deref() !== undefined); + expect(retainedEarlyReloads).toHaveLength(0); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); + test("moves through visible files with clamped file-header alignment", async () => { const controllerRef: { current: TerminalReview | null } = { current: null }; const setup = await testRender( diff --git a/packages/hunk/src/ui/hooks/useTerminalReview.ts b/packages/hunk/src/ui/hooks/useTerminalReview.ts index f23ea8b62..3a88607f1 100644 --- a/packages/hunk/src/ui/hooks/useTerminalReview.ts +++ b/packages/hunk/src/ui/hooks/useTerminalReview.ts @@ -881,17 +881,24 @@ export function useTerminalReview({ [applyLineCursor, keyByFileId, runIntent], ); + // Read stops through a ref so this callback stays stable across reloads. Closures from one + // render share its scope, so a callback rebuilt when stops change would retain the render that + // rebuilt `resolveCommentRequest` for new files, and through it every earlier review generation. + const reviewVerticalStopsRef = useRef(reviewVerticalStops); + reviewVerticalStopsRef.current = reviewVerticalStops; + /** Read the exact rendered stop that currently owns keyboard focus. */ const currentReviewVerticalStop = useCallback((): ReviewVerticalStop | null => { const activeNoteId = selectActiveStoredReviewNote(store.getSnapshot())?.note.id; if (activeNoteId) { return ( - reviewVerticalStops.find((stop) => stop.kind === "note" && stop.noteId === activeNoteId) ?? - null + reviewVerticalStopsRef.current.find( + (stop) => stop.kind === "note" && stop.noteId === activeNoteId, + ) ?? null ); } return lineCursorRef.current ? { kind: "line", cursor: lineCursorRef.current } : null; - }, [reviewVerticalStops, store]); + }, [store]); /** Focus one measured note stop through its current semantic owner. */ const focusReviewNoteStop = useCallback(