Avoids stashes mutation & cache pollution (#4427)

Improves stash loading performance with async caching

Author: eamodio

CHANGELOG.md

@@ -13,6 +13,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p
 ### Fixed
 
 - Fixes disabled GitLens view tabs on clean install ([#4416](https://github.com/gitkraken/vscode-gitlens/issues/4416))
+- Fixes Stashes view errors w/ ID already exists ([#4427](https://github.com/gitkraken/vscode-gitlens/issues/4427))
 
 ## [17.2.0] - 2025-06-17
 

src/env/node/git/sub-providers/graph.ts

@@ -136,7 +136,7 @@ export class GraphGitSubProvider implements GitGraphSubProvider {
 
 		// TODO@eamodio this is insanity -- there *HAS* to be a better way to get git log to return stashes
 		const gitStash = getSettledValue(stashResult);
-		const { stdin, stashes, remappedIds } = convertStashesToStdin(gitStash?.stashes);
+		const { stdin, remappedIds } = convertStashesToStdin(gitStash?.stashes);
 
 		const useAvatars = configuration.get('graph.avatars', undefined, true);
 
@@ -584,7 +584,7 @@ export class GraphGitSubProvider implements GitGraphSubProvider {
 					branches: branchMap,
 					remotes: remoteMap,
 					downstreams: downstreamMap,
-					stashes: stashes,
+					stashes: gitStash?.stashes,
 					worktrees: worktrees,
 					worktreesByBranch: worktreesByBranch,
 					rows: rows,

src/env/node/git/sub-providers/stash.ts

@@ -73,80 +73,89 @@ export class StashGitSubProvider implements GitStashSubProvider {
 	): Promise<GitStash | undefined> {
 		if (repoPath == null) return undefined;
 
-		let stash = this.cache.stashes?.get(repoPath);
-		if (stash == null) {
-			const parser = getStashLogParser();
-			const args = [...parser.arguments];
+		let stashPromise = this.cache.stashes?.get(repoPath);
+		if (stashPromise == null) {
+			async function load(this: StashGitSubProvider): Promise<GitStash> {
+				const parser = getStashLogParser();
+				const args = [...parser.arguments];
+
+				const similarityThreshold = configuration.get('advanced.similarityThreshold');
+				args.push(`-M${similarityThreshold == null ? '' : `${similarityThreshold}%`}`);
+
+				const result = await this.git.exec(
+					{ cwd: repoPath, cancellation: cancellation },
+					'stash',
+					'list',
+					...args,
+				);
+
+				const stashes = new Map<string, GitStashCommit>();
+
+				for (const s of parser.parse(result.stdout)) {
+					stashes.set(s.sha, createStash(this.container, s, repoPath));
+				}
 
-			const similarityThreshold = configuration.get('advanced.similarityThreshold');
-			args.push(`-M${similarityThreshold == null ? '' : `${similarityThreshold}%`}`);
+				return { repoPath: repoPath, stashes: stashes };
+			}
 
-			const result = await this.git.exec({ cwd: repoPath, cancellation: cancellation }, 'stash', 'list', ...args);
+			stashPromise = load.call(this);
+			this.cache.stashes?.set(repoPath, stashPromise);
+		}
 
-			const stashes = new Map<string, GitStashCommit>();
+		const stash = await stashPromise;
+		if (!options?.reachableFrom || !stash?.stashes.size) return stash;
 
-			for (const s of parser.parse(result.stdout)) {
-				stashes.set(s.sha, createStash(this.container, s, repoPath));
-			}
+		// Return only reachable stashes from the given ref
+		// Create a copy because we are going to modify it and we don't want to mutate the cache
+		const stashes = new Map(stash.stashes);
 
-			stash = { repoPath: repoPath, stashes: stashes };
+		const oldestStashDate = new Date(min(stash.stashes.values(), c => c.date.getTime())).toISOString();
 
-			this.cache.stashes?.set(repoPath, stash);
-		}
+		const result = await this.git.exec(
+			{ cwd: repoPath, cancellation: cancellation, errors: GitErrorHandling.Ignore },
+			'rev-list',
+			`--since="${oldestStashDate}"`,
+			'--date-order',
+			options.reachableFrom,
+			'--',
+		);
 
-		// Return only reachable stashes from the given ref
-		if (options?.reachableFrom && stash?.stashes.size) {
-			// Create a copy because we are going to modify it and we don't want to mutate the cache
-			stash = { ...stash, stashes: new Map(stash.stashes) };
-
-			const oldestStashDate = new Date(min(stash.stashes.values(), c => c.date.getTime())).toISOString();
-
-			const result = await this.git.exec(
-				{ cwd: repoPath, cancellation: cancellation, errors: GitErrorHandling.Ignore },
-				'rev-list',
-				`--since="${oldestStashDate}"`,
-				'--date-order',
-				options.reachableFrom,
-				'--',
-			);
+		const ancestors = result.stdout.trim().split('\n');
+		const reachableCommits =
+			ancestors?.length && (ancestors.length !== 1 || ancestors[0]) ? new Set(ancestors) : undefined;
+		if (reachableCommits?.size) {
+			const reachableStashes = new Set<string>();
 
-			const ancestors = result.stdout.trim().split('\n');
-			const reachableCommits =
-				ancestors?.length && (ancestors.length !== 1 || ancestors[0]) ? new Set(ancestors) : undefined;
-			if (reachableCommits?.size) {
-				const reachableStashes = new Set<string>();
+			// First pass: mark directly reachable stashes
+			for (const [sha, s] of stash.stashes) {
+				if (s.parents.some(p => p === options.reachableFrom || reachableCommits.has(p))) {
+					reachableStashes.add(sha);
+				}
+			}
 
-				// First pass: mark directly reachable stashes
+			// Second pass: mark stashes that build upon reachable stashes
+			let changed;
+			do {
+				changed = false;
 				for (const [sha, s] of stash.stashes) {
-					if (s.parents.some(p => p === options.reachableFrom || reachableCommits.has(p))) {
+					if (!reachableStashes.has(sha) && s.parents.some(p => reachableStashes.has(p))) {
 						reachableStashes.add(sha);
+						changed = true;
 					}
 				}
+			} while (changed);
 
-				// Second pass: mark stashes that build upon reachable stashes
-				let changed;
-				do {
-					changed = false;
-					for (const [sha, s] of stash.stashes) {
-						if (!reachableStashes.has(sha) && s.parents.some(p => reachableStashes.has(p))) {
-							reachableStashes.add(sha);
-							changed = true;
-						}
-					}
-				} while (changed);
-
-				// Remove unreachable stashes
-				for (const [sha] of stash.stashes) {
-					if (!reachableStashes.has(sha)) {
-						stash.stashes.delete(sha);
-					}
+			// Remove unreachable stashes
+			for (const [sha] of stash.stashes) {
+				if (!reachableStashes.has(sha)) {
+					stashes.delete(sha);
 				}
-			} else {
-				stash.stashes.clear();
 			}
+		} else {
+			stashes.clear();
 		}
 
-		return stash;
+		return { ...stash, stashes: stashes };
 	}
 
 	@log()
@@ -342,7 +351,7 @@ export class StashGitSubProvider implements GitStashSubProvider {
 	}
 }
 
-export function convertStashesToStdin(stashOrStashes: GitStash | Map<string, GitStashCommit> | undefined): {
+export function convertStashesToStdin(stashOrStashes: GitStash | ReadonlyMap<string, GitStashCommit> | undefined): {
 	readonly stdin: string | undefined;
 	readonly stashes: Map<string, GitStashCommit>;
 	readonly remappedIds: Map<string, string>;
@@ -351,40 +360,23 @@ export function convertStashesToStdin(stashOrStashes: GitStash | Map<string, Git
 	if (stashOrStashes == null) return { stdin: undefined, stashes: new Map(), remappedIds: remappedIds };
 
 	let stdin: string | undefined;
-	let stashes: Map<string, GitStashCommit>;
-
-	if ('stashes' in stashOrStashes) {
-		stashes = new Map(stashOrStashes.stashes);
-		if (stashOrStashes.stashes.size) {
-			stdin = '';
-			for (const stash of stashOrStashes.stashes.values()) {
-				stdin += `${stash.sha.substring(0, 9)}\n`;
-				// Include the stash's 2nd (index files) and 3rd (untracked files) parents
-				for (const p of skip(stash.parents, 1)) {
-					remappedIds.set(p, stash.sha);
-
+	const original = 'stashes' in stashOrStashes ? stashOrStashes.stashes : stashOrStashes;
+	const stashes = new Map<string, GitStashCommit>(original);
+
+	if (original.size) {
+		stdin = '';
+		for (const stash of original.values()) {
+			stdin += `${stash.sha.substring(0, 9)}\n`;
+			// Include the stash's 2nd (index files) and 3rd (untracked files) parents (if they aren't already in the map)
+			for (const p of skip(stash.parents, 1)) {
+				remappedIds.set(p, stash.sha);
+
+				if (!stashes.has(p)) {
 					stashes.set(p, stash);
 					stdin += `${p.substring(0, 9)}\n`;
 				}
 			}
 		}
-	} else {
-		if (stashOrStashes.size) {
-			stdin = '';
-			for (const stash of stashOrStashes.values()) {
-				stdin += `${stash.sha.substring(0, 9)}\n`;
-				// Include the stash's 2nd (index files) and 3rd (untracked files) parents (if they aren't already in the map)
-				for (const p of skip(stash.parents, 1)) {
-					remappedIds.set(p, stash.sha);
-
-					if (!stashOrStashes.has(p)) {
-						stashOrStashes.set(p, stash);
-						stdin += `${p.substring(0, 9)}\n`;
-					}
-				}
-			}
-		}
-		stashes = stashOrStashes;
 	}
 
 	return { stdin: stdin || undefined, stashes: stashes, remappedIds: remappedIds };

src/git/cache.ts

@@ -116,9 +116,9 @@ export class GitCache implements Disposable {
 		return (this._repoInfoCache ??= new Map<RepoPath, RepositoryInfo>());
 	}
 
-	private _stashesCache: Map<RepoPath, GitStash> | undefined;
-	get stashes(): Map<RepoPath, GitStash> | undefined {
-		return this.useCaching ? (this._stashesCache ??= new Map<RepoPath, GitStash>()) : undefined;
+	private _stashesCache: Map<RepoPath, Promise<GitStash>> | undefined;
+	get stashes(): Map<RepoPath, Promise<GitStash>> | undefined {
+		return this.useCaching ? (this._stashesCache ??= new Map<RepoPath, Promise<GitStash>>()) : undefined;
 	}
 
 	private _tagsCache: Map<RepoPath, Promise<PagedResult<GitTag>>> | undefined;

src/git/models/graph.ts

@@ -43,12 +43,12 @@ export interface GitGraph {
 	/** A set of all "seen" commit ids */
 	readonly ids: Set<string>;
 	readonly includes: { stats?: boolean } | undefined;
-	readonly branches: Map<string, GitBranch>;
-	readonly remotes: Map<string, GitRemote>;
-	readonly downstreams: Map<string, string[]>;
-	readonly stashes: Map<string, GitStashCommit> | undefined;
-	readonly worktrees: GitWorktree[] | undefined;
-	readonly worktreesByBranch: Map<string, GitWorktree> | undefined;
+	readonly branches: ReadonlyMap<string, GitBranch>;
+	readonly remotes: ReadonlyMap<string, GitRemote>;
+	readonly downstreams: ReadonlyMap<string, string[]>;
+	readonly stashes: ReadonlyMap<string, GitStashCommit> | undefined;
+	readonly worktrees: ReadonlyArray<GitWorktree> | undefined;
+	readonly worktreesByBranch: ReadonlyMap<string, GitWorktree> | undefined;
 
 	/** The rows for the set of commits requested */
 	readonly rows: GitGraphRow[];

src/git/models/stash.ts

@@ -2,5 +2,5 @@ import type { GitStashCommit } from './commit';
 
 export interface GitStash {
 	readonly repoPath: string;
-	readonly stashes: Map<string, GitStashCommit>;
+	readonly stashes: ReadonlyMap<string, GitStashCommit>;
 }

src/git/utils/branch.utils.ts

@@ -41,7 +41,7 @@ export function getBranchTrackingWithoutRemote(ref: GitBranchReference): string
 
 export async function getLocalBranchByUpstream(
 	remoteBranchName: string,
-	branches: PageableResult<GitBranch> | Map<unknown, GitBranch>,
+	branches: PageableResult<GitBranch> | ReadonlyMap<unknown, GitBranch>,
 ): Promise<GitBranch | undefined> {
 	let qualifiedRemoteBranchName;
 	if (remoteBranchName.startsWith('remotes/')) {

src/views/stashesView.ts

@@ -22,10 +22,7 @@ import { registerViewCommand } from './viewCommands';
 
 export class StashesRepositoryNode extends RepositoryFolderNode<StashesView, StashesNode> {
 	async getChildren(): Promise<ViewNode[]> {
-		if (this.child == null) {
-			this.child = new StashesNode(this.uri, this.view, this, this.repo);
-		}
-
+		this.child ??= new StashesNode(this.uri, this.view, this, this.repo);
 		return this.child.getChildren();
 	}