Refactor IdeContextStore (#8278)

This commit is contained in:
Tommaso Sciortino
2025-09-11 11:22:20 -07:00
committed by GitHub
parent 538e6cd19a
commit d892cde0b0
10 changed files with 92 additions and 114 deletions

View File

@@ -8,7 +8,7 @@ import * as fs from 'node:fs';
import { isSubpath } from '../utils/paths.js';
import { detectIde, type DetectedIde, getIdeInfo } from '../ide/detect-ide.js';
import {
ideContext,
ideContextStore,
IdeDiffAcceptedNotificationSchema,
IdeDiffClosedNotificationSchema,
CloseDiffResponseSchema,
@@ -362,7 +362,7 @@ export class IdeClient {
}
if (status === IDEConnectionStatus.Disconnected) {
ideContext.clearIdeContext();
ideContextStore.clear();
}
}
@@ -562,7 +562,7 @@ export class IdeClient {
this.client.setNotificationHandler(
IdeContextNotificationSchema,
(notification) => {
ideContext.setIdeContext(notification.params);
ideContextStore.set(notification.params);
const isTrusted = notification.params.workspaceState?.isTrusted;
if (isTrusted !== undefined) {
for (const listener of this.trustChangeListeners) {

View File

@@ -9,7 +9,7 @@ import {
IDE_MAX_SELECTED_TEXT_LENGTH,
} from './constants.js';
import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest';
import { createIdeContextStore } from './ideContext.js';
import { IdeContextStore } from './ideContext.js';
import {
type IdeContext,
FileSchema,
@@ -19,11 +19,11 @@ import {
describe('ideContext', () => {
describe('createIdeContextStore', () => {
let ideContextStore: ReturnType<typeof createIdeContextStore>;
let ideContextStore: IdeContextStore;
beforeEach(() => {
// Create a fresh, isolated instance for each test
ideContextStore = createIdeContextStore();
ideContextStore = new IdeContextStore();
});
afterEach(() => {
@@ -31,7 +31,7 @@ describe('ideContext', () => {
});
it('should return undefined initially for ide context', () => {
expect(ideContextStore.getIdeContext()).toBeUndefined();
expect(ideContextStore.get()).toBeUndefined();
});
it('should set and retrieve the ide context', () => {
@@ -48,9 +48,9 @@ describe('ideContext', () => {
},
};
ideContextStore.setIdeContext(testFile);
ideContextStore.set(testFile);
const activeFile = ideContextStore.getIdeContext();
const activeFile = ideContextStore.get();
expect(activeFile).toEqual(testFile);
});
@@ -67,7 +67,7 @@ describe('ideContext', () => {
],
},
};
ideContextStore.setIdeContext(firstFile);
ideContextStore.set(firstFile);
const secondFile = {
workspaceState: {
@@ -81,9 +81,9 @@ describe('ideContext', () => {
],
},
};
ideContextStore.setIdeContext(secondFile);
ideContextStore.set(secondFile);
const activeFile = ideContextStore.getIdeContext();
const activeFile = ideContextStore.get();
expect(activeFile).toEqual(secondFile);
});
@@ -100,16 +100,16 @@ describe('ideContext', () => {
],
},
};
ideContextStore.setIdeContext(testFile);
expect(ideContextStore.getIdeContext()).toEqual(testFile);
ideContextStore.set(testFile);
expect(ideContextStore.get()).toEqual(testFile);
});
it('should notify subscribers when ide context changes', () => {
const subscriber1 = vi.fn();
const subscriber2 = vi.fn();
ideContextStore.subscribeToIdeContext(subscriber1);
ideContextStore.subscribeToIdeContext(subscriber2);
ideContextStore.subscribe(subscriber1);
ideContextStore.subscribe(subscriber2);
const testFile = {
workspaceState: {
@@ -123,7 +123,7 @@ describe('ideContext', () => {
],
},
};
ideContextStore.setIdeContext(testFile);
ideContextStore.set(testFile);
expect(subscriber1).toHaveBeenCalledTimes(1);
expect(subscriber1).toHaveBeenCalledWith(testFile);
@@ -143,7 +143,7 @@ describe('ideContext', () => {
],
},
};
ideContextStore.setIdeContext(newFile);
ideContextStore.set(newFile);
expect(subscriber1).toHaveBeenCalledTimes(2);
expect(subscriber1).toHaveBeenCalledWith(newFile);
@@ -155,10 +155,10 @@ describe('ideContext', () => {
const subscriber1 = vi.fn();
const subscriber2 = vi.fn();
const unsubscribe1 = ideContextStore.subscribeToIdeContext(subscriber1);
ideContextStore.subscribeToIdeContext(subscriber2);
const unsubscribe1 = ideContextStore.subscribe(subscriber1);
ideContextStore.subscribe(subscriber2);
ideContextStore.setIdeContext({
ideContextStore.set({
workspaceState: {
openFiles: [
{
@@ -175,7 +175,7 @@ describe('ideContext', () => {
unsubscribe1();
ideContextStore.setIdeContext({
ideContextStore.set({
workspaceState: {
openFiles: [
{
@@ -205,21 +205,21 @@ describe('ideContext', () => {
},
};
ideContextStore.setIdeContext(testFile);
ideContextStore.set(testFile);
expect(ideContextStore.getIdeContext()).toEqual(testFile);
expect(ideContextStore.get()).toEqual(testFile);
ideContextStore.clearIdeContext();
ideContextStore.clear();
expect(ideContextStore.getIdeContext()).toBeUndefined();
expect(ideContextStore.get()).toBeUndefined();
});
it('should set the context and notify subscribers when no workspaceState is present', () => {
const subscriber = vi.fn();
ideContextStore.subscribeToIdeContext(subscriber);
ideContextStore.subscribe(subscriber);
const context: IdeContext = {};
ideContextStore.setIdeContext(context);
expect(ideContextStore.getIdeContext()).toBe(context);
ideContextStore.set(context);
expect(ideContextStore.get()).toBe(context);
expect(subscriber).toHaveBeenCalledWith(context);
});
@@ -229,10 +229,8 @@ describe('ideContext', () => {
openFiles: [],
},
};
ideContextStore.setIdeContext(context);
expect(
ideContextStore.getIdeContext()?.workspaceState?.openFiles,
).toEqual([]);
ideContextStore.set(context);
expect(ideContextStore.get()?.workspaceState?.openFiles).toEqual([]);
});
it('should sort openFiles by timestamp in descending order', () => {
@@ -245,9 +243,8 @@ describe('ideContext', () => {
],
},
};
ideContextStore.setIdeContext(context);
const openFiles =
ideContextStore.getIdeContext()?.workspaceState?.openFiles;
ideContextStore.set(context);
const openFiles = ideContextStore.get()?.workspaceState?.openFiles;
expect(openFiles?.[0]?.path).toBe('file2.ts');
expect(openFiles?.[1]?.path).toBe('file3.ts');
expect(openFiles?.[2]?.path).toBe('file1.ts');
@@ -279,9 +276,8 @@ describe('ideContext', () => {
],
},
};
ideContextStore.setIdeContext(context);
const openFiles =
ideContextStore.getIdeContext()?.workspaceState?.openFiles;
ideContextStore.set(context);
const openFiles = ideContextStore.get()?.workspaceState?.openFiles;
expect(openFiles?.[0]?.isActive).toBe(true);
expect(openFiles?.[0]?.cursor).toBeDefined();
expect(openFiles?.[0]?.selectedText).toBeDefined();
@@ -309,10 +305,9 @@ describe('ideContext', () => {
],
},
};
ideContextStore.setIdeContext(context);
ideContextStore.set(context);
const selectedText =
ideContextStore.getIdeContext()?.workspaceState?.openFiles?.[0]
?.selectedText;
ideContextStore.get()?.workspaceState?.openFiles?.[0]?.selectedText;
expect(selectedText).toHaveLength(
IDE_MAX_SELECTED_TEXT_LENGTH + '... [TRUNCATED]'.length,
);
@@ -333,10 +328,9 @@ describe('ideContext', () => {
],
},
};
ideContextStore.setIdeContext(context);
ideContextStore.set(context);
const selectedText =
ideContextStore.getIdeContext()?.workspaceState?.openFiles?.[0]
?.selectedText;
ideContextStore.get()?.workspaceState?.openFiles?.[0]?.selectedText;
expect(selectedText).toBe(shortText);
});
@@ -354,9 +348,8 @@ describe('ideContext', () => {
openFiles: files,
},
};
ideContextStore.setIdeContext(context);
const openFiles =
ideContextStore.getIdeContext()?.workspaceState?.openFiles;
ideContextStore.set(context);
const openFiles = ideContextStore.get()?.workspaceState?.openFiles;
expect(openFiles).toHaveLength(IDE_MAX_OPEN_FILES);
});
});

View File

@@ -69,25 +69,18 @@ export type DiffUpdateResult =
content: undefined;
};
type IdeContextSubscriber = (ideContext: IdeContext | undefined) => void;
type IdeContextSubscriber = (ideContext?: IdeContext) => void;
/**
* Creates a new store for managing the IDE's context.
* This factory function encapsulates the state and logic, allowing for the creation
* of isolated instances, which is particularly useful for testing.
*
* @returns An object with methods to interact with the IDE context.
*/
export function createIdeContextStore() {
let ideContextState: IdeContext | undefined = undefined;
const subscribers = new Set<IdeContextSubscriber>();
export class IdeContextStore {
private ideContextState?: IdeContext;
private readonly subscribers = new Set<IdeContextSubscriber>();
/**
* Notifies all registered subscribers about the current IDE context.
*/
function notifySubscribers(): void {
for (const subscriber of subscribers) {
subscriber(ideContextState);
private notifySubscribers(): void {
for (const subscriber of this.subscribers) {
subscriber(this.ideContextState);
}
}
@@ -95,11 +88,11 @@ export function createIdeContextStore() {
* Sets the IDE context and notifies all registered subscribers of the change.
* @param newIdeContext The new IDE context from the IDE.
*/
function setIdeContext(newIdeContext: IdeContext): void {
set(newIdeContext: IdeContext): void {
const { workspaceState } = newIdeContext;
if (!workspaceState) {
ideContextState = newIdeContext;
notifySubscribers();
this.ideContextState = newIdeContext;
this.notifySubscribers();
return;
}
@@ -147,24 +140,24 @@ export function createIdeContextStore() {
workspaceState.openFiles = openFiles.slice(0, IDE_MAX_OPEN_FILES);
}
}
ideContextState = newIdeContext;
notifySubscribers();
this.ideContextState = newIdeContext;
this.notifySubscribers();
}
/**
* Clears the IDE context and notifies all registered subscribers of the change.
*/
function clearIdeContext(): void {
ideContextState = undefined;
notifySubscribers();
clear(): void {
this.ideContextState = undefined;
this.notifySubscribers();
}
/**
* Retrieves the current IDE context.
* @returns The `IdeContext` object if a file is active; otherwise, `undefined`.
*/
function getIdeContext(): IdeContext | undefined {
return ideContextState;
get(): IdeContext | undefined {
return this.ideContextState;
}
/**
@@ -176,22 +169,15 @@ export function createIdeContextStore() {
* @param subscriber The function to be called when the IDE context changes.
* @returns A function that, when called, will unsubscribe the provided subscriber.
*/
function subscribeToIdeContext(subscriber: IdeContextSubscriber): () => void {
subscribers.add(subscriber);
subscribe(subscriber: IdeContextSubscriber): () => void {
this.subscribers.add(subscriber);
return () => {
subscribers.delete(subscriber);
this.subscribers.delete(subscriber);
};
}
return {
setIdeContext,
getIdeContext,
subscribeToIdeContext,
clearIdeContext,
};
}
/**
* The default, shared instance of the IDE context store for the application.
*/
export const ideContext = createIdeContextStore();
export const ideContextStore = new IdeContextStore();