From e2bbb6c3297b3968207e9b4af0902ad8e8f516ac Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Tue, 15 Feb 2022 19:17:32 -0800 Subject: [PATCH 01/12] Prototype TS plugins on web This prototype allows service plugins to be loaded on web TSServer Main changes: - Adds a new host entryPoint called `importServicePlugin` for overriding how plugins can be loaded. This may be async - Implement `importServicePlugin` for webServer - The web server plugin implementation looks for a `browser` field in the plugin's `package.json` - It then uses `import(...)` to load the plugin (the plugin source must be compiled to support being loaded as a module) --- src/server/project.ts | 23 +++++++++++++++++++++-- src/server/types.ts | 1 + src/webServer/webServer.ts | 18 ++++++++++++++++++ 3 files changed, 40 insertions(+), 2 deletions(-) diff --git a/src/server/project.ts b/src/server/project.ts index e6170d086ce81..7a963ddf03ff6 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -1577,7 +1577,7 @@ namespace ts.server { } } - protected enablePlugin(pluginConfigEntry: PluginImport, searchPaths: string[], pluginConfigOverrides: Map | undefined) { + protected async enablePlugin(pluginConfigEntry: PluginImport, searchPaths: string[], pluginConfigOverrides: Map | undefined) { this.projectService.logger.info(`Enabling plugin ${pluginConfigEntry.name} from candidate paths: ${searchPaths.join(",")}`); if (!pluginConfigEntry.name || parsePackageName(pluginConfigEntry.name).rest) { this.projectService.logger.info(`Skipped loading plugin ${pluginConfigEntry.name || JSON.stringify(pluginConfigEntry)} because only package name is allowed plugin name`); @@ -1589,8 +1589,27 @@ namespace ts.server { const logError = (message: string) => { (errorLogs || (errorLogs = [])).push(message); }; - const resolvedModule = firstDefined(searchPaths, searchPath => + + let resolvedModule: any | undefined; + if (this.projectService.host.importServicePlugin) { + for (const searchPath of searchPaths) { + try { + resolvedModule = await this.projectService.host.importServicePlugin(searchPath, pluginConfigEntry.name); + } + catch (e) { + // TODO: log this? + continue; + } + if (resolvedModule) { + break; + } + } + } + else { + resolvedModule = firstDefined(searchPaths, searchPath => Project.resolveModule(pluginConfigEntry.name, searchPath, this.projectService.host, log, logError) as PluginModuleFactory | undefined); + } + if (resolvedModule) { const configurationOverride = pluginConfigOverrides && pluginConfigOverrides.get(pluginConfigEntry.name); if (configurationOverride) { diff --git a/src/server/types.ts b/src/server/types.ts index 671e854440df3..87ba7f2329d13 100644 --- a/src/server/types.ts +++ b/src/server/types.ts @@ -16,5 +16,6 @@ declare namespace ts.server { gc?(): void; trace?(s: string): void; require?(initialPath: string, moduleName: string): RequireResult; + importServicePlugin?(root: string, moduleName: string): Promise; } } diff --git a/src/webServer/webServer.ts b/src/webServer/webServer.ts index 31f0a47915bf8..aa363e640f724 100644 --- a/src/webServer/webServer.ts +++ b/src/webServer/webServer.ts @@ -1,4 +1,6 @@ /*@internal*/ +/// + namespace ts.server { export interface HostWithWriteMessage { writeMessage(s: any): void; @@ -137,6 +139,22 @@ namespace ts.server { /* eslint-enable no-restricted-globals */ require: () => ({ module: undefined, error: new Error("Not implemented") }), + importServicePlugin: async (root: string, moduleName: string) => { + const packageRoot = combinePaths(root, "node_modules", moduleName); + + const packageJsonResponse = await fetch(combinePaths(packageRoot, "package.json")); + const packageJson = await packageJsonResponse.json(); + const browser = packageJson.browser; + if (!browser) { + throw new Error("Could not load plugin. No 'browser' field found in package.json."); + } + + const scriptPath = combinePaths(packageRoot, browser); + + // TODO: TS rewrites `import(...)` to `require`. Use eval to bypass this + // eslint-disable-next-line no-eval + return eval(`import(${JSON.stringify(scriptPath)})`); + }, exit: notImplemented, // Debugging related From 53d90b6636f90e3e5b8f96409e8a459c69b08c4b Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Wed, 23 Feb 2022 17:11:52 -0800 Subject: [PATCH 02/12] use default export from plugins This more or less matches how node plugins expect the plugin module to be an init function --- src/webServer/webServer.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/webServer/webServer.ts b/src/webServer/webServer.ts index aa363e640f724..a7478df66b591 100644 --- a/src/webServer/webServer.ts +++ b/src/webServer/webServer.ts @@ -153,7 +153,7 @@ namespace ts.server { // TODO: TS rewrites `import(...)` to `require`. Use eval to bypass this // eslint-disable-next-line no-eval - return eval(`import(${JSON.stringify(scriptPath)})`); + return (await eval(`import(${JSON.stringify(scriptPath)})`)).default; }, exit: notImplemented, From 34a83a370d9ec649d58a013d3dd658735b156400 Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Fri, 4 Mar 2022 13:40:03 -0800 Subject: [PATCH 03/12] Allow configure plugin requests against any web servers in partial semantic mode --- src/server/session.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/server/session.ts b/src/server/session.ts index b0bb42dade1ca..ebe4f3b237cfd 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -646,7 +646,6 @@ namespace ts.server { CommandNames.OrganizeImportsFull, CommandNames.GetEditsForFileRename, CommandNames.GetEditsForFileRenameFull, - CommandNames.ConfigurePlugin, CommandNames.PrepareCallHierarchy, CommandNames.ProvideCallHierarchyIncomingCalls, CommandNames.ProvideCallHierarchyOutgoingCalls, From 51680a1faa7303249bfd0e006c191ff409b445f3 Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Mon, 7 Mar 2022 18:42:57 -0800 Subject: [PATCH 04/12] Addressing some comments - Use result value instead of try/catch (`ImportPluginResult`) - Add awaits - Add logging --- src/server/project.ts | 33 ++++++++++++++++----------------- src/server/types.ts | 4 +++- src/webServer/webServer.ts | 23 ++++++++++++++++++----- 3 files changed, 37 insertions(+), 23 deletions(-) diff --git a/src/server/project.ts b/src/server/project.ts index 7a963ddf03ff6..b8cf6476b7a21 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -1545,7 +1545,7 @@ namespace ts.server { return !!this.program && this.program.isSourceOfProjectReferenceRedirect(fileName); } - protected enableGlobalPlugins(options: CompilerOptions, pluginConfigOverrides: Map | undefined) { + protected async enableGlobalPlugins(options: CompilerOptions, pluginConfigOverrides: Map | undefined): Promise { const host = this.projectService.host; if (!host.require) { @@ -1555,9 +1555,9 @@ namespace ts.server { // Search any globally-specified probe paths, then our peer node_modules const searchPaths = [ - ...this.projectService.pluginProbeLocations, - // ../../.. to walk from X/node_modules/typescript/lib/tsserver.js to X/node_modules/ - combinePaths(this.projectService.getExecutingFilePath(), "../../.."), + ...this.projectService.pluginProbeLocations, + // ../../.. to walk from X/node_modules/typescript/lib/tsserver.js to X/node_modules/ + combinePaths(this.projectService.getExecutingFilePath(), "../../.."), ]; if (this.projectService.globalPlugins) { @@ -1572,12 +1572,12 @@ namespace ts.server { // Provide global: true so plugins can detect why they can't find their config this.projectService.logger.info(`Loading global plugin ${globalPluginName}`); - this.enablePlugin({ name: globalPluginName, global: true } as PluginImport, searchPaths, pluginConfigOverrides); + await this.enablePlugin({ name: globalPluginName, global: true } as PluginImport, searchPaths, pluginConfigOverrides); } } } - protected async enablePlugin(pluginConfigEntry: PluginImport, searchPaths: string[], pluginConfigOverrides: Map | undefined) { + protected async enablePlugin(pluginConfigEntry: PluginImport, searchPaths: string[], pluginConfigOverrides: Map | undefined): Promise { this.projectService.logger.info(`Enabling plugin ${pluginConfigEntry.name} from candidate paths: ${searchPaths.join(",")}`); if (!pluginConfigEntry.name || parsePackageName(pluginConfigEntry.name).rest) { this.projectService.logger.info(`Skipped loading plugin ${pluginConfigEntry.name || JSON.stringify(pluginConfigEntry)} because only package name is allowed plugin name`); @@ -1593,21 +1593,20 @@ namespace ts.server { let resolvedModule: any | undefined; if (this.projectService.host.importServicePlugin) { for (const searchPath of searchPaths) { - try { - resolvedModule = await this.projectService.host.importServicePlugin(searchPath, pluginConfigEntry.name); + const result = await this.projectService.host.importServicePlugin(searchPath, pluginConfigEntry.name); + if (result.error) { + logError(result.error.toString()); } - catch (e) { - // TODO: log this? - continue; - } - if (resolvedModule) { + else { + resolvedModule = result.module; break; } + } } else { resolvedModule = firstDefined(searchPaths, searchPath => - Project.resolveModule(pluginConfigEntry.name, searchPath, this.projectService.host, log, logError) as PluginModuleFactory | undefined); + Project.resolveModule(pluginConfigEntry.name, searchPath, this.projectService.host, log, logError) as PluginModuleFactory | undefined); } if (resolvedModule) { @@ -2290,7 +2289,7 @@ namespace ts.server { } /*@internal*/ - enablePluginsWithOptions(options: CompilerOptions, pluginConfigOverrides: ESMap | undefined) { + async enablePluginsWithOptions(options: CompilerOptions, pluginConfigOverrides: ESMap | undefined): Promise { const host = this.projectService.host; if (!host.require) { @@ -2311,11 +2310,11 @@ namespace ts.server { // Enable tsconfig-specified plugins if (options.plugins) { for (const pluginConfigEntry of options.plugins) { - this.enablePlugin(pluginConfigEntry, searchPaths, pluginConfigOverrides); + await this.enablePlugin(pluginConfigEntry, searchPaths, pluginConfigOverrides); } } - this.enableGlobalPlugins(options, pluginConfigOverrides); + return this.enableGlobalPlugins(options, pluginConfigOverrides); } /** diff --git a/src/server/types.ts b/src/server/types.ts index 87ba7f2329d13..7e8a2de9d352e 100644 --- a/src/server/types.ts +++ b/src/server/types.ts @@ -6,6 +6,8 @@ declare namespace ts.server { } export type RequireResult = { module: {}, error: undefined } | { module: undefined, error: { stack?: string, message?: string } }; + export type ImportPluginResult = { module: {}, error: undefined } | { module: undefined, error: { stack?: string, message: string } }; + export interface ServerHost extends System { watchFile(path: string, callback: FileWatcherCallback, pollingInterval?: number, options?: WatchOptions): FileWatcher; watchDirectory(path: string, callback: DirectoryWatcherCallback, recursive?: boolean, options?: WatchOptions): FileWatcher; @@ -16,6 +18,6 @@ declare namespace ts.server { gc?(): void; trace?(s: string): void; require?(initialPath: string, moduleName: string): RequireResult; - importServicePlugin?(root: string, moduleName: string): Promise; + importServicePlugin?(root: string, moduleName: string): Promise; } } diff --git a/src/webServer/webServer.ts b/src/webServer/webServer.ts index a7478df66b591..cf24134da4bf3 100644 --- a/src/webServer/webServer.ts +++ b/src/webServer/webServer.ts @@ -139,21 +139,34 @@ namespace ts.server { /* eslint-enable no-restricted-globals */ require: () => ({ module: undefined, error: new Error("Not implemented") }), - importServicePlugin: async (root: string, moduleName: string) => { + importServicePlugin: async (root: string, moduleName: string): Promise => { const packageRoot = combinePaths(root, "node_modules", moduleName); - const packageJsonResponse = await fetch(combinePaths(packageRoot, "package.json")); - const packageJson = await packageJsonResponse.json(); + let packageJson: any | undefined; + try { + const packageJsonResponse = await fetch(combinePaths(packageRoot, "package.json")); + packageJson = await packageJsonResponse.json(); + } + catch (e) { + return { module: undefined, error: new Error("Could not load plugin. Could not load 'package.json'.") }; + } + const browser = packageJson.browser; if (!browser) { - throw new Error("Could not load plugin. No 'browser' field found in package.json."); + return { module: undefined, error: new Error("Could not load plugin. No 'browser' field found in package.json.") }; } const scriptPath = combinePaths(packageRoot, browser); // TODO: TS rewrites `import(...)` to `require`. Use eval to bypass this // eslint-disable-next-line no-eval - return (await eval(`import(${JSON.stringify(scriptPath)})`)).default; + try { + const module = (await eval(`import(${JSON.stringify(scriptPath)})`)).default; + return { module, error: undefined }; + } + catch (e) { + return { module: undefined, error: e }; + } }, exit: notImplemented, From 85e0fcad540d97609b862c25db7fc7bec340a8e7 Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Wed, 4 May 2022 11:18:26 -0700 Subject: [PATCH 05/12] add tsserverWeb to patch in dynamic import --- Gulpfile.js | 16 +++++++++++++--- scripts/produceLKG.ts | 1 + src/tsconfig.json | 1 + src/tsserverWeb/tsconfig.json | 16 ++++++++++++++++ src/tsserverWeb/tsserverWeb.ts | 3 +++ src/webServer/webServer.ts | 30 ++++++++++++++++++++++++++---- 6 files changed, 60 insertions(+), 7 deletions(-) create mode 100644 src/tsserverWeb/tsconfig.json create mode 100644 src/tsserverWeb/tsserverWeb.ts diff --git a/Gulpfile.js b/Gulpfile.js index c9d3feff80b01..a5a5536bda961 100644 --- a/Gulpfile.js +++ b/Gulpfile.js @@ -214,20 +214,29 @@ task("watch-services").flags = { " --built": "Compile using the built version of the compiler." }; -const buildServer = () => buildProject("src/tsserver", cmdLineOptions); +const buildServerWeb = () => buildProject("src/tsserverWeb", cmdLineOptions); +task("tsserverWeb", buildServerWeb); + +const buildServerMain = () => buildProject("src/tsserver", cmdLineOptions); +const buildServer = series(buildServerWeb, buildServerMain); +buildServer.displayName = "buildServer"; task("tsserver", series(preBuild, buildServer)); task("tsserver").description = "Builds the language server"; task("tsserver").flags = { " --built": "Compile using the built version of the compiler." }; -const cleanServer = () => cleanProject("src/tsserver"); +const cleanServerWeb = () => cleanProject("src/tsserverWeb"); +const cleanServerMain = () => cleanProject("src/tsserver"); +const cleanServer = series(cleanServerWeb, cleanServerMain); +cleanServer.displayName = "cleanServer"; cleanTasks.push(cleanServer); task("clean-tsserver", cleanServer); task("clean-tsserver").description = "Cleans outputs for the language server"; +const watchServerWeb = () => watchProject("src/tsserverWeb", cmdLineOptions); const watchServer = () => watchProject("src/tsserver", cmdLineOptions); -task("watch-tsserver", series(preBuild, parallel(watchLib, watchDiagnostics, watchServer))); +task("watch-tsserver", series(preBuild, parallel(watchLib, watchDiagnostics, watchServerWeb, watchServer))); task("watch-tsserver").description = "Watch for changes and rebuild the language server only"; task("watch-tsserver").flags = { " --built": "Compile using the built version of the compiler." @@ -549,6 +558,7 @@ const produceLKG = async () => { "built/local/typescriptServices.js", "built/local/typescriptServices.d.ts", "built/local/tsserver.js", + "built/local/tsserverWeb.js", "built/local/typescript.js", "built/local/typescript.d.ts", "built/local/tsserverlibrary.js", diff --git a/scripts/produceLKG.ts b/scripts/produceLKG.ts index 89199df6125c0..45fd5bd17c706 100644 --- a/scripts/produceLKG.ts +++ b/scripts/produceLKG.ts @@ -62,6 +62,7 @@ async function copyScriptOutputs() { await copyWithCopyright("cancellationToken.js"); await copyWithCopyright("tsc.release.js", "tsc.js"); await copyWithCopyright("tsserver.js"); + await copyWithCopyright("tsserverWeb.js"); await copyFromBuiltLocal("tsserverlibrary.js"); // copyright added by build await copyFromBuiltLocal("typescript.js"); // copyright added by build await copyFromBuiltLocal("typescriptServices.js"); // copyright added by build diff --git a/src/tsconfig.json b/src/tsconfig.json index 8a0f4dbce0ed8..a23eca9ac3472 100644 --- a/src/tsconfig.json +++ b/src/tsconfig.json @@ -9,6 +9,7 @@ { "path": "./watchGuard" }, { "path": "./debug" }, { "path": "./cancellationToken" }, + { "path": "./tsserverWeb" }, { "path": "./testRunner" } ] } \ No newline at end of file diff --git a/src/tsserverWeb/tsconfig.json b/src/tsserverWeb/tsconfig.json new file mode 100644 index 0000000000000..10d9cd1916865 --- /dev/null +++ b/src/tsserverWeb/tsconfig.json @@ -0,0 +1,16 @@ +{ + "extends": "../tsconfig-library-base", + "compilerOptions": { + "outDir": "../../built/local", + "rootDir": ".", + "target": "esnext", + "module": "esnext", + "lib": ["esnext"], + "declaration": false, + "sourceMap": true, + "tsBuildInfoFile": "../../built/local/tsserverWeb.tsbuildinfo" + }, + "files": [ + "tsserverWeb.ts", + ] +} diff --git a/src/tsserverWeb/tsserverWeb.ts b/src/tsserverWeb/tsserverWeb.ts new file mode 100644 index 0000000000000..aa21ba70a9566 --- /dev/null +++ b/src/tsserverWeb/tsserverWeb.ts @@ -0,0 +1,3 @@ +namespace ts.server { + export const dynamicImport = (id: string) => import(id); +} \ No newline at end of file diff --git a/src/webServer/webServer.ts b/src/webServer/webServer.ts index cf24134da4bf3..fd252656575ea 100644 --- a/src/webServer/webServer.ts +++ b/src/webServer/webServer.ts @@ -1,5 +1,6 @@ /*@internal*/ /// +/// namespace ts.server { export interface HostWithWriteMessage { @@ -111,11 +112,35 @@ namespace ts.server { } } + export declare const dynamicImport: ((id: string) => Promise) | undefined; + + // Attempt to load `dynamicImport` + if (typeof importScripts === "function") { + try { + // NOTE: importScripts is synchronous + importScripts("tsserverWeb.js"); + } + catch { + // ignored + } + } + export function createWebSystem(host: WebHost, args: string[], getExecutingFilePath: () => string): ServerHost { const returnEmptyString = () => ""; const getExecutingDirectoryPath = memoize(() => memoize(() => ensureTrailingDirectorySeparator(getDirectoryPath(getExecutingFilePath())))); // Later we could map ^memfs:/ to do something special if we want to enable more functionality like module resolution or something like that const getWebPath = (path: string) => startsWith(path, directorySeparator) ? path.replace(directorySeparator, getExecutingDirectoryPath()) : undefined; + + const dynamicImport = async (id: string): Promise => { + // Use syntactic dynamic import first, if available + if (ts.server.dynamicImport) { + return ts.server.dynamicImport(id); + } + + // Fall back to `eval` if dynamic import wasn't avaialble. + return eval(`import(${JSON.stringify(id)})`); // eslint-disable-line no-eval + }; + return { args, newLine: "\r\n", // This can be configured by clients @@ -157,11 +182,8 @@ namespace ts.server { } const scriptPath = combinePaths(packageRoot, browser); - - // TODO: TS rewrites `import(...)` to `require`. Use eval to bypass this - // eslint-disable-next-line no-eval try { - const module = (await eval(`import(${JSON.stringify(scriptPath)})`)).default; + const { default: module } = await dynamicImport(scriptPath); return { module, error: undefined }; } catch (e) { From f40a867deebe97f5852158b153e5e771f3d3e1a5 Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Tue, 10 May 2022 12:00:28 -0700 Subject: [PATCH 06/12] Remove eval We should throw instead when dynamic import is not implemented --- src/webServer/webServer.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/webServer/webServer.ts b/src/webServer/webServer.ts index fd252656575ea..81e07d3d2aec4 100644 --- a/src/webServer/webServer.ts +++ b/src/webServer/webServer.ts @@ -137,8 +137,7 @@ namespace ts.server { return ts.server.dynamicImport(id); } - // Fall back to `eval` if dynamic import wasn't avaialble. - return eval(`import(${JSON.stringify(id)})`); // eslint-disable-line no-eval + throw new Error("Dynamic import not implemented"); }; return { From 186cec9ecdc302ad3e1ac74416ba6db71b8f5d0e Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Tue, 24 May 2022 18:27:01 -0700 Subject: [PATCH 07/12] Ensure dynamically imported plugins are loaded in the correct order --- src/server/editorServices.ts | 91 +++++++++++++++++++ src/server/project.ts | 80 +++++++++++----- src/server/session.ts | 4 +- src/webServer/webServer.ts | 5 +- .../reference/api/tsserverlibrary.d.ts | 16 ++++ 5 files changed, 167 insertions(+), 29 deletions(-) diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 149332bb6f646..49cc25225dca7 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -802,6 +802,9 @@ namespace ts.server { private performanceEventHandler?: PerformanceEventHandler; + private pendingPluginEnablements?: ESMap[]>; + private currentPluginEnablementPromise?: Promise; + constructor(opts: ProjectServiceOptions) { this.host = opts.host; this.logger = opts.logger; @@ -4056,6 +4059,94 @@ namespace ts.server { return false; } + /*@internal*/ + requestEnablePlugin(project: Project, pluginConfigEntry: PluginImport, searchPaths: string[], pluginConfigOverrides: Map | undefined) { + if (!this.host.importServicePlugin && !this.host.require) { + this.logger.info("Plugins were requested but not running in environment that supports 'require'. Nothing will be loaded"); + return; + } + + this.logger.info(`Enabling plugin ${pluginConfigEntry.name} from candidate paths: ${searchPaths.join(",")}`); + if (!pluginConfigEntry.name || parsePackageName(pluginConfigEntry.name).rest) { + this.logger.info(`Skipped loading plugin ${pluginConfigEntry.name || JSON.stringify(pluginConfigEntry)} because only package name is allowed plugin name`); + return; + } + + // If the host supports dynamic import, begin enabling the plugin asynchronously. + if (this.host.importServicePlugin) { + const importPromise = project.beginEnablePluginAsync(pluginConfigEntry, searchPaths, pluginConfigOverrides); + this.pendingPluginEnablements ??= new Map(); + let promises = this.pendingPluginEnablements.get(project); + if (!promises) this.pendingPluginEnablements.set(project, promises = []); + promises.push(importPromise); + return; + } + + // Otherwise, load the plugin using `resolve` + project.endEnablePlugin(project.beginEnablePluginSync(pluginConfigEntry, searchPaths, pluginConfigOverrides)); + } + + /** + * Waits for any ongoing plugin enablement requests to complete. + */ + /* @internal */ + async waitForPendingPlugins() { + while (this.currentPluginEnablementPromise) { + await this.currentPluginEnablementPromise; + } + } + + /** + * Starts enabling any requested plugins without waiting for the result. + */ + /* @internal */ + enableRequestedPlugins() { + if (this.pendingPluginEnablements) { + void this.enableRequestedPluginsAsync(); + } + } + + private async enableRequestedPluginsAsync() { + // If we're already enabling plugins, wait for any existing operations to complete + await this.waitForPendingPlugins(); + + // Skip if there are no new plugin enablement requests + if (!this.pendingPluginEnablements) { + return; + } + + // Consume the pending plugin enablement requests + const entries = arrayFrom(this.pendingPluginEnablements.entries()); + this.pendingPluginEnablements = undefined; + + // Start processing the requests, keeping track of the promise for the operation so that + // project consumers can potentially wait for the plugins to load. + this.currentPluginEnablementPromise = this.enableRequestedPluginsWorker(entries); + await this.currentPluginEnablementPromise; + } + + private async enableRequestedPluginsWorker(pendingPlugins: [Project, Promise[]][]) { + // This should only be called from `enableRequestedServicePlugins`, which ensures this precondition is met. + Debug.assert(this.currentPluginEnablementPromise === undefined); + + // Process all pending plugins, partitioned by project. This way a project with few plugins doesn't need to wait + // on a project with many plugins. + await Promise.all(map(pendingPlugins, ([project, promises]) => this.enableRequestedPluginsForProjectAsync(project, promises))); + + // Clear the pending operation and notify the client that projects have been updated. + this.currentPluginEnablementPromise = undefined; + this.sendProjectsUpdatedInBackgroundEvent(); + } + + private async enableRequestedPluginsForProjectAsync(project: Project, promises: Promise[]) { + // Await all pending plugin imports. This ensures all requested plugin modules are fully loaded + // prior to patching the language service. + const results = await Promise.all(promises); + for (const result of results) { + project.endEnablePlugin(result); + } + } + configurePlugin(args: protocol.ConfigurePluginRequestArguments) { // For any projects that already have the plugin loaded, configure the plugin this.forEachEnabledProject(project => project.onPluginConfigurationChanged(args.pluginName, args.configuration)); diff --git a/src/server/project.ts b/src/server/project.ts index b8cf6476b7a21..4a2687fe75773 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -101,6 +101,14 @@ namespace ts.server { export type PluginModuleFactory = (mod: { typescript: typeof ts }) => PluginModule; + /* @internal */ + export interface BeginEnablePluginResult { + pluginConfigEntry: PluginImport; + pluginConfigOverrides: Map | undefined; + resolvedModule: PluginModuleFactory | undefined; + errorLogs: string[] | undefined; + } + /** * The project root can be script info - if root is present, * or it could be just normalized path if root wasn't present on the host(only for non inferred project) @@ -133,6 +141,7 @@ namespace ts.server { private externalFiles: SortedReadonlyArray | undefined; private missingFilesMap: ESMap | undefined; private generatedFilesMap: GeneratedFileWatcherMap | undefined; + private plugins: PluginModuleWithName[] = []; /*@internal*/ @@ -1545,10 +1554,10 @@ namespace ts.server { return !!this.program && this.program.isSourceOfProjectReferenceRedirect(fileName); } - protected async enableGlobalPlugins(options: CompilerOptions, pluginConfigOverrides: Map | undefined): Promise { + protected enableGlobalPlugins(options: CompilerOptions, pluginConfigOverrides: Map | undefined): void { const host = this.projectService.host; - if (!host.require) { + if (!host.require && !host.importServicePlugin) { this.projectService.logger.info("Plugins were requested but not running in environment that supports 'require'. Nothing will be loaded"); return; } @@ -1572,43 +1581,60 @@ namespace ts.server { // Provide global: true so plugins can detect why they can't find their config this.projectService.logger.info(`Loading global plugin ${globalPluginName}`); - await this.enablePlugin({ name: globalPluginName, global: true } as PluginImport, searchPaths, pluginConfigOverrides); + this.enablePlugin({ name: globalPluginName, global: true } as PluginImport, searchPaths, pluginConfigOverrides); } } } - protected async enablePlugin(pluginConfigEntry: PluginImport, searchPaths: string[], pluginConfigOverrides: Map | undefined): Promise { - this.projectService.logger.info(`Enabling plugin ${pluginConfigEntry.name} from candidate paths: ${searchPaths.join(",")}`); - if (!pluginConfigEntry.name || parsePackageName(pluginConfigEntry.name).rest) { - this.projectService.logger.info(`Skipped loading plugin ${pluginConfigEntry.name || JSON.stringify(pluginConfigEntry)} because only package name is allowed plugin name`); - return; - } + /** + * Performs the initial steps of enabling a plugin by finding and instantiating the module for a plugin synchronously using 'require'. + */ + /*@internal*/ + beginEnablePluginSync(pluginConfigEntry: PluginImport, searchPaths: string[], pluginConfigOverrides: Map | undefined): BeginEnablePluginResult { + Debug.assertIsDefined(this.projectService.host.require); - const log = (message: string) => this.projectService.logger.info(message); let errorLogs: string[] | undefined; + const log = (message: string) => this.projectService.logger.info(message); const logError = (message: string) => { - (errorLogs || (errorLogs = [])).push(message); + (errorLogs ??= []).push(message); }; + const resolvedModule = firstDefined(searchPaths, searchPath => + Project.resolveModule(pluginConfigEntry.name, searchPath, this.projectService.host, log, logError) as PluginModuleFactory | undefined); + return { pluginConfigEntry, pluginConfigOverrides, resolvedModule, errorLogs }; + } - let resolvedModule: any | undefined; - if (this.projectService.host.importServicePlugin) { - for (const searchPath of searchPaths) { + /** + * Performs the initial steps of enabling a plugin by finding and instantiating the module for a plugin asynchronously using dynamic `import`. + */ + /*@internal*/ + async beginEnablePluginAsync(pluginConfigEntry: PluginImport, searchPaths: string[], pluginConfigOverrides: Map | undefined): Promise { + Debug.assertIsDefined(this.projectService.host.importServicePlugin); + + let errorLogs: string[] | undefined; + let resolvedModule: PluginModuleFactory | undefined; + for (const searchPath of searchPaths) { + try { const result = await this.projectService.host.importServicePlugin(searchPath, pluginConfigEntry.name); if (result.error) { - logError(result.error.toString()); + (errorLogs ??= []).push(result.error.toString()); } else { - resolvedModule = result.module; + resolvedModule = result.module as PluginModuleFactory; break; } - + } + catch (e) { + (errorLogs ??= []).push(`${e}`); } } - else { - resolvedModule = firstDefined(searchPaths, searchPath => - Project.resolveModule(pluginConfigEntry.name, searchPath, this.projectService.host, log, logError) as PluginModuleFactory | undefined); - } + return { pluginConfigEntry, pluginConfigOverrides, resolvedModule, errorLogs }; + } + /** + * Performs the remaining steps of enabling a plugin after its module has been instantiated. + */ + /*@internal*/ + endEnablePlugin({ pluginConfigEntry, pluginConfigOverrides, resolvedModule, errorLogs }: BeginEnablePluginResult) { if (resolvedModule) { const configurationOverride = pluginConfigOverrides && pluginConfigOverrides.get(pluginConfigEntry.name); if (configurationOverride) { @@ -1621,11 +1647,15 @@ namespace ts.server { this.enableProxy(resolvedModule, pluginConfigEntry); } else { - forEach(errorLogs, log); + forEach(errorLogs, message => this.projectService.logger.info(message)); this.projectService.logger.info(`Couldn't find ${pluginConfigEntry.name}`); } } + protected enablePlugin(pluginConfigEntry: PluginImport, searchPaths: string[], pluginConfigOverrides: Map | undefined): void { + this.projectService.requestEnablePlugin(this, pluginConfigEntry, searchPaths, pluginConfigOverrides); + } + private enableProxy(pluginModuleFactory: PluginModuleFactory, configEntry: PluginImport) { try { if (typeof pluginModuleFactory !== "function") { @@ -2289,10 +2319,10 @@ namespace ts.server { } /*@internal*/ - async enablePluginsWithOptions(options: CompilerOptions, pluginConfigOverrides: ESMap | undefined): Promise { + enablePluginsWithOptions(options: CompilerOptions, pluginConfigOverrides: ESMap | undefined): void { const host = this.projectService.host; - if (!host.require) { + if (!host.require && !host.importServicePlugin) { this.projectService.logger.info("Plugins were requested but not running in environment that supports 'require'. Nothing will be loaded"); return; } @@ -2310,7 +2340,7 @@ namespace ts.server { // Enable tsconfig-specified plugins if (options.plugins) { for (const pluginConfigEntry of options.plugins) { - await this.enablePlugin(pluginConfigEntry, searchPaths, pluginConfigOverrides); + this.enablePlugin(pluginConfigEntry, searchPaths, pluginConfigOverrides); } } diff --git a/src/server/session.ts b/src/server/session.ts index ebe4f3b237cfd..a70d29219d981 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -3024,7 +3024,9 @@ namespace ts.server { public executeCommand(request: protocol.Request): HandlerResponse { const handler = this.handlers.get(request.command); if (handler) { - return this.executeWithRequestId(request.seq, () => handler(request)); + const response = this.executeWithRequestId(request.seq, () => handler(request)); + this.projectService.enableRequestedPlugins(); + return response; } else { this.logger.msg(`Unrecognized JSON command:${stringifyIndented(request)}`, Msg.Err); diff --git a/src/webServer/webServer.ts b/src/webServer/webServer.ts index 81e07d3d2aec4..77536b0def997 100644 --- a/src/webServer/webServer.ts +++ b/src/webServer/webServer.ts @@ -133,8 +133,8 @@ namespace ts.server { const dynamicImport = async (id: string): Promise => { // Use syntactic dynamic import first, if available - if (ts.server.dynamicImport) { - return ts.server.dynamicImport(id); + if (server.dynamicImport) { + return server.dynamicImport(id); } throw new Error("Dynamic import not implemented"); @@ -162,7 +162,6 @@ namespace ts.server { clearImmediate: handle => clearTimeout(handle), /* eslint-enable no-restricted-globals */ - require: () => ({ module: undefined, error: new Error("Not implemented") }), importServicePlugin: async (root: string, moduleName: string): Promise => { const packageRoot = combinePaths(root, "node_modules", moduleName); diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index f822907cd1430..b8998da185042 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -6844,6 +6844,16 @@ declare namespace ts.server { message?: string; }; }; + type ImportPluginResult = { + module: {}; + error: undefined; + } | { + module: undefined; + error: { + stack?: string; + message: string; + }; + }; interface ServerHost extends System { watchFile(path: string, callback: FileWatcherCallback, pollingInterval?: number, options?: WatchOptions): FileWatcher; watchDirectory(path: string, callback: DirectoryWatcherCallback, recursive?: boolean, options?: WatchOptions): FileWatcher; @@ -6854,6 +6864,7 @@ declare namespace ts.server { gc?(): void; trace?(s: string): void; require?(initialPath: string, moduleName: string): RequireResult; + importServicePlugin?(root: string, moduleName: string): Promise; } } declare namespace ts.server { @@ -10255,6 +10266,8 @@ declare namespace ts.server { /** Tracks projects that we have already sent telemetry for. */ private readonly seenProjects; private performanceEventHandler?; + private pendingPluginEnablements?; + private currentPluginEnablementPromise?; constructor(opts: ProjectServiceOptions); toPath(fileName: string): Path; private loadTypesMap; @@ -10414,6 +10427,9 @@ declare namespace ts.server { applySafeList(proj: protocol.ExternalProject): NormalizedPath[]; openExternalProject(proj: protocol.ExternalProject): void; hasDeferredExtension(): boolean; + private enableRequestedPluginsAsync; + private enableRequestedPluginsWorker; + private enableRequestedPluginsForProjectAsync; configurePlugin(args: protocol.ConfigurePluginRequestArguments): void; } export {}; From 39dac60da9461a297fbf000049477ed66fa6e13b Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Wed, 25 May 2022 15:59:03 -0700 Subject: [PATCH 08/12] Add tests for async service plugin timing --- src/server/editorServices.ts | 16 ++- .../unittests/tsserver/webServer.ts | 119 +++++++++++++++++- 2 files changed, 131 insertions(+), 4 deletions(-) diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 49cc25225dca7..d30ac381dbad4 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -4086,6 +4086,16 @@ namespace ts.server { project.endEnablePlugin(project.beginEnablePluginSync(pluginConfigEntry, searchPaths, pluginConfigOverrides)); } + /* @internal */ + hasNewPluginEnablementRequests() { + return !!this.pendingPluginEnablements; + } + + /* @internal */ + hasPendingPluginEnablements() { + return !!this.currentPluginEnablementPromise; + } + /** * Waits for any ongoing plugin enablement requests to complete. */ @@ -4107,8 +4117,10 @@ namespace ts.server { } private async enableRequestedPluginsAsync() { - // If we're already enabling plugins, wait for any existing operations to complete - await this.waitForPendingPlugins(); + if (this.currentPluginEnablementPromise) { + // If we're already enabling plugins, wait for any existing operations to complete + await this.waitForPendingPlugins(); + } // Skip if there are no new plugin enablement requests if (!this.pendingPluginEnablements) { diff --git a/src/testRunner/unittests/tsserver/webServer.ts b/src/testRunner/unittests/tsserver/webServer.ts index 1891596ffb067..f02cb7e7a0df6 100644 --- a/src/testRunner/unittests/tsserver/webServer.ts +++ b/src/testRunner/unittests/tsserver/webServer.ts @@ -1,3 +1,4 @@ +/* eslint-disable boolean-trivia */ namespace ts.projectSystem { describe("unittests:: tsserver:: webServer", () => { class TestWorkerSession extends server.WorkerSession { @@ -27,7 +28,8 @@ namespace ts.projectSystem { return this.projectService; } } - function setup(logLevel: server.LogLevel | undefined) { + + function setup(logLevel: server.LogLevel | undefined, options?: Partial, importServicePlugin?: server.ServerHost["importServicePlugin"]) { const host = createServerHost([libFile], { windowsStyleRoot: "c:/" }); const messages: any[] = []; const webHost: server.WebHost = { @@ -36,8 +38,9 @@ namespace ts.projectSystem { writeMessage: s => messages.push(s), }; const webSys = server.createWebSystem(webHost, emptyArray, () => host.getExecutingFilePath()); + if (importServicePlugin) webSys.importServicePlugin = importServicePlugin; const logger = logLevel !== undefined ? new server.MainProcessLogger(logLevel, webHost) : nullLogger(); - const session = new TestWorkerSession(webSys, webHost, { serverMode: LanguageServiceMode.PartialSemantic }, logger); + const session = new TestWorkerSession(webSys, webHost, { serverMode: LanguageServiceMode.PartialSemantic, ...options }, logger); return { getMessages: () => messages, clearMessages: () => messages.length = 0, session }; } @@ -153,5 +156,117 @@ namespace ts.projectSystem { verify(/*logLevel*/ undefined); }); }); + + describe("async loaded plugins", () => { + it("plugins are not loaded immediately", async () => { + let pluginModuleInstantiated = false; + let pluginInvoked = false; + const importServicePlugin = async (_root: string, _moduleName: string): Promise => { + await Promise.resolve(); // simulate at least a single turn delay + pluginModuleInstantiated = true; + return { + module: (() => { + pluginInvoked = true; + return { create: info => info.languageService }; + }) as server.PluginModuleFactory, + error: undefined + }; + }; + + const { session } = setup(/*logLevel*/ undefined, { globalPlugins: ["plugin-a"] }, importServicePlugin); + const projectService = session.getProjectService(); + + session.executeCommand({ seq: 1, type: "request", command: protocol.CommandTypes.Open, arguments: { file: "^memfs:/foo.ts", content: "" } }); + + // This should be false because `executeCommand` should have already triggered + // plugin enablement asynchronously and there are no plugin enablements currently + // being processed. + expect(projectService.hasNewPluginEnablementRequests()).eq(false); + + // Should be true because async imports have already been triggered in the background + expect(projectService.hasPendingPluginEnablements()).eq(true); + + // Should be false because resolution of async imports happens in a later turn. + expect(pluginModuleInstantiated).eq(false); + + await projectService.waitForPendingPlugins(); + + // at this point all plugin modules should have been instantiated and all plugins + // should have been invoked + expect(pluginModuleInstantiated).eq(true); + expect(pluginInvoked).eq(true); + }); + + it("plugins evaluation in correct order even if imports resolve out of order", async () => { + let resolvePluginA!: () => void; + let resolvePluginB!: () => void; + const pluginAPromise = new Promise(_resolve => resolvePluginA = _resolve); + const pluginBPromise = new Promise(_resolve => resolvePluginB = _resolve); + const log: string[] = []; + const importServicePlugin = async (_root: string, moduleName: string): Promise => { + log.push(`request import ${moduleName}`); + const promise = moduleName === "plugin-a" ? pluginAPromise : pluginBPromise; + await promise; + log.push(`fulfill import ${moduleName}`); + return { + module: (() => { + log.push(`invoke plugin ${moduleName}`); + return { create: info => info.languageService }; + }) as server.PluginModuleFactory, + error: undefined + }; + }; + + const { session } = setup(/*logLevel*/ undefined, { globalPlugins: ["plugin-a", "plugin-b"] }, importServicePlugin); + const projectService = session.getProjectService(); + + session.executeCommand({ seq: 1, type: "request", command: protocol.CommandTypes.Open, arguments: { file: "^memfs:/foo.ts", content: "" } }); + + // wait a turn + await Promise.resolve(); + + // resolve imports out of order + resolvePluginB(); + resolvePluginA(); + + // wait for load to complete + await projectService.waitForPendingPlugins(); + + expect(log).to.deep.equal([ + "request import plugin-a", + "request import plugin-b", + "fulfill import plugin-b", + "fulfill import plugin-a", + "invoke plugin plugin-a", + "invoke plugin plugin-b", + ]); + }); + + it("sends projectsUpdatedInBackground event", async () => { + const importServicePlugin = async (_root: string, _moduleName: string): Promise => { + await Promise.resolve(); // simulate at least a single turn delay + return { + module: (() => ({ create: info => info.languageService })) as server.PluginModuleFactory, + error: undefined + }; + }; + + const { session, getMessages } = setup(/*logLevel*/ undefined, { globalPlugins: ["plugin-a"] }, importServicePlugin); + const projectService = session.getProjectService(); + + session.executeCommand({ seq: 1, type: "request", command: protocol.CommandTypes.Open, arguments: { file: "^memfs:/foo.ts", content: "" } }); + + await projectService.waitForPendingPlugins(); + + expect(getMessages()).to.deep.equal([{ + seq: 0, + type: "event", + event: "projectsUpdatedInBackground", + body: { + openFiles: ["^memfs:/foo.ts"] + } + }]); + }); + }); }); } From 78e016c6299d2393dcd8ada85e490fd14a91f503 Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Thu, 2 Jun 2022 17:36:00 -0700 Subject: [PATCH 09/12] Update src/server/editorServices.ts Co-authored-by: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> --- src/server/editorServices.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index d30ac381dbad4..d9e548e0ca96f 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -4138,7 +4138,7 @@ namespace ts.server { } private async enableRequestedPluginsWorker(pendingPlugins: [Project, Promise[]][]) { - // This should only be called from `enableRequestedServicePlugins`, which ensures this precondition is met. + // This should only be called from `enableRequestedPluginsAsync`, which ensures this precondition is met. Debug.assert(this.currentPluginEnablementPromise === undefined); // Process all pending plugins, partitioned by project. This way a project with few plugins doesn't need to wait From c760041f8b3e45ca3c9273b4bf48375e63e722a4 Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Sat, 4 Jun 2022 12:40:28 -0700 Subject: [PATCH 10/12] Partial PR feedback --- src/server/editorServices.ts | 10 ++++- src/server/project.ts | 40 +++++++++++++------ src/server/types.ts | 10 +++-- .../unittests/tsserver/webServer.ts | 8 ++-- src/tsserver/nodeServer.ts | 2 +- src/webServer/webServer.ts | 4 +- .../reference/api/tsserverlibrary.d.ts | 18 +++------ 7 files changed, 55 insertions(+), 37 deletions(-) diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index d9e548e0ca96f..36090b908d286 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -4152,11 +4152,19 @@ namespace ts.server { private async enableRequestedPluginsForProjectAsync(project: Project, promises: Promise[]) { // Await all pending plugin imports. This ensures all requested plugin modules are fully loaded - // prior to patching the language service. + // prior to patching the language service, and that any promise rejections are observed. const results = await Promise.all(promises); + if (project.isClosed() && (!(project instanceof ConfiguredProject) || !project.hasOpenRef())) { + // project is not alive, so don't enable plugins. + return; + } + for (const result of results) { project.endEnablePlugin(result); } + + // Plugins may have modified external files, so mark the project as dirty. + project.markAsDirty(); } configurePlugin(args: protocol.ConfigurePluginRequestArguments) { diff --git a/src/server/project.ts b/src/server/project.ts index 4a2687fe75773..617c4b6781ee3 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -250,6 +250,26 @@ namespace ts.server { return result.module; } + /*@internal*/ + public static async importServicePluginAsync(moduleName: string, initialDir: string, host: ServerHost, log: (message: string) => void, logErrors?: (message: string) => void): Promise<{} | undefined> { + Debug.assertIsDefined(host.importServicePlugin); + const resolvedPath = combinePaths(initialDir, "node_modules"); + log(`Dynamically importing ${moduleName} from ${initialDir} (resolved to ${resolvedPath})`); + let result: ModuleImportResult; + try { + result = await host.importServicePlugin(resolvedPath, moduleName); + } + catch (e) { + result = { module: undefined, error: e }; + } + if (result.error) { + const err = result.error.stack || result.error.message || JSON.stringify(result.error); + (logErrors || log)(`Failed to dynamically import module '${moduleName}' from ${resolvedPath}: ${err}`); + return undefined; + } + return result.module; + } + /*@internal*/ readonly currentDirectory: string; @@ -1611,20 +1631,16 @@ namespace ts.server { Debug.assertIsDefined(this.projectService.host.importServicePlugin); let errorLogs: string[] | undefined; + const log = (message: string) => this.projectService.logger.info(message); + const logError = (message: string) => { + (errorLogs ??= []).push(message); + }; + let resolvedModule: PluginModuleFactory | undefined; for (const searchPath of searchPaths) { - try { - const result = await this.projectService.host.importServicePlugin(searchPath, pluginConfigEntry.name); - if (result.error) { - (errorLogs ??= []).push(result.error.toString()); - } - else { - resolvedModule = result.module as PluginModuleFactory; - break; - } - } - catch (e) { - (errorLogs ??= []).push(`${e}`); + resolvedModule = await Project.importServicePluginAsync(pluginConfigEntry.name, searchPath, this.projectService.host, log, logError) as PluginModuleFactory | undefined; + if (resolvedModule !== undefined) { + break; } } return { pluginConfigEntry, pluginConfigOverrides, resolvedModule, errorLogs }; diff --git a/src/server/types.ts b/src/server/types.ts index 7e8a2de9d352e..b19879a94920f 100644 --- a/src/server/types.ts +++ b/src/server/types.ts @@ -5,8 +5,10 @@ declare namespace ts.server { data: any; } - export type RequireResult = { module: {}, error: undefined } | { module: undefined, error: { stack?: string, message?: string } }; - export type ImportPluginResult = { module: {}, error: undefined } | { module: undefined, error: { stack?: string, message: string } }; + export type ModuleImportResult = { module: {}, error: undefined } | { module: undefined, error: { stack?: string, message?: string } }; + + /** @deprecated Use {@link ModuleImportResult} instead. */ + export type RequireResult = ModuleImportResult; export interface ServerHost extends System { watchFile(path: string, callback: FileWatcherCallback, pollingInterval?: number, options?: WatchOptions): FileWatcher; @@ -17,7 +19,7 @@ declare namespace ts.server { clearImmediate(timeoutId: any): void; gc?(): void; trace?(s: string): void; - require?(initialPath: string, moduleName: string): RequireResult; - importServicePlugin?(root: string, moduleName: string): Promise; + require?(initialPath: string, moduleName: string): ModuleImportResult; + importServicePlugin?(root: string, moduleName: string): Promise; } } diff --git a/src/testRunner/unittests/tsserver/webServer.ts b/src/testRunner/unittests/tsserver/webServer.ts index f02cb7e7a0df6..701b092dec705 100644 --- a/src/testRunner/unittests/tsserver/webServer.ts +++ b/src/testRunner/unittests/tsserver/webServer.ts @@ -38,7 +38,7 @@ namespace ts.projectSystem { writeMessage: s => messages.push(s), }; const webSys = server.createWebSystem(webHost, emptyArray, () => host.getExecutingFilePath()); - if (importServicePlugin) webSys.importServicePlugin = importServicePlugin; + webSys.importServicePlugin = importServicePlugin; const logger = logLevel !== undefined ? new server.MainProcessLogger(logLevel, webHost) : nullLogger(); const session = new TestWorkerSession(webSys, webHost, { serverMode: LanguageServiceMode.PartialSemantic, ...options }, logger); return { getMessages: () => messages, clearMessages: () => messages.length = 0, session }; @@ -161,7 +161,7 @@ namespace ts.projectSystem { it("plugins are not loaded immediately", async () => { let pluginModuleInstantiated = false; let pluginInvoked = false; - const importServicePlugin = async (_root: string, _moduleName: string): Promise => { + const importServicePlugin = async (_root: string, _moduleName: string): Promise => { await Promise.resolve(); // simulate at least a single turn delay pluginModuleInstantiated = true; return { @@ -203,7 +203,7 @@ namespace ts.projectSystem { const pluginAPromise = new Promise(_resolve => resolvePluginA = _resolve); const pluginBPromise = new Promise(_resolve => resolvePluginB = _resolve); const log: string[] = []; - const importServicePlugin = async (_root: string, moduleName: string): Promise => { + const importServicePlugin = async (_root: string, moduleName: string): Promise => { log.push(`request import ${moduleName}`); const promise = moduleName === "plugin-a" ? pluginAPromise : pluginBPromise; await promise; @@ -243,7 +243,7 @@ namespace ts.projectSystem { }); it("sends projectsUpdatedInBackground event", async () => { - const importServicePlugin = async (_root: string, _moduleName: string): Promise => { + const importServicePlugin = async (_root: string, _moduleName: string): Promise => { await Promise.resolve(); // simulate at least a single turn delay return { module: (() => ({ create: info => info.languageService })) as server.PluginModuleFactory, diff --git a/src/tsserver/nodeServer.ts b/src/tsserver/nodeServer.ts index c85943496e386..a660f9f2647a7 100644 --- a/src/tsserver/nodeServer.ts +++ b/src/tsserver/nodeServer.ts @@ -259,7 +259,7 @@ namespace ts.server { sys.gc = () => global.gc?.(); } - sys.require = (initialDir: string, moduleName: string): RequireResult => { + sys.require = (initialDir: string, moduleName: string): ModuleImportResult => { try { return { module: require(resolveJSModule(moduleName, initialDir, sys)), error: undefined }; } diff --git a/src/webServer/webServer.ts b/src/webServer/webServer.ts index 77536b0def997..8b2a4be061dd7 100644 --- a/src/webServer/webServer.ts +++ b/src/webServer/webServer.ts @@ -162,8 +162,8 @@ namespace ts.server { clearImmediate: handle => clearTimeout(handle), /* eslint-enable no-restricted-globals */ - importServicePlugin: async (root: string, moduleName: string): Promise => { - const packageRoot = combinePaths(root, "node_modules", moduleName); + importServicePlugin: async (initialDir: string, moduleName: string): Promise => { + const packageRoot = combinePaths(initialDir, moduleName); let packageJson: any | undefined; try { diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index b8998da185042..b8ec305a5738c 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -6834,7 +6834,7 @@ declare namespace ts.server { compressionKind: string; data: any; } - type RequireResult = { + type ModuleImportResult = { module: {}; error: undefined; } | { @@ -6844,16 +6844,8 @@ declare namespace ts.server { message?: string; }; }; - type ImportPluginResult = { - module: {}; - error: undefined; - } | { - module: undefined; - error: { - stack?: string; - message: string; - }; - }; + /** @deprecated Use {@link ModuleImportResult} instead. */ + type RequireResult = ModuleImportResult; interface ServerHost extends System { watchFile(path: string, callback: FileWatcherCallback, pollingInterval?: number, options?: WatchOptions): FileWatcher; watchDirectory(path: string, callback: DirectoryWatcherCallback, recursive?: boolean, options?: WatchOptions): FileWatcher; @@ -6863,8 +6855,8 @@ declare namespace ts.server { clearImmediate(timeoutId: any): void; gc?(): void; trace?(s: string): void; - require?(initialPath: string, moduleName: string): RequireResult; - importServicePlugin?(root: string, moduleName: string): Promise; + require?(initialPath: string, moduleName: string): ModuleImportResult; + importServicePlugin?(root: string, moduleName: string): Promise; } } declare namespace ts.server { From 9bc39d17b9ac2af4c31f13826069d6f4f2d2cb7a Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Sat, 4 Jun 2022 12:49:53 -0700 Subject: [PATCH 11/12] Rename tsserverWeb to dynamicImportCompat --- Gulpfile.js | 16 ++++++++-------- scripts/produceLKG.ts | 2 +- .../dynamicImportCompat.ts} | 0 .../tsconfig.json | 4 ++-- src/server/editorServices.ts | 2 +- src/tsconfig.json | 2 +- src/webServer/webServer.ts | 2 +- 7 files changed, 14 insertions(+), 14 deletions(-) rename src/{tsserverWeb/tsserverWeb.ts => dynamicImportCompat/dynamicImportCompat.ts} (100%) rename src/{tsserverWeb => dynamicImportCompat}/tsconfig.json (70%) diff --git a/Gulpfile.js b/Gulpfile.js index a5a5536bda961..a59894996e65a 100644 --- a/Gulpfile.js +++ b/Gulpfile.js @@ -214,11 +214,11 @@ task("watch-services").flags = { " --built": "Compile using the built version of the compiler." }; -const buildServerWeb = () => buildProject("src/tsserverWeb", cmdLineOptions); -task("tsserverWeb", buildServerWeb); +const buildDynamicImportCompat = () => buildProject("src/dynamicImportCompat", cmdLineOptions); +task("dynamicImportCompat", buildDynamicImportCompat); const buildServerMain = () => buildProject("src/tsserver", cmdLineOptions); -const buildServer = series(buildServerWeb, buildServerMain); +const buildServer = series(buildDynamicImportCompat, buildServerMain); buildServer.displayName = "buildServer"; task("tsserver", series(preBuild, buildServer)); task("tsserver").description = "Builds the language server"; @@ -226,17 +226,17 @@ task("tsserver").flags = { " --built": "Compile using the built version of the compiler." }; -const cleanServerWeb = () => cleanProject("src/tsserverWeb"); +const cleanDynamicImportCompat = () => cleanProject("src/dynamicImportCompat"); const cleanServerMain = () => cleanProject("src/tsserver"); -const cleanServer = series(cleanServerWeb, cleanServerMain); +const cleanServer = series(cleanDynamicImportCompat, cleanServerMain); cleanServer.displayName = "cleanServer"; cleanTasks.push(cleanServer); task("clean-tsserver", cleanServer); task("clean-tsserver").description = "Cleans outputs for the language server"; -const watchServerWeb = () => watchProject("src/tsserverWeb", cmdLineOptions); +const watchDynamicImportCompat = () => watchProject("src/dynamicImportCompat", cmdLineOptions); const watchServer = () => watchProject("src/tsserver", cmdLineOptions); -task("watch-tsserver", series(preBuild, parallel(watchLib, watchDiagnostics, watchServerWeb, watchServer))); +task("watch-tsserver", series(preBuild, parallel(watchLib, watchDiagnostics, watchDynamicImportCompat, watchServer))); task("watch-tsserver").description = "Watch for changes and rebuild the language server only"; task("watch-tsserver").flags = { " --built": "Compile using the built version of the compiler." @@ -558,7 +558,7 @@ const produceLKG = async () => { "built/local/typescriptServices.js", "built/local/typescriptServices.d.ts", "built/local/tsserver.js", - "built/local/tsserverWeb.js", + "built/local/dynamicImportCompat.js", "built/local/typescript.js", "built/local/typescript.d.ts", "built/local/tsserverlibrary.js", diff --git a/scripts/produceLKG.ts b/scripts/produceLKG.ts index 45fd5bd17c706..bd23b5329bb2b 100644 --- a/scripts/produceLKG.ts +++ b/scripts/produceLKG.ts @@ -62,7 +62,7 @@ async function copyScriptOutputs() { await copyWithCopyright("cancellationToken.js"); await copyWithCopyright("tsc.release.js", "tsc.js"); await copyWithCopyright("tsserver.js"); - await copyWithCopyright("tsserverWeb.js"); + await copyWithCopyright("dynamicImportCompat.js"); await copyFromBuiltLocal("tsserverlibrary.js"); // copyright added by build await copyFromBuiltLocal("typescript.js"); // copyright added by build await copyFromBuiltLocal("typescriptServices.js"); // copyright added by build diff --git a/src/tsserverWeb/tsserverWeb.ts b/src/dynamicImportCompat/dynamicImportCompat.ts similarity index 100% rename from src/tsserverWeb/tsserverWeb.ts rename to src/dynamicImportCompat/dynamicImportCompat.ts diff --git a/src/tsserverWeb/tsconfig.json b/src/dynamicImportCompat/tsconfig.json similarity index 70% rename from src/tsserverWeb/tsconfig.json rename to src/dynamicImportCompat/tsconfig.json index 10d9cd1916865..1ae167524208f 100644 --- a/src/tsserverWeb/tsconfig.json +++ b/src/dynamicImportCompat/tsconfig.json @@ -8,9 +8,9 @@ "lib": ["esnext"], "declaration": false, "sourceMap": true, - "tsBuildInfoFile": "../../built/local/tsserverWeb.tsbuildinfo" + "tsBuildInfoFile": "../../built/local/dynamicImportCompat.tsbuildinfo" }, "files": [ - "tsserverWeb.ts", + "dynamicImportCompat.ts", ] } diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 36090b908d286..8f253820a1663 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -4082,7 +4082,7 @@ namespace ts.server { return; } - // Otherwise, load the plugin using `resolve` + // Otherwise, load the plugin using `require` project.endEnablePlugin(project.beginEnablePluginSync(pluginConfigEntry, searchPaths, pluginConfigOverrides)); } diff --git a/src/tsconfig.json b/src/tsconfig.json index a23eca9ac3472..ca929ee198ca1 100644 --- a/src/tsconfig.json +++ b/src/tsconfig.json @@ -9,7 +9,7 @@ { "path": "./watchGuard" }, { "path": "./debug" }, { "path": "./cancellationToken" }, - { "path": "./tsserverWeb" }, + { "path": "./dynamicImportCompat" }, { "path": "./testRunner" } ] } \ No newline at end of file diff --git a/src/webServer/webServer.ts b/src/webServer/webServer.ts index 8b2a4be061dd7..0cad87dd97b67 100644 --- a/src/webServer/webServer.ts +++ b/src/webServer/webServer.ts @@ -118,7 +118,7 @@ namespace ts.server { if (typeof importScripts === "function") { try { // NOTE: importScripts is synchronous - importScripts("tsserverWeb.js"); + importScripts("dynamicImportCompat.js"); } catch { // ignored From 186f97ed3b3ba793ec356174d91d54a321513a52 Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Mon, 13 Jun 2022 16:59:57 -0700 Subject: [PATCH 12/12] Additional PR feedback --- src/harness/util.ts | 16 +++ src/server/editorServices.ts | 4 +- .../unittests/tsserver/webServer.ts | 101 ++++++++++++++++-- 3 files changed, 112 insertions(+), 9 deletions(-) diff --git a/src/harness/util.ts b/src/harness/util.ts index 6e0ab3cfb7df3..65e0e2fbe3d51 100644 --- a/src/harness/util.ts +++ b/src/harness/util.ts @@ -109,4 +109,20 @@ namespace Utils { value === undefined ? "undefined" : JSON.stringify(value); } + + export interface Deferred { + resolve: (value: T | PromiseLike) => void; + reject: (reason: unknown) => void; + promise: Promise; + } + + export function defer(): Deferred { + let resolve!: (value: T | PromiseLike) => void; + let reject!: (reason: unknown) => void; + const promise = new Promise((_resolve, _reject) => { + resolve = _resolve; + reject = _reject; + }); + return { resolve, reject, promise }; + } } \ No newline at end of file diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 8f253820a1663..15c457a923252 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -4154,7 +4154,7 @@ namespace ts.server { // Await all pending plugin imports. This ensures all requested plugin modules are fully loaded // prior to patching the language service, and that any promise rejections are observed. const results = await Promise.all(promises); - if (project.isClosed() && (!(project instanceof ConfiguredProject) || !project.hasOpenRef())) { + if (project.isClosed()) { // project is not alive, so don't enable plugins. return; } @@ -4164,7 +4164,7 @@ namespace ts.server { } // Plugins may have modified external files, so mark the project as dirty. - project.markAsDirty(); + this.delayUpdateProjectGraph(project); } configurePlugin(args: protocol.ConfigurePluginRequestArguments) { diff --git a/src/testRunner/unittests/tsserver/webServer.ts b/src/testRunner/unittests/tsserver/webServer.ts index 701b092dec705..d9f09eb4d4b2d 100644 --- a/src/testRunner/unittests/tsserver/webServer.ts +++ b/src/testRunner/unittests/tsserver/webServer.ts @@ -198,14 +198,12 @@ namespace ts.projectSystem { }); it("plugins evaluation in correct order even if imports resolve out of order", async () => { - let resolvePluginA!: () => void; - let resolvePluginB!: () => void; - const pluginAPromise = new Promise(_resolve => resolvePluginA = _resolve); - const pluginBPromise = new Promise(_resolve => resolvePluginB = _resolve); + const pluginADeferred = Utils.defer(); + const pluginBDeferred = Utils.defer(); const log: string[] = []; const importServicePlugin = async (_root: string, moduleName: string): Promise => { log.push(`request import ${moduleName}`); - const promise = moduleName === "plugin-a" ? pluginAPromise : pluginBPromise; + const promise = moduleName === "plugin-a" ? pluginADeferred.promise : pluginBDeferred.promise; await promise; log.push(`fulfill import ${moduleName}`); return { @@ -226,8 +224,8 @@ namespace ts.projectSystem { await Promise.resolve(); // resolve imports out of order - resolvePluginB(); - resolvePluginA(); + pluginBDeferred.resolve(); + pluginADeferred.resolve(); // wait for load to complete await projectService.waitForPendingPlugins(); @@ -267,6 +265,95 @@ namespace ts.projectSystem { } }]); }); + + it("adds external files", async () => { + const pluginAShouldLoad = Utils.defer(); + const pluginAExternalFilesRequested = Utils.defer(); + + const importServicePlugin = async (_root: string, _moduleName: string): Promise => { + // wait until the initial external files are requested from the project service. + await pluginAShouldLoad.promise; + + return { + module: (() => ({ + create: info => info.languageService, + getExternalFiles: () => { + // signal that external files have been requested by the project service. + pluginAExternalFilesRequested.resolve(); + return ["external.txt"]; + } + })) as server.PluginModuleFactory, + error: undefined + }; + }; + + const { session } = setup(/*logLevel*/ undefined, { globalPlugins: ["plugin-a"] }, importServicePlugin); + const projectService = session.getProjectService(); + + session.executeCommand({ seq: 1, type: "request", command: protocol.CommandTypes.Open, arguments: { file: "^memfs:/foo.ts", content: "" } }); + + const project = projectService.inferredProjects[0]; + + // get the external files we know about before plugins are loaded + const initialExternalFiles = project.getExternalFiles(); + + // we've ready the initial set of external files, allow the plugin to continue loading. + pluginAShouldLoad.resolve(); + + // wait for plugins + await projectService.waitForPendingPlugins(); + + // wait for the plugin's external files to be requested + await pluginAExternalFilesRequested.promise; + + // get the external files we know aobut after plugins are loaded + const pluginExternalFiles = project.getExternalFiles(); + + expect(initialExternalFiles).to.deep.equal([]); + expect(pluginExternalFiles).to.deep.equal(["external.txt"]); + }); + + it("project is closed before plugins are loaded", async () => { + const pluginALoaded = Utils.defer(); + const projectClosed = Utils.defer(); + const importServicePlugin = async (_root: string, _moduleName: string): Promise => { + // mark that the plugin has started loading + pluginALoaded.resolve(); + + // wait until after a project close has been requested to continue + await projectClosed.promise; + return { + module: (() => ({ create: info => info.languageService })) as server.PluginModuleFactory, + error: undefined + }; + }; + + const { session, getMessages } = setup(/*logLevel*/ undefined, { globalPlugins: ["plugin-a"] }, importServicePlugin); + const projectService = session.getProjectService(); + + session.executeCommand({ seq: 1, type: "request", command: protocol.CommandTypes.Open, arguments: { file: "^memfs:/foo.ts", content: "" } }); + + // wait for the plugin to start loading + await pluginALoaded.promise; + + // close the project + session.executeCommand({ seq: 2, type: "request", command: protocol.CommandTypes.Close, arguments: { file: "^memfs:/foo.ts" } }); + + // continue loading the plugin + projectClosed.resolve(); + + await projectService.waitForPendingPlugins(); + + // the project was closed before plugins were ready. no project update should have been requested + expect(getMessages()).not.to.deep.equal([{ + seq: 0, + type: "event", + event: "projectsUpdatedInBackground", + body: { + openFiles: ["^memfs:/foo.ts"] + } + }]); + }); }); }); }