Refactoring version picker

- Rename `versionPicker` -> `versionManager`
- Simplify running of picked items
- Cleaning up interfaces
- 💄

extensions/typescript-language-features/src/typescriptService.ts

@@ -8,7 +8,6 @@ import BufferSyncSupport from './features/bufferSyncSupport';
 import * as Proto from './protocol';
 import API from './utils/api';
 import { TypeScriptServiceConfiguration } from './utils/configuration';
-import Logger from './utils/logger';
 import { PluginManager } from './utils/plugins';
 
 export namespace ServerResponse {
@@ -127,7 +126,6 @@ export interface ITypeScriptServiceClient {
 	readonly apiVersion: API;
 	readonly pluginManager: PluginManager;
 	readonly configuration: TypeScriptServiceConfiguration;
-	readonly logger: Logger;
 	readonly bufferSyncSupport: BufferSyncSupport;
 
 	execute<K extends keyof StandardTsServerRequests>(

extensions/typescript-language-features/src/typescriptServiceClient.ts

@@ -25,7 +25,7 @@ import { PluginManager } from './utils/plugins';
 import { TelemetryReporter, VSCodeTelemetryReporter, TelemetryProperties } from './utils/telemetry';
 import Tracer from './utils/tracer';
 import { inferredProjectCompilerOptions, ProjectType } from './utils/tsconfig';
-import { TypeScriptVersionPicker } from './utils/versionPicker';
+import { TypeScriptVersionManager } from './utils/versionManager';
 import { TypeScriptVersion, TypeScriptVersionProvider } from './utils/versionProvider';
 
 const localize = nls.loadMessageBundle();
@@ -99,10 +99,10 @@ export default class TypeScriptServiceClient extends Disposable implements IType
 	private _configuration: TypeScriptServiceConfiguration;
 	private versionProvider: TypeScriptVersionProvider;
 	private pluginPathsProvider: TypeScriptPluginPathsProvider;
-	private versionPicker: TypeScriptVersionPicker;
+	private readonly _versionManager: TypeScriptVersionManager;
 
-	private tracer: Tracer;
-	public readonly logger: Logger = new Logger();
+	private readonly logger = new Logger();
+	private readonly tracer = new Tracer(this.logger);
 
 	private readonly typescriptServerSpawner: TypeScriptServerSpawner;
 	private serverState: ServerState.State = ServerState.None;
@@ -139,9 +139,10 @@ export default class TypeScriptServiceClient extends Disposable implements IType
 		this._configuration = TypeScriptServiceConfiguration.loadFromWorkspace();
 		this.versionProvider = new TypeScriptVersionProvider(this._configuration);
 		this.pluginPathsProvider = new TypeScriptPluginPathsProvider(this._configuration);
-		this.versionPicker = new TypeScriptVersionPicker(this.versionProvider, this.workspaceState);
-
-		this.tracer = new Tracer(this.logger);
+		this._versionManager = this._register(new TypeScriptVersionManager(this.versionProvider, this.workspaceState));
+		this._register(this._versionManager.onDidPickNewVersion(() => {
+			this.restartTsServer();
+		}));
 
 		this.bufferSyncSupport = new BufferSyncSupport(this, allModeIds);
 		this.onReady(() => { this.bufferSyncSupport.listen(); });
@@ -311,20 +312,20 @@ export default class TypeScriptServiceClient extends Disposable implements IType
 			return ServerState.None;
 		}
 
-		let currentVersion = this.versionPicker.currentVersion;
+		let version = this._versionManager.currentVersion;
 
-		this.info(`Using tsserver from: ${currentVersion.path}`);
-		if (!fs.existsSync(currentVersion.tsServerPath)) {
-			vscode.window.showWarningMessage(localize('noServerFound', 'The path {0} doesn\'t point to a valid tsserver install. Falling back to bundled TypeScript version.', currentVersion.path));
+		this.info(`Using tsserver from: ${version.path}`);
+		if (!fs.existsSync(version.tsServerPath)) {
+			vscode.window.showWarningMessage(localize('noServerFound', 'The path {0} doesn\'t point to a valid tsserver install. Falling back to bundled TypeScript version.', version.path));
 
-			this.versionPicker.useBundledVersion();
-			currentVersion = this.versionPicker.currentVersion;
+			this._versionManager.reset();
+			version = this._versionManager.currentVersion;
 		}
 
-		const apiVersion = this.versionPicker.currentVersion.apiVersion || API.defaultVersion;
-		this.onDidChangeTypeScriptVersion(currentVersion);
+		const apiVersion = version.apiVersion || API.defaultVersion;
+		this.onDidChangeTypeScriptVersion(version);
 		let mytoken = ++this.token;
-		const handle = this.typescriptServerSpawner.spawn(currentVersion, this.configuration, this.pluginManager, {
+		const handle = this.typescriptServerSpawner.spawn(version, this.configuration, this.pluginManager, {
 			onFatalError: (command, err) => this.fatalError(command, err),
 		});
 		this.serverState = new ServerState.Running(handle, apiVersion, undefined, true);
@@ -341,7 +342,7 @@ export default class TypeScriptServiceClient extends Disposable implements IType
 		*/
 		this.logTelemetry('tsserver.spawned', {
 			localTypeScriptVersion: this.versionProvider.localVersion ? this.versionProvider.localVersion.displayName : '',
-			typeScriptVersionSource: currentVersion.source,
+			typeScriptVersionSource: version.source,
 		});
 
 		handle.onError((err: Error) => {
@@ -403,7 +404,7 @@ export default class TypeScriptServiceClient extends Disposable implements IType
 		handle.onEvent(event => this.dispatchEvent(event));
 
 		this._onReady!.resolve();
-		this._onTsServerStarted.fire(currentVersion.apiVersion);
+		this._onTsServerStarted.fire(version.apiVersion);
 
 		if (apiVersion.gte(API.v300)) {
 			this.loadingIndicator.startedLoadingProject(undefined /* projectName */);
@@ -415,10 +416,7 @@ export default class TypeScriptServiceClient extends Disposable implements IType
 	}
 
 	public async showVersionPicker(): Promise<void> {
-		const change = await this.versionPicker.show();
-		if (change.newVersion && change.oldVersion && change.oldVersion.eq(change.newVersion)) {
-			this.restartTsServer();
-		}
+		this._versionManager.promptUserForVersion();
 	}
 
 	public async openTsServerLogFile(): Promise<boolean> {

extensions/typescript-language-features/src/utils/versionManager.ts

@@ -6,29 +6,26 @@
 import * as vscode from 'vscode';
 import * as nls from 'vscode-nls';
 import { TypeScriptVersion, TypeScriptVersionProvider } from './versionProvider';
+import { Disposable } from './dispose';
 
 const localize = nls.loadMessageBundle();
 
 const useWorkspaceTsdkStorageKey = 'typescript.useWorkspaceTsdk';
 
-interface MyQuickPickItem extends vscode.QuickPickItem {
-	id: MessageAction;
-	version?: TypeScriptVersion;
+interface QuickPickItem extends vscode.QuickPickItem {
+	run(): void;
 }
 
-enum MessageAction {
-	useLocal,
-	useBundled,
-	learnMore,
-}
+export class TypeScriptVersionManager extends Disposable {
 
-export class TypeScriptVersionPicker {
 	private _currentVersion: TypeScriptVersion;
 
 	public constructor(
 		private readonly versionProvider: TypeScriptVersionProvider,
 		private readonly workspaceState: vscode.Memento
 	) {
+		super();
+
 		this._currentVersion = this.versionProvider.defaultVersion;
 
 		if (this.useWorkspaceTsdkSetting) {
@@ -39,84 +36,81 @@ export class TypeScriptVersionPicker {
 		}
 	}
 
-	private get useWorkspaceTsdkSetting(): boolean {
-		return this.workspaceState.get<boolean>(useWorkspaceTsdkStorageKey, false);
-	}
+	private readonly _onDidPickNewVersion = this._register(new vscode.EventEmitter<void>());
+	public readonly onDidPickNewVersion = this._onDidPickNewVersion.event;
 
 	public get currentVersion(): TypeScriptVersion {
 		return this._currentVersion;
 	}
 
-	public useBundledVersion(): void {
+	public reset(): void {
 		this._currentVersion = this.versionProvider.bundledVersion;
 	}
 
-	public async show(): Promise<{ oldVersion?: TypeScriptVersion, newVersion?: TypeScriptVersion }> {
-		const pickOptions: MyQuickPickItem[] = [];
-
-		const shippedVersion = this.versionProvider.defaultVersion;
-		pickOptions.push({
-			label: (!this.useWorkspaceTsdkSetting
-				? '• '
-				: '') + localize('useVSCodeVersionOption', "Use VS Code's Version"),
-			description: shippedVersion.displayName,
-			detail: shippedVersion.pathLabel,
-			id: MessageAction.useBundled,
-		});
-
-		for (const version of this.versionProvider.localVersions) {
-			pickOptions.push({
-				label: (this.useWorkspaceTsdkSetting && this.currentVersion.eq(version)
-					? '• '
-					: '') + localize('useWorkspaceVersionOption', "Use Workspace Version"),
-				description: version.displayName,
-				detail: version.pathLabel,
-				id: MessageAction.useLocal,
-				version
-			});
-		}
-
-		pickOptions.push({
-			label: localize('learnMore', 'Learn more about managing TypeScript versions'),
-			description: '',
-			id: MessageAction.learnMore
-		});
-
-		const selected = await vscode.window.showQuickPick<MyQuickPickItem>(pickOptions, {
+	public async promptUserForVersion(): Promise<void> {
+		const selected = await vscode.window.showQuickPick<QuickPickItem>([
+			this.getBundledPickItem(),
+			...this.getLocalPickItems(),
+			LearnMorePickItem,
+		], {
 			placeHolder: localize(
 				'selectTsVersion',
 				"Select the TypeScript version used for JavaScript and TypeScript language features"),
 		});
 
-		if (!selected) {
-			return { oldVersion: this.currentVersion };
-		}
+		return selected?.run();
+	}
 
-		switch (selected.id) {
-			case MessageAction.useLocal:
-				await this.workspaceState.update(useWorkspaceTsdkStorageKey, true);
-				if (selected.version) {
-					const tsConfig = vscode.workspace.getConfiguration('typescript');
-					await tsConfig.update('tsdk', selected.version.pathLabel, false);
-
-					const previousVersion = this.currentVersion;
-					this._currentVersion = selected.version;
-					return { oldVersion: previousVersion, newVersion: selected.version };
-				}
-				return { oldVersion: this.currentVersion };
-
-			case MessageAction.useBundled:
+	private getBundledPickItem(): QuickPickItem {
+		const bundledVersion = this.versionProvider.defaultVersion;
+		return {
+			label: (!this.useWorkspaceTsdkSetting
+				? '• '
+				: '') + localize('useVSCodeVersionOption', "Use VS Code's Version"),
+			description: bundledVersion.displayName,
+			detail: bundledVersion.pathLabel,
+			run: async () => {
 				await this.workspaceState.update(useWorkspaceTsdkStorageKey, false);
-				const previousVersion = this.currentVersion;
-				this._currentVersion = shippedVersion;
-				return { oldVersion: previousVersion, newVersion: shippedVersion };
+				this.updateForPickedVersion(bundledVersion);
+			},
+		};
+	}
 
-			case MessageAction.learnMore:
-				vscode.env.openExternal(vscode.Uri.parse('https://go.microsoft.com/fwlink/?linkid=839919'));
-				return { oldVersion: this.currentVersion };
+	private getLocalPickItems(): QuickPickItem[] {
+		return this.versionProvider.localVersions.map(version => {
+			return {
+				label: (this.useWorkspaceTsdkSetting && this.currentVersion.eq(version)
+					? '• '
+					: '') + localize('useWorkspaceVersionOption', "Use Workspace Version"),
+				description: version.displayName,
+				detail: version.pathLabel,
+				run: async () => {
+					await this.workspaceState.update(useWorkspaceTsdkStorageKey, true);
+					const tsConfig = vscode.workspace.getConfiguration('typescript');
+					await tsConfig.update('tsdk', version.pathLabel, false);
+					this.updateForPickedVersion(version);
+				},
+			};
+		});
+	}
 
-			default:
-				return { oldVersion: this.currentVersion };
+	private updateForPickedVersion(pickedVersion: TypeScriptVersion) {
+		const oldVersion = this.currentVersion;
+		this._currentVersion = pickedVersion;
+		if (!oldVersion.eq(pickedVersion)) {
+			this._onDidPickNewVersion.fire();
 		}
 	}
+
+	private get useWorkspaceTsdkSetting(): boolean {
+		return this.workspaceState.get<boolean>(useWorkspaceTsdkStorageKey, false);
+	}
 }
+
+const LearnMorePickItem: QuickPickItem = {
+	label: localize('learnMore', 'Learn more about managing TypeScript versions'),
+	description: '',
+	run: () => {
+		vscode.env.openExternal(vscode.Uri.parse('https://go.microsoft.com/fwlink/?linkid=839919'));
+	}
+};

extensions/typescript-language-features/src/utils/versionProvider.ts

@@ -12,7 +12,7 @@ import { RelativeWorkspacePathResolver } from './relativePathResolver';
 
 const localize = nls.loadMessageBundle();
 
-export const enum TypeScriptVersionSource {
+const enum TypeScriptVersionSource {
 	Bundled = 'bundled',
 	TsNightlyExtension = 'ts-nightly-extension',
 	NodeModules = 'node-modules',
@@ -227,7 +227,7 @@ export class TypeScriptVersionProvider {
 		const versions: TypeScriptVersion[] = [];
 		for (const root of vscode.workspace.workspaceFolders) {
 			let label: string = relativePath;
-			if (vscode.workspace.workspaceFolders && vscode.workspace.workspaceFolders.length > 1) {
+			if (vscode.workspace.workspaceFolders.length > 1) {
 				label = path.join(root.name, relativePath);
 			}