Ensure dynamically imported plugins are loaded in the correct order

rbuckton · rbuckton · commit 0c78b837a1d3 · 2022-05-24T18:52:56.000-07:00
diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts
@@ -802,6 +802,9 @@ namespace ts.server {
 
         private performanceEventHandler?: PerformanceEventHandler;
 
+        private pendingPluginEnablements?: ESMap<Project, Promise<BeginEnablePluginResult>[]>;
+        private currentPluginEnablementPromise?: Promise<void>;
+
         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<any> | 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<BeginEnablePluginResult>[]][]) {
+            // 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<BeginEnablePluginResult>[]) {
+            // 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
@@ -101,6 +101,14 @@ namespace ts.server {
 
     export type PluginModuleFactory = (mod: { typescript: typeof ts }) => PluginModule;
 
+    /* @internal */
+    export interface BeginEnablePluginResult {
+        pluginConfigEntry: PluginImport;
+        pluginConfigOverrides: Map<any> | 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<string> | undefined;
         private missingFilesMap: ESMap<Path, FileWatcher> | 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<any> | undefined): Promise<void> {
+        protected enableGlobalPlugins(options: CompilerOptions, pluginConfigOverrides: Map<any> | 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,58 @@ 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<any> | undefined): Promise<void> {
-            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<any> | undefined): BeginEnablePluginResult {
+            Debug.assertIsDefined(this.projectService.host.require);
 
-            const log = (message: string) => this.projectService.logger.info(message);
             let errorLogs: string[] | undefined;
-            const logError = (message: string) => {
-                (errorLogs || (errorLogs = [])).push(message);
-            };
+            const log = (message: string) => this.projectService.logger.info(message);
+            const logError = (message: string) => { (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 };
+        }
+
+        /**
+         * 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<any> | undefined): Promise<BeginEnablePluginResult> {
+            Debug.assertIsDefined(this.projectService.host.importServicePlugin);
 
-            let resolvedModule: any | undefined;
-            if (this.projectService.host.importServicePlugin) {
-                for (const searchPath of searchPaths) {
+            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 +1645,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<any> | undefined): void {
+            this.projectService.requestEnablePlugin(this, pluginConfigEntry, searchPaths, pluginConfigOverrides);
+        }
+
         private enableProxy(pluginModuleFactory: PluginModuleFactory, configEntry: PluginImport) {
             try {
                 if (typeof pluginModuleFactory !== "function") {
@@ -2289,10 +2317,10 @@ namespace ts.server {
         }
 
         /*@internal*/
-        async enablePluginsWithOptions(options: CompilerOptions, pluginConfigOverrides: ESMap<string, any> | undefined): Promise<void> {
+        enablePluginsWithOptions(options: CompilerOptions, pluginConfigOverrides: ESMap<string, any> | 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 +2338,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
@@ -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
@@ -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<ImportPluginResult> => {
                 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
@@ -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<ImportPluginResult>;
     }
 }
 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 {};
