From 6db065e984f9010e03c0cadd4c9b378312e2edd8 Mon Sep 17 00:00:00 2001 From: Federico Jaramillo Martinez Date: Fri, 24 Jul 2026 12:09:55 +0200 Subject: [PATCH] refactor(ui): dedupe git file helpers and memoize git view model - N1: extract the copy-pasted pointerName/segmentName helpers from gitFileList.ts and gitFileTree.ts into gitFileShared.ts. - N3: drop the dead conditional "tree" class (no CSS rule exists). - N4 (P3): memoize computeViewState on (status, view) identity so renders from expand/collapse or diff selection skip the full model rebuild; expand state is read live at render time, never cached. --- .../src/components/WorkspaceGitPanel.ts | 21 ++++++++++- src/client/src/gitFileList.ts | 13 +------ src/client/src/gitFileShared.test.ts | 35 +++++++++++++++++++ src/client/src/gitFileShared.ts | 18 ++++++++++ src/client/src/gitFileTree.ts | 13 +------ 5 files changed, 75 insertions(+), 25 deletions(-) create mode 100644 src/client/src/gitFileShared.test.ts create mode 100644 src/client/src/gitFileShared.ts diff --git a/src/client/src/components/WorkspaceGitPanel.ts b/src/client/src/components/WorkspaceGitPanel.ts index 73b5c68..006cc94 100644 --- a/src/client/src/components/WorkspaceGitPanel.ts +++ b/src/client/src/components/WorkspaceGitPanel.ts @@ -13,6 +13,12 @@ interface GitViewState { readonly expandablePaths: readonly string[]; } +interface GitViewStateCache { + readonly status: GitStatusResponse | undefined; + readonly view: GitFileView; + readonly viewState: GitViewState; +} + const EMPTY_LIST_MODEL: GitFileListModel = { submodules: [], files: [] }; const EMPTY_VIEW_STATE: GitViewState = { nodes: [], listModel: EMPTY_LIST_MODEL, expandablePaths: [] }; @@ -45,6 +51,11 @@ export class WorkspaceGitPanel extends LitElement { // submodule groups, both keyed by their path. @state() private expandedDirectories = new Set(); + // Memoized on (status, view) identity: renders triggered by expand/collapse + // or diff selection reuse the model, while a new poll or view switch rebuilds + // it. Expand state is read live at render time, never baked into the model. + private viewStateCache: GitViewStateCache | undefined; + protected override willUpdate(changedProperties: PropertyValues): void { if (!changedProperties.has("context")) return; const previous = changedProperties.get("context"); @@ -70,7 +81,7 @@ export class WorkspaceGitPanel extends LitElement {
-
+
${this.renderFileList(context, status, viewState)}
${renderDiffViewer(context)}
@@ -166,6 +177,14 @@ export class WorkspaceGitPanel extends LitElement { } private computeViewState(status: GitStatusResponse | undefined): GitViewState { + const cached = this.viewStateCache; + if (cached !== undefined && cached.status === status && cached.view === this.view) return cached.viewState; + const viewState = this.buildViewState(status); + this.viewStateCache = { status, view: this.view, viewState }; + return viewState; + } + + private buildViewState(status: GitStatusResponse | undefined): GitViewState { if (status === undefined || !status.isGitRepo || status.files.length === 0) return EMPTY_VIEW_STATE; if (this.view === "tree") { const nodes = buildGitFileTree(status.files, status.submodules); diff --git a/src/client/src/gitFileList.ts b/src/client/src/gitFileList.ts index 32faa6e..133ff72 100644 --- a/src/client/src/gitFileList.ts +++ b/src/client/src/gitFileList.ts @@ -1,4 +1,5 @@ import type { GitStatusFile } from "./api"; +import { pointerName, segmentName } from "./gitFileShared"; /** A changed file inside a submodule, shown flat in list view. `path` is the * full superproject-relative path (the diff key); `relativePath` is shown in @@ -70,15 +71,3 @@ function ownerSubmodule(path: string, submodules: readonly string[]): string | u } return best; } - -function pointerName(file: GitStatusFile): string { - const from = file.submoduleFromCommit; - const to = file.submoduleToCommit; - return from !== undefined && to !== undefined ? `${from} → ${to}` : "commit"; -} - -function segmentName(path: string): string { - const segments = path.split("/"); - const last = segments[segments.length - 1]; - return last ?? path; -} diff --git a/src/client/src/gitFileShared.test.ts b/src/client/src/gitFileShared.test.ts new file mode 100644 index 0000000..9ffda1a --- /dev/null +++ b/src/client/src/gitFileShared.test.ts @@ -0,0 +1,35 @@ +import { describe, expect, it } from "vitest"; +import type { GitStatusFile } from "./api"; +import { pointerName, segmentName } from "./gitFileShared"; + +describe("pointerName", () => { + it("renders the moved pointer as → ", () => { + expect(pointerName(pointer("1111111", "2222222"))).toBe("1111111 → 2222222"); + }); + + it("falls back to a plain label when either end is unresolved", () => { + expect(pointerName(pointer(undefined, "2222222"))).toBe("commit"); + expect(pointerName(pointer("1111111", undefined))).toBe("commit"); + expect(pointerName(pointer(undefined, undefined))).toBe("commit"); + }); +}); + +describe("segmentName", () => { + it("returns the final path segment", () => { + expect(segmentName("deps/sub/module")).toBe("module"); + }); + + it("returns the whole path when there is no separator", () => { + expect(segmentName("HARL")).toBe("HARL"); + }); +}); + +function pointer(from: string | undefined, to: string | undefined): GitStatusFile { + return { + path: "HARL", + index: "unmodified", + workingTree: "modified", + ...(from === undefined ? {} : { submoduleFromCommit: from }), + ...(to === undefined ? {} : { submoduleToCommit: to }), + }; +} diff --git a/src/client/src/gitFileShared.ts b/src/client/src/gitFileShared.ts new file mode 100644 index 0000000..d1e5d1d --- /dev/null +++ b/src/client/src/gitFileShared.ts @@ -0,0 +1,18 @@ +import type { GitStatusFile } from "./api"; + +/** + * Display label for a submodule commit-pointer row: the ` → ` + * short-SHA summary, or "commit" when the server did not resolve both ends. + */ +export function pointerName(file: GitStatusFile): string { + const from = file.submoduleFromCommit; + const to = file.submoduleToCommit; + return from !== undefined && to !== undefined ? `${from} → ${to}` : "commit"; +} + +/** Final segment of a path, used as the display name for tree/list rows. */ +export function segmentName(path: string): string { + const segments = path.split("/"); + const last = segments[segments.length - 1]; + return last ?? path; +} diff --git a/src/client/src/gitFileTree.ts b/src/client/src/gitFileTree.ts index 972f15e..4c2813e 100644 --- a/src/client/src/gitFileTree.ts +++ b/src/client/src/gitFileTree.ts @@ -1,4 +1,5 @@ import type { GitStatusFile } from "./api"; +import { pointerName, segmentName } from "./gitFileShared"; /** * A changed file placed at a leaf of the Git file tree. `path` is the full @@ -93,12 +94,6 @@ export function collectGitFileTreeDirectoryPaths(nodes: readonly GitFileTreeNode return paths; } -function pointerName(file: GitStatusFile): string { - const from = file.submoduleFromCommit; - const to = file.submoduleToCommit; - return from !== undefined && to !== undefined ? `${from} → ${to}` : "commit"; -} - function createDirectoryAccumulator(path: string): DirectoryAccumulator { return { path, directories: new Map(), files: [], isSubmodule: false }; } @@ -125,9 +120,3 @@ function finalizeChildren(directory: DirectoryAccumulator): GitFileTreeNode[] { const files = [...directory.files].sort((left, right) => left.name.localeCompare(right.name)); return [...(directory.pointer === undefined ? [] : [directory.pointer]), ...directories, ...files]; } - -function segmentName(path: string): string { - const segments = path.split("/"); - const last = segments[segments.length - 1]; - return last ?? path; -}