From 30fb9fa57e409c14e2e17df0b2344d51c8a827e7 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Thu, 20 Apr 2023 16:49:47 -0700 Subject: [PATCH] Refactor plugin related code (#53942) --- src/compiler/sys.ts | 4 +- src/compiler/types.ts | 2 +- src/harness/harnessLanguageService.ts | 2 +- src/server/editorServices.ts | 51 +++++- src/server/project.ts | 173 +++++++----------- .../helpers/virtualFileSystemWithWatch.ts | 4 +- src/tsserver/nodeServer.ts | 11 -- .../reference/api/tsserverlibrary.d.ts | 8 +- 8 files changed, 118 insertions(+), 137 deletions(-) diff --git a/src/compiler/sys.ts b/src/compiler/sys.ts index 50e7076774c..ace8ed10678 100644 --- a/src/compiler/sys.ts +++ b/src/compiler/sys.ts @@ -28,6 +28,7 @@ import { matchesExclude, matchFiles, memoize, + ModuleImportResult, noop, normalizePath, normalizeSlashes, @@ -35,7 +36,6 @@ import { Path, perfLogger, PollingWatchKind, - RequireResult, resolveJSModule, some, startsWith, @@ -1428,7 +1428,7 @@ export interface System { base64decode?(input: string): string; base64encode?(input: string): string; /** @internal */ bufferFrom?(input: string, encoding?: string): Buffer; - /** @internal */ require?(baseDir: string, moduleName: string): RequireResult; + /** @internal */ require?(baseDir: string, moduleName: string): ModuleImportResult; // For testing /** @internal */ now?(): Date; diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 18f985f8b69..c631f8cf79f 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -7371,7 +7371,7 @@ export interface ConfigFileSpecs { } /** @internal */ -export type RequireResult = +export type ModuleImportResult = | { module: T, modulePath?: string, error: undefined } | { module: undefined, modulePath?: undefined, error: { stack?: string, message?: string } }; diff --git a/src/harness/harnessLanguageService.ts b/src/harness/harnessLanguageService.ts index a8c854325a2..d8b7e5eb9a4 100644 --- a/src/harness/harnessLanguageService.ts +++ b/src/harness/harnessLanguageService.ts @@ -885,7 +885,7 @@ class SessionServerHost implements ts.server.ServerHost, ts.server.Logger { return mockHash(s); } - require(_initialDir: string, _moduleName: string): ts.RequireResult { + require(_initialDir: string, _moduleName: string): ts.ModuleImportResult { switch (_moduleName) { // Adds to the Quick Info a fixed string and a string from the config file // and replaces the first display part diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 09975236e53..4fb5020748c 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -962,7 +962,7 @@ export class ProjectService { public readonly globalPlugins: readonly string[]; public readonly pluginProbeLocations: readonly string[]; public readonly allowLocalPluginLoads: boolean; - private currentPluginConfigOverrides: Map | undefined; + /** @internal */ currentPluginConfigOverrides: Map | undefined; public readonly typesMapLocation: string | undefined; @@ -2189,7 +2189,6 @@ export class ProjectService { /*lastFileExceededProgramSize*/ this.getFilenameForExceededTotalSizeLimitForNonTsFiles(projectFileName, compilerOptions, files, externalFilePropertyReader), options.compileOnSave === undefined ? true : options.compileOnSave, /*projectFilePath*/ undefined, - this.currentPluginConfigOverrides, watchOptionsAndErrors?.watchOptions ); project.setProjectErrors(watchOptionsAndErrors?.errors); @@ -2354,7 +2353,7 @@ export class ProjectService { project.enableLanguageService(); this.watchWildcards(configFilename, configFileExistenceInfo, project); } - project.enablePluginsWithOptions(compilerOptions, this.currentPluginConfigOverrides); + project.enablePluginsWithOptions(compilerOptions); const filesToAdd = parsedCommandLine.fileNames.concat(project.getExternalFiles()); this.updateRootAndOptionsOfNonInferredProject(project, filesToAdd, fileNamePropertyReader, compilerOptions, parsedCommandLine.typeAcquisition!, parsedCommandLine.compileOnSave, parsedCommandLine.watchOptions); tracing?.pop(); @@ -2737,7 +2736,7 @@ export class ProjectService { typeAcquisition = this.typeAcquisitionForInferredProjects; } watchOptionsAndErrors = watchOptionsAndErrors || undefined; - const project = new InferredProject(this, this.documentRegistry, compilerOptions, watchOptionsAndErrors?.watchOptions, projectRootPath, currentDirectory, this.currentPluginConfigOverrides, typeAcquisition); + const project = new InferredProject(this, this.documentRegistry, compilerOptions, watchOptionsAndErrors?.watchOptions, projectRootPath, currentDirectory, typeAcquisition); project.setProjectErrors(watchOptionsAndErrors?.errors); if (isSingleInferredProject) { this.inferredProjects.unshift(project); @@ -4242,8 +4241,11 @@ export class ProjectService { return false; } - /** @internal */ - requestEnablePlugin(project: Project, pluginConfigEntry: PluginImport, searchPaths: string[], pluginConfigOverrides: Map | undefined) { + /** + * Performs the initial steps of enabling a plugin by finding and instantiating the module for a plugin either asynchronously or synchronously + * @internal + */ + requestEnablePlugin(project: Project, pluginConfigEntry: PluginImport, searchPaths: string[]) { if (!this.host.importPlugin && !this.host.require) { this.logger.info("Plugins were requested but not running in environment that supports 'require'. Nothing will be loaded"); return; @@ -4257,7 +4259,12 @@ export class ProjectService { // If the host supports dynamic import, begin enabling the plugin asynchronously. if (this.host.importPlugin) { - const importPromise = project.beginEnablePluginAsync(pluginConfigEntry, searchPaths, pluginConfigOverrides); + const importPromise = Project.importServicePluginAsync( + pluginConfigEntry, + searchPaths, + this.host, + s => this.logger.info(s), + ) as Promise; this.pendingPluginEnablements ??= new Map(); let promises = this.pendingPluginEnablements.get(project); if (!promises) this.pendingPluginEnablements.set(project, promises = []); @@ -4266,7 +4273,33 @@ export class ProjectService { } // Otherwise, load the plugin using `require` - project.endEnablePlugin(project.beginEnablePluginSync(pluginConfigEntry, searchPaths, pluginConfigOverrides)); + this.endEnablePlugin(project, Project.importServicePluginSync( + pluginConfigEntry, + searchPaths, + this.host, + s => this.logger.info(s), + )); + } + + /** + * Performs the remaining steps of enabling a plugin after its module has been instantiated. + * @internal + */ + private endEnablePlugin(project: Project, { pluginConfigEntry, resolvedModule, errorLogs }: BeginEnablePluginResult) { + if (resolvedModule) { + const configurationOverride = this.currentPluginConfigOverrides?.get(pluginConfigEntry.name); + if (configurationOverride) { + // Preserve the name property since it's immutable + const pluginName = pluginConfigEntry.name; + pluginConfigEntry = configurationOverride; + pluginConfigEntry.name = pluginName; + } + project.enableProxy(resolvedModule, pluginConfigEntry); + } + else { + forEach(errorLogs, message => this.logger.info(message)); + this.logger.info(`Couldn't find ${pluginConfigEntry.name}`); + } } /** @internal */ @@ -4345,7 +4378,7 @@ export class ProjectService { } for (const result of results) { - project.endEnablePlugin(result); + this.endEnablePlugin(project, result); } // Plugins may have modified external files, so mark the project as dirty. diff --git a/src/server/project.ts b/src/server/project.ts index f0fdc305b02..d7f2c4b3c50 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -38,7 +38,6 @@ import { FileWatcherCallback, FileWatcherEventKind, filter, - firstDefined, flatMap, forEach, forEachEntry, @@ -253,13 +252,15 @@ export interface PluginModuleWithName { export type PluginModuleFactory = (mod: { typescript: typeof ts }) => PluginModule; /** @internal */ -export interface BeginEnablePluginResult { +export interface PluginImportResult { pluginConfigEntry: PluginImport; - pluginConfigOverrides: Map | undefined; - resolvedModule: PluginModuleFactory | undefined; + resolvedModule: T | undefined; errorLogs: string[] | undefined; } +/** @internal */ +export type BeginEnablePluginResult = PluginImportResult; + /** * 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) @@ -398,36 +399,62 @@ export abstract class Project implements LanguageServiceHost, ModuleResolutionHo return hasOneOrMoreJsAndNoTsFiles(this); } - public static resolveModule(moduleName: string, initialDir: string, host: ServerHost, log: (message: string) => void, logErrors?: (message: string) => void): {} | undefined { - const resolvedPath = normalizeSlashes(host.resolvePath(combinePaths(initialDir, "node_modules"))); - log(`Loading ${moduleName} from ${initialDir} (resolved to ${resolvedPath})`); - const result = host.require!(resolvedPath, moduleName); // TODO: GH#18217 - if (result.error) { - const err = result.error.stack || result.error.message || JSON.stringify(result.error); - (logErrors || log)(`Failed to load module '${moduleName}' from ${resolvedPath}: ${err}`); - return undefined; - } - return result.module; + public static resolveModule(moduleName: string, initialDir: string, host: ServerHost, log: (message: string) => void): {} | undefined { + return Project.importServicePluginSync({ name: moduleName }, [initialDir], host, log).resolvedModule; } /** @internal */ - public static async importServicePluginAsync(moduleName: string, initialDir: string, host: ServerHost, log: (message: string) => void, logErrors?: (message: string) => void): Promise<{} | undefined> { - Debug.assertIsDefined(host.importPlugin); - const resolvedPath = combinePaths(initialDir, "node_modules"); - log(`Dynamically importing ${moduleName} from ${initialDir} (resolved to ${resolvedPath})`); - let result: ModuleImportResult; - try { - result = await host.importPlugin(resolvedPath, moduleName); - } - catch (e) { - result = { module: undefined, error: e }; - } - if (result.error) { + public static importServicePluginSync( + pluginConfigEntry: PluginImport, + searchPaths: string[], + host: ServerHost, + log: (message: string) => void, + ): PluginImportResult { + Debug.assertIsDefined(host.require); + let errorLogs: string[] | undefined; + let resolvedModule: T | undefined; + for (const initialDir of searchPaths) { + const resolvedPath = normalizeSlashes(host.resolvePath(combinePaths(initialDir, "node_modules"))); + log(`Loading ${pluginConfigEntry.name} from ${initialDir} (resolved to ${resolvedPath})`); + const result = host.require(resolvedPath, pluginConfigEntry.name); // TODO: GH#18217 + if (!result.error) { + resolvedModule = result.module as T; + break; + } 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; + (errorLogs ??= []).push(`Failed to load module '${pluginConfigEntry.name}' from ${resolvedPath}: ${err}`); } - return result.module; + return { pluginConfigEntry, resolvedModule, errorLogs }; + } + + /** @internal */ + public static async importServicePluginAsync( + pluginConfigEntry: PluginImport, + searchPaths: string[], + host: ServerHost, + log: (message: string) => void, + ): Promise> { + Debug.assertIsDefined(host.importPlugin); + let errorLogs: string[] | undefined; + let resolvedModule: T | undefined; + for (const initialDir of searchPaths) { + const resolvedPath = combinePaths(initialDir, "node_modules"); + log(`Dynamically importing ${pluginConfigEntry.name} from ${initialDir} (resolved to ${resolvedPath})`); + let result: ModuleImportResult; + try { + result = await host.importPlugin(resolvedPath, pluginConfigEntry.name); + } + catch (e) { + result = { module: undefined, error: e }; + } + if (!result.error) { + resolvedModule = result.module as T; + break; + } + const err = result.error.stack || result.error.message || JSON.stringify(result.error); + (errorLogs ??= []).push(`Failed to dynamically import module '${pluginConfigEntry.name}' from ${resolvedPath}: ${err}`); + } + return { pluginConfigEntry, resolvedModule, errorLogs }; } /** @internal */ @@ -1807,7 +1834,7 @@ export abstract class Project implements LanguageServiceHost, ModuleResolutionHo ]; } - protected enableGlobalPlugins(options: CompilerOptions, pluginConfigOverrides: Map | undefined): void { + protected enableGlobalPlugins(options: CompilerOptions): void { if (!this.projectService.globalPlugins.length) return; const host = this.projectService.host; @@ -1828,80 +1855,16 @@ export abstract class Project implements LanguageServiceHost, ModuleResolutionHo // 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); + this.enablePlugin({ name: globalPluginName, global: true } as PluginImport, searchPaths); } } - /** - * 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); - - let errorLogs: string[] | undefined; - 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 }; + protected enablePlugin(pluginConfigEntry: PluginImport, searchPaths: string[]): void { + this.projectService.requestEnablePlugin(this, pluginConfigEntry, 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.importPlugin); - - 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) { - resolvedModule = await Project.importServicePluginAsync(pluginConfigEntry.name, searchPath, this.projectService.host, log, logError) as PluginModuleFactory | undefined; - if (resolvedModule !== undefined) { - break; - } - } - 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) { - // Preserve the name property since it's immutable - const pluginName = pluginConfigEntry.name; - pluginConfigEntry = configurationOverride; - pluginConfigEntry.name = pluginName; - } - - this.enableProxy(resolvedModule, pluginConfigEntry); - } - else { - 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) { + /** @internal */ + enableProxy(pluginModuleFactory: PluginModuleFactory, configEntry: PluginImport) { try { if (typeof pluginModuleFactory !== "function") { this.projectService.logger.info(`Skipped loading plugin ${configEntry.name} because it did not expose a proper factory function`); @@ -2188,7 +2151,6 @@ export class InferredProject extends Project { watchOptions: WatchOptions | undefined, projectRootPath: NormalizedPath | undefined, currentDirectory: string, - pluginConfigOverrides: Map | undefined, typeAcquisition: TypeAcquisition | undefined) { super(projectService.newInferredProjectName(), ProjectKind.Inferred, @@ -2207,7 +2169,7 @@ export class InferredProject extends Project { if (!projectRootPath && !projectService.useSingleInferredProject) { this.canonicalCurrentDirectory = projectService.toCanonicalFileName(this.currentDirectory); } - this.enableGlobalPlugins(this.getCompilerOptions(), pluginConfigOverrides); + this.enableGlobalPlugins(this.getCompilerOptions()); } override addRoot(info: ScriptInfo) { @@ -2729,7 +2691,7 @@ export class ConfiguredProject extends Project { } /** @internal */ - enablePluginsWithOptions(options: CompilerOptions, pluginConfigOverrides: Map | undefined): void { + enablePluginsWithOptions(options: CompilerOptions): void { this.plugins.length = 0; if (!options.plugins?.length && !this.projectService.globalPlugins.length) return; const host = this.projectService.host; @@ -2748,11 +2710,11 @@ export class ConfiguredProject extends Project { // Enable tsconfig-specified plugins if (options.plugins) { for (const pluginConfigEntry of options.plugins) { - this.enablePlugin(pluginConfigEntry, searchPaths, pluginConfigOverrides); + this.enablePlugin(pluginConfigEntry, searchPaths); } } - return this.enableGlobalPlugins(options, pluginConfigOverrides); + return this.enableGlobalPlugins(options); } /** @@ -2884,7 +2846,6 @@ export class ExternalProject extends Project { lastFileExceededProgramSize: string | undefined, public override compileOnSaveEnabled: boolean, projectFilePath?: string, - pluginConfigOverrides?: Map, watchOptions?: WatchOptions) { super(externalProjectName, ProjectKind.External, @@ -2897,7 +2858,7 @@ export class ExternalProject extends Project { watchOptions, projectService.host, getDirectoryPath(projectFilePath || normalizeSlashes(externalProjectName))); - this.enableGlobalPlugins(this.getCompilerOptions(), pluginConfigOverrides); + this.enableGlobalPlugins(this.getCompilerOptions()); } override updateGraph() { diff --git a/src/testRunner/unittests/helpers/virtualFileSystemWithWatch.ts b/src/testRunner/unittests/helpers/virtualFileSystemWithWatch.ts index 6ee669a78d0..82a4094f251 100644 --- a/src/testRunner/unittests/helpers/virtualFileSystemWithWatch.ts +++ b/src/testRunner/unittests/helpers/virtualFileSystemWithWatch.ts @@ -31,13 +31,13 @@ import { isString, mapDefined, matchFiles, + ModuleImportResult, ModuleResolutionHost, MultiMap, noop, patchWriteFileEnsuringDirectory, Path, PollingInterval, - RequireResult, server, SortedArray, sys, @@ -292,7 +292,7 @@ export class TestServerHost implements server.ServerHost, FormatDiagnosticsHost, private readonly environmentVariables?: Map; private readonly executingFilePath: string; private readonly currentDirectory: string; - public require: ((initialPath: string, moduleName: string) => RequireResult) | undefined; + public require: ((initialPath: string, moduleName: string) => ModuleImportResult) | undefined; public storeFilesChangingSignatureDuringEmit = true; watchFile: HostWatchFile; private inodeWatching: boolean | undefined; diff --git a/src/tsserver/nodeServer.ts b/src/tsserver/nodeServer.ts index ce3a0c2d682..1154b4d5ec0 100644 --- a/src/tsserver/nodeServer.ts +++ b/src/tsserver/nodeServer.ts @@ -20,7 +20,6 @@ import { normalizePath, normalizeSlashes, perfLogger, - resolveJSModule, SortedReadonlyArray, startTracing, stripQuotes, @@ -57,7 +56,6 @@ import { ITypingsInstaller, Logger, LogLevel, - ModuleImportResult, Msg, nowString, nullCancellationToken, @@ -381,15 +379,6 @@ export function initializeNodeSystem(): StartInput { sys.gc = () => global.gc?.(); } - sys.require = (initialDir: string, moduleName: string): ModuleImportResult => { - try { - return { module: require(resolveJSModule(moduleName, initialDir, sys)), error: undefined }; - } - catch (error) { - return { module: undefined, error }; - } - }; - let cancellationToken: ServerCancellationToken; try { const factory = require("./cancellationToken"); diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index cf6c3c7018d..90833b31015 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -3254,7 +3254,7 @@ declare namespace ts { private readonly cancellationToken; isNonTsProject(): boolean; isJsOnlyProject(): boolean; - static resolveModule(moduleName: string, initialDir: string, host: ServerHost, log: (message: string) => void, logErrors?: (message: string) => void): {} | undefined; + static resolveModule(moduleName: string, initialDir: string, host: ServerHost, log: (message: string) => void): {} | undefined; isKnownTypesPackageName(name: string): boolean; installPackage(options: InstallPackageOptions): Promise; private get typingsCache(); @@ -3341,9 +3341,8 @@ declare namespace ts { setTypeAcquisition(newTypeAcquisition: TypeAcquisition | undefined): void; getTypeAcquisition(): ts.TypeAcquisition; protected removeRoot(info: ScriptInfo): void; - protected enableGlobalPlugins(options: CompilerOptions, pluginConfigOverrides: Map | undefined): void; - protected enablePlugin(pluginConfigEntry: PluginImport, searchPaths: string[], pluginConfigOverrides: Map | undefined): void; - private enableProxy; + protected enableGlobalPlugins(options: CompilerOptions): void; + protected enablePlugin(pluginConfigEntry: PluginImport, searchPaths: string[]): void; /** Starts a new check for diagnostics. Call this if some file has updated that would cause diagnostics to be changed. */ refreshDiagnostics(): void; } @@ -3644,7 +3643,6 @@ declare namespace ts { readonly globalPlugins: readonly string[]; readonly pluginProbeLocations: readonly string[]; readonly allowLocalPluginLoads: boolean; - private currentPluginConfigOverrides; readonly typesMapLocation: string | undefined; readonly serverMode: LanguageServiceMode; /** Tracks projects that we have already sent telemetry for. */