fix(git): dedupe status fetches by mode (#1166)
* fix(git): dedupe status fetches by mode * test(git): assert reused status result --------- Co-authored-by: Isaac Sanchez <isanchez-hawkins@arize.com>
This commit is contained in:
committed by
GitHub
co-authored by
Isaac Sanchez
parent
e4227c7f06
commit
afe86a84b3
@@ -0,0 +1,94 @@
|
|||||||
|
import { beforeEach, describe, expect, test } from 'bun:test';
|
||||||
|
import type { GitStatus } from '@/lib/api/types';
|
||||||
|
import { useGitStore } from './useGitStore';
|
||||||
|
|
||||||
|
type Deferred<T> = {
|
||||||
|
promise: Promise<T>;
|
||||||
|
resolve: (value: T) => void;
|
||||||
|
reject: (error: unknown) => void;
|
||||||
|
};
|
||||||
|
|
||||||
|
type GitAPI = Parameters<ReturnType<typeof useGitStore.getState>['fetchStatus']>[1];
|
||||||
|
|
||||||
|
const createDeferred = <T>(): Deferred<T> => {
|
||||||
|
let resolve!: (value: T) => void;
|
||||||
|
let reject!: (error: unknown) => void;
|
||||||
|
const promise = new Promise<T>((res, rej) => {
|
||||||
|
resolve = res;
|
||||||
|
reject = rej;
|
||||||
|
});
|
||||||
|
return { promise, resolve, reject };
|
||||||
|
};
|
||||||
|
|
||||||
|
const createStatus = (diffStats?: GitStatus['diffStats']): GitStatus => ({
|
||||||
|
current: 'main',
|
||||||
|
tracking: null,
|
||||||
|
ahead: 0,
|
||||||
|
behind: 0,
|
||||||
|
files: [],
|
||||||
|
isClean: true,
|
||||||
|
diffStats,
|
||||||
|
});
|
||||||
|
|
||||||
|
const createGitApi = (getGitStatus: GitAPI['getGitStatus']): GitAPI => ({
|
||||||
|
checkIsGitRepository: async () => true,
|
||||||
|
getGitStatus,
|
||||||
|
getGitBranches: async () => ({ all: [], current: 'main', branches: {} }),
|
||||||
|
getGitLog: async () => ({ all: [], latest: null, total: 0 }),
|
||||||
|
getCurrentGitIdentity: async () => null,
|
||||||
|
getGitFileDiff: async (_directory, options) => ({ original: '', modified: '', path: options.path }),
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('useGitStore', () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
useGitStore.setState({
|
||||||
|
directories: new Map(),
|
||||||
|
activeDirectory: null,
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
test('does not reuse an in-flight light status request for full status', async () => {
|
||||||
|
const requests: Deferred<GitStatus>[] = [];
|
||||||
|
const statusCalls: Array<{ directory: string; options?: { mode?: 'light' } }> = [];
|
||||||
|
const git = createGitApi((directory, options) => {
|
||||||
|
statusCalls.push({ directory, options });
|
||||||
|
const request = createDeferred<GitStatus>();
|
||||||
|
requests.push(request);
|
||||||
|
return request.promise;
|
||||||
|
});
|
||||||
|
|
||||||
|
const lightPromise = useGitStore.getState().fetchStatus('/repo', git, { mode: 'light', silent: true });
|
||||||
|
const fullPromise = useGitStore.getState().fetchStatus('/repo', git, { silent: true });
|
||||||
|
await Promise.resolve();
|
||||||
|
|
||||||
|
expect(statusCalls).toEqual([
|
||||||
|
{ directory: '/repo', options: { mode: 'light' } },
|
||||||
|
{ directory: '/repo', options: undefined },
|
||||||
|
]);
|
||||||
|
|
||||||
|
requests[0].resolve(createStatus());
|
||||||
|
requests[1].resolve(createStatus({ 'src/index.ts': { insertions: 1, deletions: 0 } }));
|
||||||
|
await Promise.all([lightPromise, fullPromise]);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('reuses an in-flight full status request for light status', async () => {
|
||||||
|
const requests: Deferred<GitStatus>[] = [];
|
||||||
|
const statusCalls: Array<{ directory: string; options?: { mode?: 'light' } }> = [];
|
||||||
|
const git = createGitApi((directory, options) => {
|
||||||
|
statusCalls.push({ directory, options });
|
||||||
|
const request = createDeferred<GitStatus>();
|
||||||
|
requests.push(request);
|
||||||
|
return request.promise;
|
||||||
|
});
|
||||||
|
|
||||||
|
const fullPromise = useGitStore.getState().fetchStatus('/repo', git, { silent: true });
|
||||||
|
const lightPromise = useGitStore.getState().fetchStatus('/repo', git, { mode: 'light', silent: true });
|
||||||
|
await Promise.resolve();
|
||||||
|
|
||||||
|
expect(statusCalls).toEqual([{ directory: '/repo', options: undefined }]);
|
||||||
|
|
||||||
|
requests[0].resolve(createStatus({ 'src/index.ts': { insertions: 1, deletions: 0 } }));
|
||||||
|
const [fullResult, lightResult] = await Promise.all([fullPromise, lightPromise]);
|
||||||
|
expect(lightResult).toBe(fullResult);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -22,6 +22,7 @@ const DIFF_PREFETCH_LARGE_FILE_THRESHOLD = 500; // skip prefetch for files with
|
|||||||
// Diff cache limits to prevent memory bloat with many modified files
|
// Diff cache limits to prevent memory bloat with many modified files
|
||||||
const DIFF_CACHE_MAX_ENTRIES = 30;
|
const DIFF_CACHE_MAX_ENTRIES = 30;
|
||||||
const DIFF_CACHE_MAX_TOTAL_SIZE_BYTES = 20 * 1024 * 1024; // 20MB
|
const DIFF_CACHE_MAX_TOTAL_SIZE_BYTES = 20 * 1024 * 1024; // 20MB
|
||||||
|
type GitStatusFetchMode = 'full' | 'light';
|
||||||
|
|
||||||
interface DirectoryGitState {
|
interface DirectoryGitState {
|
||||||
isGitRepo: boolean | null;
|
isGitRepo: boolean | null;
|
||||||
@@ -90,9 +91,11 @@ interface GitAPI {
|
|||||||
|
|
||||||
const inFlightDiffFetchesByDirectory = new Map<string, Set<string>>();
|
const inFlightDiffFetchesByDirectory = new Map<string, Set<string>>();
|
||||||
const diffFetchGenerationByDirectory = new Map<string, number>();
|
const diffFetchGenerationByDirectory = new Map<string, number>();
|
||||||
const inFlightStatusFetchesByDirectory = new Map<string, Promise<boolean>>();
|
const inFlightStatusFetches = new Map<string, Promise<boolean>>();
|
||||||
const inFlightEnsureAllByDirectory = new Map<string, Promise<void>>();
|
const inFlightEnsureAllByDirectory = new Map<string, Promise<void>>();
|
||||||
|
|
||||||
|
const getStatusFetchKey = (directory: string, mode: GitStatusFetchMode): string => `${mode}:${directory}`;
|
||||||
|
|
||||||
const getDiffFetchGeneration = (directory: string): number =>
|
const getDiffFetchGeneration = (directory: string): number =>
|
||||||
diffFetchGenerationByDirectory.get(directory) ?? 0;
|
diffFetchGenerationByDirectory.get(directory) ?? 0;
|
||||||
|
|
||||||
@@ -306,7 +309,10 @@ export const useGitStore = create<GitStore>()(
|
|||||||
},
|
},
|
||||||
|
|
||||||
fetchStatus: async (directory, git, options = {}) => {
|
fetchStatus: async (directory, git, options = {}) => {
|
||||||
const existing = inFlightStatusFetchesByDirectory.get(directory);
|
const statusFetchMode: GitStatusFetchMode = options.mode ?? 'full';
|
||||||
|
const statusFetchKey = getStatusFetchKey(directory, statusFetchMode);
|
||||||
|
const existing = inFlightStatusFetches.get(statusFetchKey)
|
||||||
|
?? (statusFetchMode === 'light' ? inFlightStatusFetches.get(getStatusFetchKey(directory, 'full')) : undefined);
|
||||||
if (existing) {
|
if (existing) {
|
||||||
return existing;
|
return existing;
|
||||||
}
|
}
|
||||||
@@ -428,13 +434,13 @@ export const useGitStore = create<GitStore>()(
|
|||||||
return statusChanged;
|
return statusChanged;
|
||||||
})();
|
})();
|
||||||
|
|
||||||
inFlightStatusFetchesByDirectory.set(directory, fetchPromise);
|
inFlightStatusFetches.set(statusFetchKey, fetchPromise);
|
||||||
|
|
||||||
try {
|
try {
|
||||||
return await fetchPromise;
|
return await fetchPromise;
|
||||||
} finally {
|
} finally {
|
||||||
if (inFlightStatusFetchesByDirectory.get(directory) === fetchPromise) {
|
if (inFlightStatusFetches.get(statusFetchKey) === fetchPromise) {
|
||||||
inFlightStatusFetchesByDirectory.delete(directory);
|
inFlightStatusFetches.delete(statusFetchKey);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
|
|||||||
Reference in New Issue
Block a user