diff --git a/.changeset/legacy-sidebar-mutable-files.md b/.changeset/legacy-sidebar-mutable-files.md new file mode 100644 index 000000000..cbc2e852e --- /dev/null +++ b/.changeset/legacy-sidebar-mutable-files.md @@ -0,0 +1,5 @@ +--- +"hunkdiff": patch +--- + +Deprecated `registerSidebarView` components now receive a mutable copy of the files list, so sorting or appending to it no longer throws. diff --git a/packages/hunk/src/extensions/panes.ts b/packages/hunk/src/extensions/panes.ts index 5915915cb..aa6d2f0a3 100644 --- a/packages/hunk/src/extensions/panes.ts +++ b/packages/hunk/src/extensions/panes.ts @@ -1,7 +1,11 @@ import type { ExtensionPane, + ExtensionPaneComponent, ExtensionPanePlacement, + ExtensionPaneProps, ExtensionPaneSize, + ExtensionSidebarComponent, + ExtensionSidebarViewProps, } from "../extension-api/types"; const DEFAULT_VERTICAL_PANE_WIDTH = Object.freeze({ preferred: 34, min: 22 }); @@ -29,3 +33,29 @@ export function extensionPaneSize( defaultExtensionPaneSize(placement) ); } + +/** + * Mount a deprecated sidebar component through the pane contract. + * + * The legacy props declare a mutable `files` array, so each render receives a + * shallow copy of the frozen host list; the file views themselves stay frozen. + * Pane-only fields (`review`, `reviewGeneration`, `placement`, `height`, and + * `currentLine`) are deliberately not forwarded. + */ +export function adaptLegacySidebarComponent( + component: ExtensionSidebarComponent, +): ExtensionPaneComponent { + return (props: ExtensionPaneProps) => { + const legacyProps: ExtensionSidebarViewProps = { + files: [...props.files], + selectedFileId: props.selectedFileId, + selectedHunkIndex: props.selectedHunkIndex, + width: props.width, + theme: props.theme, + keybindings: props.keybindings, + actions: props.actions, + }; + + return component(legacyProps); + }; +} diff --git a/packages/hunk/src/extensions/runExtension.test.ts b/packages/hunk/src/extensions/runExtension.test.ts index 49aef6000..067bbd6fa 100644 --- a/packages/hunk/src/extensions/runExtension.test.ts +++ b/packages/hunk/src/extensions/runExtension.test.ts @@ -1,4 +1,5 @@ import { describe, expect, test } from "bun:test"; +import type { ExtensionPaneProps, ExtensionSidebarViewProps } from "../extension-api/types"; import { resolveExtensionPanes } from "./apply"; import { runExtensionFactory, toInternalVcsAdapter } from "./runExtension"; import { @@ -363,12 +364,64 @@ describe("registerSidebarView", () => { id: "tree", placement: "left", width: { preferred: 34, min: 22, max: Number.MAX_SAFE_INTEGER }, - component, + component: expect.any(Function), }, }, ]); }); + test("adapts pane props to the deprecated sidebar props with a mutable files copy", () => { + const registry = createEmptyExtensionRegistry(); + const issues: ExtensionLoadIssue[] = []; + let received: ExtensionSidebarViewProps | undefined; + + runExtensionFactory({ + metadata: bundledMetadata("legacy-side"), + registry, + issues, + factory: (hunk) => { + hunk.registerSidebarView({ + id: "tree", + component: (props) => { + received = props; + return "legacy"; + }, + }); + }, + }); + + const files = Object.freeze([{ id: "a" }, { id: "b" }]); + const paneProps = { + review: null, + reviewGeneration: "generation", + files, + selectedFileId: "a", + selectedHunkIndex: 0, + placement: "left", + width: 30, + height: 12, + theme: {}, + keybindings: {}, + actions: {}, + currentLine: null, + } as unknown as ExtensionPaneProps; + + expect(registry.panes[0]?.pane.component(paneProps)).toBe("legacy"); + expect(received).toEqual({ + files: [...files], + selectedFileId: "a", + selectedHunkIndex: 0, + width: 30, + theme: paneProps.theme, + keybindings: paneProps.keybindings, + actions: paneProps.actions, + } as unknown as ExtensionSidebarViewProps); + expect(received?.files).not.toBe(files); + expect(Object.isFrozen(received?.files)).toBe(false); + received?.files.reverse(); + expect(files.map((file) => file.id)).toEqual(["a", "b"]); + }); + test("rejects a view without a component function and rolls the factory back", () => { const registry = createEmptyExtensionRegistry(); const issues: ExtensionLoadIssue[] = []; @@ -409,8 +462,8 @@ describe("registerSidebarView", () => { test("collides with registerPane through one identity path and keeps the first", () => { const registry = createEmptyExtensionRegistry(); const issues: ExtensionLoadIssue[] = []; - const first = () => null; - const duplicate = () => null; + const first = () => "first"; + const duplicate = () => "duplicate"; runExtensionFactory({ metadata: bundledMetadata("mixed"), @@ -425,7 +478,9 @@ describe("registerSidebarView", () => { const resolved = resolveExtensionPanes(registry); expect(issues).toEqual([]); expect(resolved.panes).toHaveLength(1); - expect(resolved.panes[0]?.pane.component).toBe(first); + expect(resolved.panes[0]?.pane.component({ files: [] } as unknown as ExtensionPaneProps)).toBe( + "first", + ); expect(resolved.issues).toEqual([ { extensionId: "mixed", diff --git a/packages/hunk/src/extensions/runExtension.ts b/packages/hunk/src/extensions/runExtension.ts index 91b2cccb7..e92a46a70 100644 --- a/packages/hunk/src/extensions/runExtension.ts +++ b/packages/hunk/src/extensions/runExtension.ts @@ -37,7 +37,12 @@ import type { } from "../extension-api/types"; import type { VcsAdapter, VcsHistorySource, VcsOperation, VcsReviewInput } from "../core/vcs/types"; import { sanitizeTerminalLine, sanitizeTerminalText } from "../lib/terminalText"; -import { defaultExtensionPaneSize, extensionPaneSize, isVerticalPanePlacement } from "./panes"; +import { + adaptLegacySidebarComponent, + defaultExtensionPaneSize, + extensionPaneSize, + isVerticalPanePlacement, +} from "./panes"; import { isReservedExtensionCliCommandName, isValidExtensionCliCommandName, @@ -927,6 +932,9 @@ export function createExtensionApi( `registerSidebarView placement must be "left" or "right", got "${String(view.placement)}".`, ); } + if (typeof view.component !== "function") { + throw new Error("registerSidebarView requires a view with a component function."); + } api.registerPane({ id: view.id, ...(view.title ? { title: view.title } : {}), @@ -934,7 +942,7 @@ export function createExtensionApi( width: defaultExtensionPaneSize("left"), defaultOpen: view.defaultOpen, replaces: view.replacesDefault ? "hunk:files" : undefined, - component: view.component as unknown as ExtensionPane["component"], + component: adaptLegacySidebarComponent(view.component), }); }, registerFileView(view: ExtensionFileView) { diff --git a/packages/hunk/src/ui/AppHost.extension-sidebar.test.tsx b/packages/hunk/src/ui/AppHost.extension-sidebar.test.tsx index d7e4c52bd..1c49d5f64 100644 --- a/packages/hunk/src/ui/AppHost.extension-sidebar.test.tsx +++ b/packages/hunk/src/ui/AppHost.extension-sidebar.test.tsx @@ -277,6 +277,54 @@ describe("extension sidebar views", () => { }); }); + test("a legacy sidebar view may mutate its files array without touching host state", async () => { + const repo = createTestRepo("hunk-ext-sidebar-legacy-files-"); + const extPath = join(createTempDir("hunk-ext-sidebar-legacy-files-ext-"), "ext.ts"); + // The deprecated contract declares `files` as a mutable array, so a legacy + // view may sort or push in place. A modern pane mounted beside it must + // still see the untouched, frozen host projection. + writeFileSync( + extPath, + `import { createElement } from "react";\n` + + `export default function (hunk) {\n` + + ` hunk.registerSidebarView({\n` + + ` id: "legacy",\n` + + ` defaultOpen: true,\n` + + ` component: (props) => {\n` + + ` props.files.sort((a, b) => b.path.localeCompare(a.path));\n` + + ` props.files.push(props.files[0]);\n` + + ` return createElement("text", {\n` + + ` content: "LEGACY " + props.files.length + " " + props.files[0].path,\n` + + ` });\n` + + ` },\n` + + ` });\n` + + ` hunk.registerPane({\n` + + ` id: "modern",\n` + + ` placement: "right",\n` + + ` defaultOpen: true,\n` + + ` component: (props) => createElement("text", {\n` + + ` content: "MODERN " + props.files.length + " " + props.files[0].path +\n` + + ` " frozen=" + Object.isFrozen(props.files),\n` + + ` }),\n` + + ` });\n` + + `}\n`, + ); + + const bootstrap = await launchWithExtension(repo, extPath); + await withAppHost(bootstrap, async (setup) => { + await flushUntil( + setup, + () => { + const frame = setup.captureCharFrame(); + return ( + frame.includes("LEGACY 3 beta.txt") && frame.includes("MODERN 2 alpha.txt frozen=true") + ); + }, + "the legacy view to mutate a private copy while the modern pane keeps the host list", + ); + }); + }); + test("a command handler sees the current review selection", async () => { const repo = createTestRepo("hunk-ext-selection-"); // Outside the repo, so the fixture and its log never join the review as