From 2aced89ae16668c42c45a25fabc541a79f5e3a03 Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Tue, 18 Feb 2020 14:19:38 -0800 Subject: [PATCH] Show more clear TS Version picker when reinstalling a different TS version locally - Make sure that `TypeScriptVersion` is immutable by getting and caching `apiVersion` on init - Only show dot next to currently active version if both path and api versions match --- .../src/typescriptServiceClient.ts | 11 +++--- .../src/utils/api.ts | 4 ++ .../src/utils/versionPicker.ts | 2 +- .../src/utils/versionProvider.ts | 39 ++++++++++++++----- 4 files changed, 39 insertions(+), 17 deletions(-) diff --git a/extensions/typescript-language-features/src/typescriptServiceClient.ts b/extensions/typescript-language-features/src/typescriptServiceClient.ts index 1fb1dce18b0..b3f4877ee37 100644 --- a/extensions/typescript-language-features/src/typescriptServiceClient.ts +++ b/extensions/typescript-language-features/src/typescriptServiceClient.ts @@ -414,15 +414,14 @@ export default class TypeScriptServiceClient extends Disposable implements IType } public onVersionStatusClicked(): Thenable { - return this.showVersionPicker(false); + return this.showVersionPicker(); } - private showVersionPicker(firstRun: boolean): Thenable { - return this.versionPicker.show(firstRun).then(change => { - if (firstRun || !change.newVersion || !change.oldVersion || change.oldVersion.path === change.newVersion.path) { - return; + private showVersionPicker(): Thenable { + return this.versionPicker.show().then(change => { + if (change.newVersion && change.oldVersion && change.oldVersion.eq(change.newVersion)) { + this.restartTsServer(); } - this.restartTsServer(); }); } diff --git a/extensions/typescript-language-features/src/utils/api.ts b/extensions/typescript-language-features/src/utils/api.ts index 4a1c217471d..25be4b5e516 100644 --- a/extensions/typescript-language-features/src/utils/api.ts +++ b/extensions/typescript-language-features/src/utils/api.ts @@ -65,6 +65,10 @@ export default class API { public readonly fullVersionString: string, ) { } + public eq(other: API): boolean { + return semver.eq(this.version, other.version); + } + public gte(other: API): boolean { return semver.gte(this.version, other.version); } diff --git a/extensions/typescript-language-features/src/utils/versionPicker.ts b/extensions/typescript-language-features/src/utils/versionPicker.ts index 20bae2ab64d..de042b3b01a 100644 --- a/extensions/typescript-language-features/src/utils/versionPicker.ts +++ b/extensions/typescript-language-features/src/utils/versionPicker.ts @@ -66,7 +66,7 @@ export class TypeScriptVersionPicker { for (const version of this.versionProvider.localVersions) { pickOptions.push({ - label: (this.useWorkspaceTsdkSetting && this.currentVersion.path === version.path + label: (this.useWorkspaceTsdkSetting && this.currentVersion.eq(version) ? '• ' : '') + localize('useWorkspaceVersionOption', "Use Workspace Version"), description: version.displayName, diff --git a/extensions/typescript-language-features/src/utils/versionProvider.ts b/extensions/typescript-language-features/src/utils/versionProvider.ts index 2864d9f9273..8023631c46d 100644 --- a/extensions/typescript-language-features/src/utils/versionProvider.ts +++ b/extensions/typescript-language-features/src/utils/versionProvider.ts @@ -21,11 +21,16 @@ export const enum TypeScriptVersionSource { } export class TypeScriptVersion { + + public readonly apiVersion: API | undefined; + constructor( public readonly source: TypeScriptVersionSource, public readonly path: string, private readonly _pathLabel?: string - ) { } + ) { + this.apiVersion = TypeScriptVersion.getApiVersion(this.tsServerPath); + } public get tsServerPath(): string { return path.join(this.path, 'tsserver.js'); @@ -39,8 +44,28 @@ export class TypeScriptVersion { return this.apiVersion !== undefined; } - public get apiVersion(): API | undefined { - const version = this.getTypeScriptVersion(this.tsServerPath); + public eq(other: TypeScriptVersion): boolean { + if (this.path !== other.path) { + return false; + } + + if (this.apiVersion === other.apiVersion) { + return true; + } + if (!this.apiVersion || !other.apiVersion) { + return false; + } + return this.apiVersion.eq(other.apiVersion); + } + + public get displayName(): string { + const version = this.apiVersion; + return version ? version.displayName : localize( + 'couldNotLoadTsVersion', 'Could not load the TypeScript version at this path'); + } + + public static getApiVersion(serverPath: string): API | undefined { + const version = TypeScriptVersion.getTypeScriptVersion(serverPath); if (version) { return version; } @@ -54,13 +79,7 @@ export class TypeScriptVersion { return undefined; } - public get displayName(): string { - const version = this.apiVersion; - return version ? version.displayName : localize( - 'couldNotLoadTsVersion', 'Could not load the TypeScript version at this path'); - } - - private getTypeScriptVersion(serverPath: string): API | undefined { + private static getTypeScriptVersion(serverPath: string): API | undefined { if (!fs.existsSync(serverPath)) { return undefined; }