From 0e722f0de513d6141b1718c29aa0b30b9e9974ce Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Wed, 19 May 2021 16:33:36 -0700 Subject: [PATCH 01/11] Add cache invalidation for node_modules, config, and preferences changes --- src/compiler/moduleSpecifiers.ts | 49 +++++++-- src/compiler/types.ts | 6 +- src/server/editorServices.ts | 13 ++- src/server/project.ts | 14 ++- src/server/watchType.ts | 6 +- src/services/utilities.ts | 54 ++++++++-- .../tsserver/moduleSpecifierCache.ts | 100 ++++++++++++++---- 7 files changed, 195 insertions(+), 47 deletions(-) diff --git a/src/compiler/moduleSpecifiers.ts b/src/compiler/moduleSpecifiers.ts index 45b31154344c8..9dbf23ee02c3c 100644 --- a/src/compiler/moduleSpecifiers.ts +++ b/src/compiler/moduleSpecifiers.ts @@ -106,9 +106,19 @@ namespace ts.moduleSpecifiers { if (!moduleSourceFile) { return []; } - const modulePaths = getAllModulePaths(importingSourceFile.path, moduleSourceFile.originalFileName, host); - const preferences = getPreferences(userPreferences, compilerOptions, importingSourceFile); + const cache = host.getModuleSpecifierCache?.(); + const cached = cache?.get(importingSourceFile.path, moduleSourceFile.path); + let modulePaths; + if (typeof cached === "object") { + if (cached.moduleSpecifiers) return cached.moduleSpecifiers; + modulePaths = cached.modulePaths; + } + else { + modulePaths = getAllModulePathsWorker(importingSourceFile.path, moduleSourceFile.originalFileName, host); + } + + const preferences = getPreferences(userPreferences, compilerOptions, importingSourceFile); const existingSpecifier = forEach(modulePaths, modulePath => forEach( host.getFileIncludeReasons().get(toPath(modulePath.path, host.getCurrentDirectory(), info.getCanonicalFileName)), reason => { @@ -120,7 +130,11 @@ namespace ts.moduleSpecifiers { undefined; } )); - if (existingSpecifier) return [existingSpecifier]; + if (existingSpecifier) { + const moduleSpecifiers = [existingSpecifier]; + cache?.set(importingSourceFile.path, moduleSourceFile.path, { modulePaths, moduleSpecifiers }); + return moduleSpecifiers; + } const importedFileIsInNodeModules = some(modulePaths, p => p.isInNodeModules); @@ -138,6 +152,7 @@ namespace ts.moduleSpecifiers { if (specifier && modulePath.isRedirect) { // If we got a specifier for a redirect, it was a bare package specifier (e.g. "@foo/bar", // not "@foo/bar/path/to/file"). No other specifier will be this good, so stop looking. + cache?.set(importingSourceFile.path, moduleSourceFile.path, { modulePaths, moduleSpecifiers: nodeModulesSpecifiers! }); return nodeModulesSpecifiers!; } @@ -161,9 +176,11 @@ namespace ts.moduleSpecifiers { } } - return pathsSpecifiers?.length ? pathsSpecifiers : + const moduleSpecifiers = pathsSpecifiers?.length ? pathsSpecifiers : nodeModulesSpecifiers?.length ? nodeModulesSpecifiers : Debug.checkDefined(relativeSpecifiers); + cache?.set(importingSourceFile.path, moduleSourceFile.path, { modulePaths, moduleSpecifiers }); + return moduleSpecifiers; } interface Info { @@ -332,13 +349,26 @@ namespace ts.moduleSpecifiers { * Looks for existing imports that use symlinks to this module. * Symlinks will be returned first so they are preferred over the real path. */ - function getAllModulePaths(importingFileName: Path, importedFileName: string, host: ModuleSpecifierResolutionHost): readonly ModulePath[] { + function getAllModulePaths( + importingFilePath: Path, + importedFileName: string, + host: ModuleSpecifierResolutionHost, + importedFilePath = toPath(importedFileName, host.getCurrentDirectory(), hostGetCanonicalFileName(host)) + ) { const cache = host.getModuleSpecifierCache?.(); - const getCanonicalFileName = hostGetCanonicalFileName(host); if (cache) { - const cached = cache.get(importingFileName, toPath(importedFileName, host.getCurrentDirectory(), getCanonicalFileName)); - if (typeof cached === "object") return cached; + const cached = cache.get(importingFilePath, importedFilePath); + if (typeof cached === "object") return cached.modulePaths; + } + const modulePaths = getAllModulePathsWorker(importingFilePath, importedFileName, host); + if (cache) { + cache.setModulePaths(importingFilePath, importedFilePath, modulePaths); } + return modulePaths; + } + + function getAllModulePathsWorker(importingFileName: Path, importedFileName: string, host: ModuleSpecifierResolutionHost): readonly ModulePath[] { + const getCanonicalFileName = hostGetCanonicalFileName(host); const allFileNames = new Map(); let importedFileFromNodeModules = false; forEachFileNameOfModule( @@ -384,9 +414,6 @@ namespace ts.moduleSpecifiers { sortedPaths.push(...remainingPaths); } - if (cache) { - cache.set(importingFileName, toPath(importedFileName, host.getCurrentDirectory(), getCanonicalFileName), sortedPaths); - } return sortedPaths; } diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 430a2f2b20176..5c0a5fb9d2a19 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -8108,8 +8108,10 @@ namespace ts { /* @internal */ export interface ModuleSpecifierCache { - get(fromFileName: Path, toFileName: Path): boolean | readonly ModulePath[] | undefined; - set(fromFileName: Path, toFileName: Path, moduleSpecifiers: boolean | readonly ModulePath[]): void; + getModulePaths(fromFileName: Path, toFileName: Path): boolean | readonly ModulePath[] | undefined; + setModulePaths(fromFileName: Path, toFileName: Path, modulePaths: boolean | readonly ModulePath[]): void; + get(fromFileName: Path, toFileName: Path): { modulePaths: readonly ModulePath[], moduleSpecifiers?: readonly string[] } | boolean | undefined; + set(fromFileName: Path, toFileName: Path, cached: { modulePaths: readonly ModulePath[], moduleSpecifiers?: readonly string[] }): void; clear(): void; count(): number; } diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index ceebdc4020315..716bd47635756 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -2945,7 +2945,13 @@ namespace ts.server { this.logger.info("Format host information updated"); } if (args.preferences) { - const { lazyConfiguredProjectsFromExternalProject, includePackageJsonAutoImports } = this.hostConfiguration.preferences; + const { + lazyConfiguredProjectsFromExternalProject, + includePackageJsonAutoImports, + importModuleSpecifierEnding, + importModuleSpecifierPreference, + } = this.hostConfiguration.preferences; + this.hostConfiguration.preferences = { ...this.hostConfiguration.preferences, ...args.preferences }; if (lazyConfiguredProjectsFromExternalProject && !this.hostConfiguration.preferences.lazyConfiguredProjectsFromExternalProject) { // Load configured projects for external projects that are pending reload @@ -2960,6 +2966,11 @@ namespace ts.server { if (includePackageJsonAutoImports !== args.preferences.includePackageJsonAutoImports) { this.invalidateProjectPackageJson(/*packageJsonPath*/ undefined); } + if (importModuleSpecifierEnding !== args.preferences.importModuleSpecifierEnding || importModuleSpecifierPreference !== args.preferences.importModuleSpecifierPreference) { + this.configuredProjects.forEach(p => p.getModuleSpecifierCache().clear()); + this.inferredProjects.forEach(p => p.getModuleSpecifierCache().clear()); + this.externalProjects.forEach(p => p.getModuleSpecifierCache().clear()); + } } if (args.extraFileExtensions) { this.hostConfiguration.extraFileExtensions = args.extraFileExtensions; diff --git a/src/server/project.ts b/src/server/project.ts index ef48ebf530bc2..6fd73cc0d8e4a 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -254,7 +254,7 @@ namespace ts.server { /*@internal*/ private changedFilesForExportMapCache: Set | undefined; /*@internal*/ - private moduleSpecifierCache = createModuleSpecifierCache(); + private moduleSpecifierCache = createModuleSpecifierCache(this); /*@internal*/ private symlinks: SymlinkCache | undefined; /*@internal*/ @@ -1389,6 +1389,7 @@ namespace ts.server { this.cachedUnresolvedImportsPerFile.clear(); this.lastCachedUnresolvedImportsList = undefined; this.resolutionCache.clear(); + this.moduleSpecifierCache.clear(); } this.markAsDirty(); } @@ -1730,6 +1731,17 @@ namespace ts.server { this.projectService.openFiles, (_, fileName) => this.projectService.tryGetDefaultProjectForFile(toNormalizedPath(fileName)) === this); } + + /*@internal*/ + watchNodeModulesDirectory(directoryPath: string, cb: DirectoryWatcherCallback) { + return this.projectService.watchFactory.watchDirectory( + directoryPath, + cb, + WatchDirectoryFlags.Recursive, + this.projectService.getWatchOptions(this), + WatchType.NodeModulesForClosedScriptInfo + ); + } } function getUnresolvedImports(program: Program, cachedUnresolvedImportsPerFile: ESMap): SortedReadonlyArray { diff --git a/src/server/watchType.ts b/src/server/watchType.ts index 8cc5014b0e57b..8a9d94dfb213b 100644 --- a/src/server/watchType.ts +++ b/src/server/watchType.ts @@ -8,7 +8,8 @@ namespace ts { MissingSourceMapFile: "Missing source map file", NoopConfigFileForInferredRoot: "Noop Config file for the inferred project root", MissingGeneratedFile: "Missing generated file", - PackageJsonFile: "package.json file for import suggestions" + PackageJsonFile: "package.json file for import suggestions", + NodeModulesForModuleSpecifierCache: "node_modules for module specifier cache invalidation", } WatchType.ClosedScriptInfo = "Closed Script info"; WatchType.ConfigFileForInferredRoot = "Config file for the inferred project root"; @@ -17,4 +18,5 @@ namespace ts { WatchType.NoopConfigFileForInferredRoot = "Noop Config file for the inferred project root"; WatchType.MissingGeneratedFile = "Missing generated file"; WatchType.PackageJsonFile = "package.json file for import suggestions"; -} \ No newline at end of file + WatchType.NodeModulesForModuleSpecifierCache = "node_modules for module specifier cache invalidation"; +} diff --git a/src/services/utilities.ts b/src/services/utilities.ts index dda10c00bece3..92330f159ade1 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -3273,23 +3273,61 @@ namespace ts { } } - export function createModuleSpecifierCache(): ModuleSpecifierCache { - let cache: ESMap | undefined; + export interface ModuleSpecifierResolutionCacheHost { + watchNodeModulesDirectory(directoryPath: string, cb: DirectoryWatcherCallback): FileWatcher; + } + + export function createModuleSpecifierCache(host: ModuleSpecifierResolutionCacheHost): ModuleSpecifierCache { + let containedNodeModulesWatchers: ESMap | undefined; + let cache: ESMap | undefined; let importingFileName: Path | undefined; const wrapped: ModuleSpecifierCache = { get(fromFileName, toFileName) { if (!cache || fromFileName !== importingFileName) return undefined; return cache.get(toFileName); }, - set(fromFileName, toFileName, moduleSpecifiers) { + set(fromFileName, toFileName, cached) { + if (cache && fromFileName !== importingFileName) { + this.clear(); + } + importingFileName = fromFileName; + (cache ||= new Map()).set(toFileName, cached); + + // If any module specifiers were generated based off paths in node_modules, + // a package.json file in that package was read and is an input to the cached. + // Instead of watching each individual package.json file, set up a wildcard + // directory watcher for any node_modules referenced and clear the cache when + // it sees any changes. + if (cached.moduleSpecifiers) { + for (const p of cached.modulePaths) { + if (p.isInNodeModules) { + const nodeModulesPath = p.path.substring(0, p.path.indexOf(nodeModulesPathPart) + nodeModulesPathPart.length); + if (!containedNodeModulesWatchers?.has(nodeModulesPath)) { + (containedNodeModulesWatchers ||= new Map()).set( + nodeModulesPath, + host.watchNodeModulesDirectory(nodeModulesPath, this.clear), + ); + } + } + } + } + }, + getModulePaths(fromFileName, toFileName) { + if (!cache || fromFileName !== importingFileName) return undefined; + const cached = cache.get(toFileName); + return typeof cached === "object" ? cached.modulePaths : cached; + }, + setModulePaths(fromFileName, toFileName, modulePaths) { if (cache && fromFileName !== importingFileName) { - cache.clear(); + this.clear(); } importingFileName = fromFileName; - (cache ||= new Map()).set(toFileName, moduleSpecifiers); + (cache ||= new Map()).set(toFileName, typeof modulePaths === "boolean" ? modulePaths : { modulePaths }); }, clear() { - cache = undefined; + containedNodeModulesWatchers?.forEach(watcher => watcher.close()); + cache?.clear(); + containedNodeModulesWatchers?.clear(); importingFileName = undefined; }, count() { @@ -3311,7 +3349,7 @@ namespace ts { moduleSpecifierCache: ModuleSpecifierCache | undefined, ): boolean { if (from === to) return false; - const cachedResult = moduleSpecifierCache?.get(from.path, to.path); + const cachedResult = moduleSpecifierCache?.getModulePaths(from.path, to.path); if (cachedResult !== undefined) { return !!cachedResult; } @@ -3334,7 +3372,7 @@ namespace ts { if (packageJsonFilter) { const isImportable = hasImportablePath && packageJsonFilter.allowsImportingSourceFile(to, moduleSpecifierResolutionHost); - moduleSpecifierCache?.set(from.path, to.path, isImportable); + moduleSpecifierCache?.setModulePaths(from.path, to.path, isImportable); return isImportable; } diff --git a/src/testRunner/unittests/tsserver/moduleSpecifierCache.ts b/src/testRunner/unittests/tsserver/moduleSpecifierCache.ts index f109cab9d1f6e..d094dfe72cffe 100644 --- a/src/testRunner/unittests/tsserver/moduleSpecifierCache.ts +++ b/src/testRunner/unittests/tsserver/moduleSpecifierCache.ts @@ -4,23 +4,27 @@ namespace ts.projectSystem { content: `{ "dependencies": { "mobx": "*" } }` }; const aTs: File = { - path: "/a.ts", + path: "/src/a.ts", content: "export const foo = 0;", }; const bTs: File = { - path: "/b.ts", + path: "/src/b.ts", content: "foo", }; + const cTs: File = { + path: "/src/c.ts", + content: "import ", + }; const bSymlink: SymLink = { - path: "/b-link.ts", + path: "/src/b-link.ts", symLink: "./b.ts", }; const tsconfig: File = { path: "/tsconfig.json", - content: "{}", + content: `{ "include": ["src"] }`, }; const ambientDeclaration: File = { - path: "/ambient.d.ts", + path: "/src/ambient.d.ts", content: "declare module 'ambient' {}" }; const mobxDts: File = { @@ -31,50 +35,102 @@ namespace ts.projectSystem { describe("unittests:: tsserver:: moduleSpecifierCache", () => { it("caches importability within a file", () => { const { moduleSpecifierCache } = setup(); - assert.isTrue(moduleSpecifierCache.get(bTs.path as Path, aTs.path as Path)); + assert.isTrue(moduleSpecifierCache.getModulePaths(bTs.path as Path, aTs.path as Path)); + }); + + it("caches module specifiers within a file", () => { + const { moduleSpecifierCache, triggerCompletions } = setup(); + // Completion at an import statement will calculate and cache module specifiers + triggerCompletions({ file: cTs.path, line: 1, offset: cTs.content.length + 1 }); + const mobxCache = moduleSpecifierCache.get(cTs.path as Path, mobxDts.path as Path); + assert.deepEqual(mobxCache, { + modulePaths: [{ + path: mobxDts.path, + isInNodeModules: true, + isRedirect: false + }], + moduleSpecifiers: ["mobx"] + }); + }); + + it("invalidates module specifiers when changes happen in contained node_modules directories", () => { + const { host, moduleSpecifierCache, triggerCompletions } = setup(); + // Completion at an import statement will calculate and cache module specifiers + triggerCompletions({ file: cTs.path, line: 1, offset: cTs.content.length + 1 }); + checkWatchedDirectories(host, ["/src", "/node_modules"], /*recursive*/ true); + host.writeFile("/node_modules/.staging/mobx-12345678/package.json", "{}"); + host.runQueuedTimeoutCallbacks(); + assert.equal(moduleSpecifierCache.count(), 0); }); it("does not invalidate the cache when new files are added", () => { const { host, moduleSpecifierCache } = setup(); host.writeFile("/src/a2.ts", aTs.content); host.runQueuedTimeoutCallbacks(); - assert.isTrue(moduleSpecifierCache.get(bTs.path as Path, aTs.path as Path)); + assert.isTrue(moduleSpecifierCache.getModulePaths(bTs.path as Path, aTs.path as Path)); }); it("invalidates the cache when symlinks are added or removed", () => { const { host, moduleSpecifierCache } = setup(); - host.renameFile(bSymlink.path, "/b-link2.ts"); + host.renameFile(bSymlink.path, "/src/b-link2.ts"); host.runQueuedTimeoutCallbacks(); assert.equal(moduleSpecifierCache.count(), 0); }); - it("invalidates the cache when package.json changes", () => { + it("invalidates the cache when local package.json changes", () => { const { host, moduleSpecifierCache } = setup(); host.writeFile("/package.json", `{}`); host.runQueuedTimeoutCallbacks(); - assert.isUndefined(moduleSpecifierCache.get(bTs.path as Path, aTs.path as Path)); + assert.equal(moduleSpecifierCache.count(), 0); + }); + + it("invalidates the cache when module resolution settings change", () => { + const { host, moduleSpecifierCache } = setup(); + host.writeFile(tsconfig.path, `{ "compilerOptions": { "moduleResolution": "classic" }, "include": ["src"] }`); + host.runQueuedTimeoutCallbacks(); + assert.equal(moduleSpecifierCache.count(), 0); + }); + + it("invalidates the cache when user preferences change", () => { + const { moduleSpecifierCache, session, triggerCompletions } = setup(); + executeSessionRequest(session, protocol.CommandTypes.Configure, { + preferences: { importModuleSpecifierPreference: "project-relative" }, + }); + assert.equal(moduleSpecifierCache.count(), 0); + + // Reset + triggerCompletions({ file: bTs.path, line: 1, offset: 3 }); + assert.equal(moduleSpecifierCache.count(), 1); + + // Test other affecting preference + executeSessionRequest(session, protocol.CommandTypes.Configure, { + preferences: { importModuleSpecifierEnding: "js" }, + }); + assert.equal(moduleSpecifierCache.count(), 0); }); }); function setup() { - const host = createServerHost([aTs, bTs, bSymlink, ambientDeclaration, tsconfig, packageJson, mobxDts]); + const host = createServerHost([aTs, bTs, cTs, bSymlink, ambientDeclaration, tsconfig, packageJson, mobxDts]); const session = createSession(host); - openFilesForSession([aTs, bTs], session); + openFilesForSession([aTs, bTs, cTs], session); const projectService = session.getProjectService(); const project = configuredProjectAt(projectService, 0); - triggerCompletions(); - return { host, project, projectService, moduleSpecifierCache: project.getModuleSpecifierCache(), triggerCompletions }; + executeSessionRequest(session, protocol.CommandTypes.Configure, { + preferences: { + includeCompletionsForImportStatements: true, + includeCompletionsForModuleExports: true, + includeCompletionsWithInsertText: true, + includeCompletionsWithSnippetText: true, + }, + }); + triggerCompletions({ file: bTs.path, line: 1, offset: 3 }); + + return { host, project, projectService, session, moduleSpecifierCache: project.getModuleSpecifierCache(), triggerCompletions }; - function triggerCompletions() { - const requestLocation: protocol.FileLocationRequestArgs = { - file: bTs.path, - line: 1, - offset: 3, - }; + function triggerCompletions(requestLocation: protocol.FileLocationRequestArgs) { executeSessionRequest(session, protocol.CommandTypes.CompletionInfo, { ...requestLocation, - includeExternalModuleExports: true, - prefix: "foo", }); } } From 80632994cb20677917aab87b321c971bc289c629 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Fri, 4 Jun 2021 09:56:46 -0500 Subject: [PATCH 02/11] Share watches with closed script info --- src/server/editorServices.ts | 114 +++++++++++++++++++++++++---------- src/server/project.ts | 11 +--- src/services/utilities.ts | 7 ++- 3 files changed, 88 insertions(+), 44 deletions(-) diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 716bd47635756..5451b898358ab 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -589,8 +589,11 @@ namespace ts.server { ); } - interface ScriptInfoInNodeModulesWatcher extends FileWatcher { - refCount: number; + interface NodeModulesWatcher extends FileWatcher { + /** How many watchers of this directory were for closed ScriptInfo */ + refreshScriptInfoRefCount: number; + /** List of project names whose module specifier cache should be cleared when package.jsons change */ + affectedModuleSpecifierCacheProjects: Set; } function getDetailWatchInfo(watchType: WatchType, project: Project | NormalizedPath | undefined) { @@ -671,7 +674,7 @@ namespace ts.server { */ /*@internal*/ readonly filenameToScriptInfo = new Map(); - private readonly scriptInfoInNodeModulesWatchers = new Map(); + private readonly nodeModulesWatchers = new Map(); /** * Contains all the deleted script info's version information so that * it does not reset when creating script info again @@ -2617,36 +2620,40 @@ namespace ts.server { } } - private watchClosedScriptInfoInNodeModules(dir: Path): ScriptInfoInNodeModulesWatcher { - // Watch only directory - const existing = this.scriptInfoInNodeModulesWatchers.get(dir); - if (existing) { - existing.refCount++; - return existing; - } - - const watchDir = dir + "/node_modules" as Path; + private createNodeModulesWatcher(dir: Path, affectedModuleSpecifierCacheProject?: Project) { const watcher = this.watchFactory.watchDirectory( - watchDir, + dir, fileOrDirectory => { const fileOrDirectoryPath = removeIgnoredPath(this.toPath(fileOrDirectory)); if (!fileOrDirectoryPath) return; - // Has extension - Debug.assert(result.refCount > 0); - if (watchDir === fileOrDirectoryPath) { - this.refreshScriptInfosInDirectory(watchDir); + // Clear module specifier cache for any projects whose cache was affected by + // dependency package.jsons in this node_modules directory + const basename = getBaseFileName(fileOrDirectoryPath); + if (result.affectedModuleSpecifierCacheProjects.size && ( + basename === "package.json" || basename === "node_modules" + )) { + result.affectedModuleSpecifierCacheProjects.forEach(projectName => { + this.findProject(projectName)?.getModuleSpecifierCache()?.clear(); + }); } - else { - const info = this.getScriptInfoForPath(fileOrDirectoryPath); - if (info) { - if (isScriptInfoWatchedFromNodeModules(info)) { - this.refreshScriptInfo(info); - } + + // Refresh closed script info after an npm install + if (result.refreshScriptInfoRefCount) { + if (dir === fileOrDirectoryPath) { + this.refreshScriptInfosInDirectory(dir); } - // Folder - else if (!hasExtension(fileOrDirectoryPath)) { - this.refreshScriptInfosInDirectory(fileOrDirectoryPath); + else { + const info = this.getScriptInfoForPath(fileOrDirectoryPath); + if (info) { + if (isScriptInfoWatchedFromNodeModules(info)) { + this.refreshScriptInfo(info); + } + } + // Folder + else if (!hasExtension(fileOrDirectoryPath)) { + this.refreshScriptInfosInDirectory(fileOrDirectoryPath); + } } } }, @@ -2654,20 +2661,61 @@ namespace ts.server { this.hostConfiguration.watchOptions, WatchType.NodeModulesForClosedScriptInfo ); - const result: ScriptInfoInNodeModulesWatcher = { + const result: NodeModulesWatcher = { + refreshScriptInfoRefCount: affectedModuleSpecifierCacheProject ? 0 : 1, + affectedModuleSpecifierCacheProjects: new Set( + affectedModuleSpecifierCacheProject + ? [affectedModuleSpecifierCacheProject.getProjectName()] + : emptyArray), close: () => { - if (result.refCount === 1) { + watcher.close(); + this.nodeModulesWatchers.delete(dir); + }, + }; + this.nodeModulesWatchers.set(dir, result); + return result; + } + + /*@internal*/ + watchPackageJsonsInNodeModules(dir: Path, project: Project): FileWatcher { + const existing = this.nodeModulesWatchers.get(dir); + if (existing) { + existing.affectedModuleSpecifierCacheProjects.add(project.getProjectName()); + return existing; + } + + const watcher = this.createNodeModulesWatcher(dir, project); + return { + close: () => { + if (watcher.refreshScriptInfoRefCount === 0 && watcher.affectedModuleSpecifierCacheProjects.size === 1) { watcher.close(); - this.scriptInfoInNodeModulesWatchers.delete(dir); } else { - result.refCount--; + watcher.affectedModuleSpecifierCacheProjects.delete(project.getProjectName()); + } + }, + }; + } + + private watchClosedScriptInfoInNodeModules(dir: Path): FileWatcher { + const watchDir = dir + "/node_modules" as Path; + const existing = this.nodeModulesWatchers.get(watchDir); + if (existing) { + existing.refreshScriptInfoRefCount++; + return existing; + } + + const watcher = this.createNodeModulesWatcher(watchDir); + return { + close: () => { + if (watcher.refreshScriptInfoRefCount === 1 && watcher.affectedModuleSpecifierCacheProjects.size === 0) { + watcher.close(); + } + else { + watcher.refreshScriptInfoRefCount--; } }, - refCount: 1 }; - this.scriptInfoInNodeModulesWatchers.set(dir, result); - return result; } private getModifiedTime(info: ScriptInfo) { diff --git a/src/server/project.ts b/src/server/project.ts index 6fd73cc0d8e4a..58e5ddd96e465 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -785,6 +785,7 @@ namespace ts.server { this.resolutionCache.clear(); this.resolutionCache = undefined!; this.cachedUnresolvedImportsPerFile = undefined!; + this.moduleSpecifierCache = undefined!; this.directoryStructureHost = undefined!; this.projectErrors = undefined; @@ -1733,14 +1734,8 @@ namespace ts.server { } /*@internal*/ - watchNodeModulesDirectory(directoryPath: string, cb: DirectoryWatcherCallback) { - return this.projectService.watchFactory.watchDirectory( - directoryPath, - cb, - WatchDirectoryFlags.Recursive, - this.projectService.getWatchOptions(this), - WatchType.NodeModulesForClosedScriptInfo - ); + watchNodeModulesForPackageJsonChanges(directoryPath: string) { + return this.projectService.watchPackageJsonsInNodeModules(this.toPath(directoryPath), this); } } diff --git a/src/services/utilities.ts b/src/services/utilities.ts index 92330f159ade1..f8dbbab7dbbd6 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -3274,7 +3274,7 @@ namespace ts { } export interface ModuleSpecifierResolutionCacheHost { - watchNodeModulesDirectory(directoryPath: string, cb: DirectoryWatcherCallback): FileWatcher; + watchNodeModulesForPackageJsonChanges(directoryPath: string): FileWatcher; } export function createModuleSpecifierCache(host: ModuleSpecifierResolutionCacheHost): ModuleSpecifierCache { @@ -3301,11 +3301,12 @@ namespace ts { if (cached.moduleSpecifiers) { for (const p of cached.modulePaths) { if (p.isInNodeModules) { - const nodeModulesPath = p.path.substring(0, p.path.indexOf(nodeModulesPathPart) + nodeModulesPathPart.length); + // No trailing slash + const nodeModulesPath = p.path.substring(0, p.path.indexOf(nodeModulesPathPart) + nodeModulesPathPart.length - 1); if (!containedNodeModulesWatchers?.has(nodeModulesPath)) { (containedNodeModulesWatchers ||= new Map()).set( nodeModulesPath, - host.watchNodeModulesDirectory(nodeModulesPath, this.clear), + host.watchNodeModulesForPackageJsonChanges(nodeModulesPath), ); } } From 8f2c5d1c61bc5e1b57934c4647bcccd6e1f00bb3 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Fri, 4 Jun 2021 10:14:09 -0500 Subject: [PATCH 03/11] Update API baseline --- src/server/editorServices.ts | 2 +- src/server/watchType.ts | 5 ++--- tests/baselines/reference/api/tsserverlibrary.d.ts | 3 ++- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 5451b898358ab..726646e2fda0d 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -2659,7 +2659,7 @@ namespace ts.server { }, WatchDirectoryFlags.Recursive, this.hostConfiguration.watchOptions, - WatchType.NodeModulesForClosedScriptInfo + WatchType.NodeModules ); const result: NodeModulesWatcher = { refreshScriptInfoRefCount: affectedModuleSpecifierCacheProject ? 0 : 1, diff --git a/src/server/watchType.ts b/src/server/watchType.ts index 8a9d94dfb213b..2b9edafb09870 100644 --- a/src/server/watchType.ts +++ b/src/server/watchType.ts @@ -4,7 +4,7 @@ namespace ts { export interface WatchTypeRegistry { ClosedScriptInfo: "Closed Script info", ConfigFileForInferredRoot: "Config file for the inferred project root", - NodeModulesForClosedScriptInfo: "node_modules for closed script infos in them", + NodeModules: "node_modules for closed script infos and package.jsons affecting module specifier cache", MissingSourceMapFile: "Missing source map file", NoopConfigFileForInferredRoot: "Noop Config file for the inferred project root", MissingGeneratedFile: "Missing generated file", @@ -13,10 +13,9 @@ namespace ts { } WatchType.ClosedScriptInfo = "Closed Script info"; WatchType.ConfigFileForInferredRoot = "Config file for the inferred project root"; - WatchType.NodeModulesForClosedScriptInfo = "node_modules for closed script infos in them"; + WatchType.NodeModules = "node_modules for closed script infos and package.jsons affecting module specifier cache"; WatchType.MissingSourceMapFile = "Missing source map file"; WatchType.NoopConfigFileForInferredRoot = "Noop Config file for the inferred project root"; WatchType.MissingGeneratedFile = "Missing generated file"; WatchType.PackageJsonFile = "package.json file for import suggestions"; - WatchType.NodeModulesForModuleSpecifierCache = "node_modules for module specifier cache invalidation"; } diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index edd094bb4d964..20990feaac9c7 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -9920,7 +9920,7 @@ declare namespace ts.server { errors: Diagnostic[] | undefined; } export class ProjectService { - private readonly scriptInfoInNodeModulesWatchers; + private readonly nodeModulesWatchers; /** * Contains all the deleted script info's version information so that * it does not reset when creating script info again @@ -10070,6 +10070,7 @@ declare namespace ts.server { private createInferredProject; getScriptInfo(uncheckedFileName: string): ScriptInfo | undefined; private watchClosedScriptInfo; + private createNodeModulesWatcher; private watchClosedScriptInfoInNodeModules; private getModifiedTime; private refreshScriptInfo; From b3bb946ea615df4c2cdc7269f84a8fa91d40f3a9 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Mon, 7 Jun 2021 10:12:48 -0500 Subject: [PATCH 04/11] Small adjustments for fewer object allocations --- src/compiler/moduleSpecifiers.ts | 8 ++++---- src/compiler/types.ts | 6 +++--- src/server/editorServices.ts | 28 +++++++++++----------------- src/services/utilities.ts | 28 +++++++++++++--------------- 4 files changed, 31 insertions(+), 39 deletions(-) diff --git a/src/compiler/moduleSpecifiers.ts b/src/compiler/moduleSpecifiers.ts index 9dbf23ee02c3c..39e87e41c5353 100644 --- a/src/compiler/moduleSpecifiers.ts +++ b/src/compiler/moduleSpecifiers.ts @@ -132,7 +132,7 @@ namespace ts.moduleSpecifiers { )); if (existingSpecifier) { const moduleSpecifiers = [existingSpecifier]; - cache?.set(importingSourceFile.path, moduleSourceFile.path, { modulePaths, moduleSpecifiers }); + cache?.set(importingSourceFile.path, moduleSourceFile.path, modulePaths, moduleSpecifiers); return moduleSpecifiers; } @@ -152,7 +152,7 @@ namespace ts.moduleSpecifiers { if (specifier && modulePath.isRedirect) { // If we got a specifier for a redirect, it was a bare package specifier (e.g. "@foo/bar", // not "@foo/bar/path/to/file"). No other specifier will be this good, so stop looking. - cache?.set(importingSourceFile.path, moduleSourceFile.path, { modulePaths, moduleSpecifiers: nodeModulesSpecifiers! }); + cache?.set(importingSourceFile.path, moduleSourceFile.path, modulePaths, nodeModulesSpecifiers); return nodeModulesSpecifiers!; } @@ -179,7 +179,7 @@ namespace ts.moduleSpecifiers { const moduleSpecifiers = pathsSpecifiers?.length ? pathsSpecifiers : nodeModulesSpecifiers?.length ? nodeModulesSpecifiers : Debug.checkDefined(relativeSpecifiers); - cache?.set(importingSourceFile.path, moduleSourceFile.path, { modulePaths, moduleSpecifiers }); + cache?.set(importingSourceFile.path, moduleSourceFile.path, modulePaths, moduleSpecifiers); return moduleSpecifiers; } @@ -362,7 +362,7 @@ namespace ts.moduleSpecifiers { } const modulePaths = getAllModulePathsWorker(importingFilePath, importedFileName, host); if (cache) { - cache.setModulePaths(importingFilePath, importedFilePath, modulePaths); + cache.set(importingFilePath, importedFilePath, modulePaths); } return modulePaths; } diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 5c0a5fb9d2a19..60d5d8e9acd61 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -8109,9 +8109,9 @@ namespace ts { /* @internal */ export interface ModuleSpecifierCache { getModulePaths(fromFileName: Path, toFileName: Path): boolean | readonly ModulePath[] | undefined; - setModulePaths(fromFileName: Path, toFileName: Path, modulePaths: boolean | readonly ModulePath[]): void; - get(fromFileName: Path, toFileName: Path): { modulePaths: readonly ModulePath[], moduleSpecifiers?: readonly string[] } | boolean | undefined; - set(fromFileName: Path, toFileName: Path, cached: { modulePaths: readonly ModulePath[], moduleSpecifiers?: readonly string[] }): void; + get(fromFileName: Path, toFileName: Path): { modulePaths: readonly ModulePath[], moduleSpecifiers: readonly string[] } | boolean | undefined; + set(fromFileName: Path, toFileName: Path, modulePaths: readonly ModulePath[] | boolean): void; + set(fromFileName: Path, toFileName: Path, modulePaths: readonly ModulePath[], moduleSpecifiers?: readonly string[]): void; clear(): void; count(): number; } diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 726646e2fda0d..c249b5fa273d3 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -593,7 +593,7 @@ namespace ts.server { /** How many watchers of this directory were for closed ScriptInfo */ refreshScriptInfoRefCount: number; /** List of project names whose module specifier cache should be cleared when package.jsons change */ - affectedModuleSpecifierCacheProjects: Set; + affectedModuleSpecifierCacheProjects?: Set; } function getDetailWatchInfo(watchType: WatchType, project: Project | NormalizedPath | undefined) { @@ -2630,7 +2630,7 @@ namespace ts.server { // Clear module specifier cache for any projects whose cache was affected by // dependency package.jsons in this node_modules directory const basename = getBaseFileName(fileOrDirectoryPath); - if (result.affectedModuleSpecifierCacheProjects.size && ( + if (result.affectedModuleSpecifierCacheProjects?.size && ( basename === "package.json" || basename === "node_modules" )) { result.affectedModuleSpecifierCacheProjects.forEach(projectName => { @@ -2668,8 +2668,10 @@ namespace ts.server { ? [affectedModuleSpecifierCacheProject.getProjectName()] : emptyArray), close: () => { - watcher.close(); - this.nodeModulesWatchers.delete(dir); + if (!result.refreshScriptInfoRefCount && !result.affectedModuleSpecifierCacheProjects?.size) { + watcher.close(); + this.nodeModulesWatchers.delete(dir); + } }, }; this.nodeModulesWatchers.set(dir, result); @@ -2680,19 +2682,15 @@ namespace ts.server { watchPackageJsonsInNodeModules(dir: Path, project: Project): FileWatcher { const existing = this.nodeModulesWatchers.get(dir); if (existing) { - existing.affectedModuleSpecifierCacheProjects.add(project.getProjectName()); + (existing.affectedModuleSpecifierCacheProjects ||= new Set()).add(project.getProjectName()); return existing; } const watcher = this.createNodeModulesWatcher(dir, project); return { close: () => { - if (watcher.refreshScriptInfoRefCount === 0 && watcher.affectedModuleSpecifierCacheProjects.size === 1) { - watcher.close(); - } - else { - watcher.affectedModuleSpecifierCacheProjects.delete(project.getProjectName()); - } + watcher.affectedModuleSpecifierCacheProjects?.delete(project.getProjectName()); + watcher.close(); }, }; } @@ -2708,12 +2706,8 @@ namespace ts.server { const watcher = this.createNodeModulesWatcher(watchDir); return { close: () => { - if (watcher.refreshScriptInfoRefCount === 1 && watcher.affectedModuleSpecifierCacheProjects.size === 0) { - watcher.close(); - } - else { - watcher.refreshScriptInfoRefCount--; - } + watcher.refreshScriptInfoRefCount--; + watcher.close(); }, }; } diff --git a/src/services/utilities.ts b/src/services/utilities.ts index f8dbbab7dbbd6..d3ba494916869 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -3279,27 +3279,32 @@ namespace ts { export function createModuleSpecifierCache(host: ModuleSpecifierResolutionCacheHost): ModuleSpecifierCache { let containedNodeModulesWatchers: ESMap | undefined; - let cache: ESMap | undefined; + let cache: ESMap | undefined; let importingFileName: Path | undefined; const wrapped: ModuleSpecifierCache = { get(fromFileName, toFileName) { if (!cache || fromFileName !== importingFileName) return undefined; - return cache.get(toFileName); + const cached = cache.get(toFileName); + return isArray(cached) ? { modulePaths: cached, moduleSpecifiers: emptyArray } : cached; }, - set(fromFileName, toFileName, cached) { + set(fromFileName: Path, toFileName: Path, modulePaths: readonly ModulePath[] | boolean, moduleSpecifiers?: readonly string[]) { if (cache && fromFileName !== importingFileName) { this.clear(); } importingFileName = fromFileName; - (cache ||= new Map()).set(toFileName, cached); + if (typeof modulePaths === "boolean") { + (cache ||= new Map()).set(toFileName, modulePaths); + return; + } + (cache ||= new Map()).set(toFileName, moduleSpecifiers ? { modulePaths, moduleSpecifiers } : modulePaths); // If any module specifiers were generated based off paths in node_modules, // a package.json file in that package was read and is an input to the cached. // Instead of watching each individual package.json file, set up a wildcard // directory watcher for any node_modules referenced and clear the cache when // it sees any changes. - if (cached.moduleSpecifiers) { - for (const p of cached.modulePaths) { + if (moduleSpecifiers) { + for (const p of modulePaths) { if (p.isInNodeModules) { // No trailing slash const nodeModulesPath = p.path.substring(0, p.path.indexOf(nodeModulesPathPart) + nodeModulesPathPart.length - 1); @@ -3316,14 +3321,7 @@ namespace ts { getModulePaths(fromFileName, toFileName) { if (!cache || fromFileName !== importingFileName) return undefined; const cached = cache.get(toFileName); - return typeof cached === "object" ? cached.modulePaths : cached; - }, - setModulePaths(fromFileName, toFileName, modulePaths) { - if (cache && fromFileName !== importingFileName) { - this.clear(); - } - importingFileName = fromFileName; - (cache ||= new Map()).set(toFileName, typeof modulePaths === "boolean" ? modulePaths : { modulePaths }); + return (!cached || typeof cached === "boolean" || isArray(cached)) ? cached : cached.modulePaths; }, clear() { containedNodeModulesWatchers?.forEach(watcher => watcher.close()); @@ -3373,7 +3371,7 @@ namespace ts { if (packageJsonFilter) { const isImportable = hasImportablePath && packageJsonFilter.allowsImportingSourceFile(to, moduleSpecifierResolutionHost); - moduleSpecifierCache?.setModulePaths(from.path, to.path, isImportable); + moduleSpecifierCache?.set(from.path, to.path, isImportable); return isImportable; } From 117900318d574427895c8d031266aa0634e20320 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Mon, 7 Jun 2021 10:26:16 -0500 Subject: [PATCH 05/11] Document overloaded return value --- src/compiler/moduleSpecifiers.ts | 8 ++++---- src/compiler/types.ts | 9 ++++++++- src/services/utilities.ts | 3 +-- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/compiler/moduleSpecifiers.ts b/src/compiler/moduleSpecifiers.ts index 39e87e41c5353..af82c7579b4ec 100644 --- a/src/compiler/moduleSpecifiers.ts +++ b/src/compiler/moduleSpecifiers.ts @@ -111,8 +111,8 @@ namespace ts.moduleSpecifiers { const cached = cache?.get(importingSourceFile.path, moduleSourceFile.path); let modulePaths; if (typeof cached === "object") { - if (cached.moduleSpecifiers) return cached.moduleSpecifiers; - modulePaths = cached.modulePaths; + if (!isArray(cached)) return cached.moduleSpecifiers; + modulePaths = cached; } else { modulePaths = getAllModulePathsWorker(importingSourceFile.path, moduleSourceFile.originalFileName, host); @@ -357,8 +357,8 @@ namespace ts.moduleSpecifiers { ) { const cache = host.getModuleSpecifierCache?.(); if (cache) { - const cached = cache.get(importingFilePath, importedFilePath); - if (typeof cached === "object") return cached.modulePaths; + const cached = cache.getModulePaths(importingFilePath, importedFilePath); + if (typeof cached === "object") return cached; } const modulePaths = getAllModulePathsWorker(importingFilePath, importedFileName, host); if (cache) { diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 60d5d8e9acd61..b50f4d8548a06 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -8109,7 +8109,14 @@ namespace ts { /* @internal */ export interface ModuleSpecifierCache { getModulePaths(fromFileName: Path, toFileName: Path): boolean | readonly ModulePath[] | undefined; - get(fromFileName: Path, toFileName: Path): { modulePaths: readonly ModulePath[], moduleSpecifiers: readonly string[] } | boolean | undefined; + /** + * @returns + * - true if it's known that at least one module path is importable but the full array hasn't been computed ({@see isImportableFile}) + * - false if it's known that no module paths are importable + * - an array of ModulePaths if module paths have been computed but final module specifiers have not + * - an object containing ModulePaths and module specifiers if they've both been computed + */ + get(fromFileName: Path, toFileName: Path): readonly ModulePath[] | { modulePaths: readonly ModulePath[], moduleSpecifiers: readonly string[] } | boolean | undefined; set(fromFileName: Path, toFileName: Path, modulePaths: readonly ModulePath[] | boolean): void; set(fromFileName: Path, toFileName: Path, modulePaths: readonly ModulePath[], moduleSpecifiers?: readonly string[]): void; clear(): void; diff --git a/src/services/utilities.ts b/src/services/utilities.ts index d3ba494916869..48390ecc6120e 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -3284,8 +3284,7 @@ namespace ts { const wrapped: ModuleSpecifierCache = { get(fromFileName, toFileName) { if (!cache || fromFileName !== importingFileName) return undefined; - const cached = cache.get(toFileName); - return isArray(cached) ? { modulePaths: cached, moduleSpecifiers: emptyArray } : cached; + return cache.get(toFileName); }, set(fromFileName: Path, toFileName: Path, modulePaths: readonly ModulePath[] | boolean, moduleSpecifiers?: readonly string[]) { if (cache && fromFileName !== importingFileName) { From 7ee4dc703be343548727c85281c795d48a3ba3f5 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Mon, 7 Jun 2021 11:06:58 -0500 Subject: [PATCH 06/11] Update baselines --- ...to-file-that-has-same-ambient-module-and-is-also-module.js | 4 ++-- .../npm-install-when-timeout-occurs-after-installation.js | 4 ++-- .../npm-install-when-timeout-occurs-inbetween-installation.js | 4 ++-- ...when-built-with-disableSourceOfProjectReferenceRedirect.js | 2 ++ .../auto-import-with-referenced-project-when-built.js | 2 ++ .../projectReferences/auto-import-with-referenced-project.js | 2 ++ .../tsserver/resolutionCache/npm-install-@types-works.js | 4 ++-- 7 files changed, 14 insertions(+), 8 deletions(-) diff --git a/tests/baselines/reference/tsserver/projectErrors/correct-errors-when-resolution-resolves-to-file-that-has-same-ambient-module-and-is-also-module.js b/tests/baselines/reference/tsserver/projectErrors/correct-errors-when-resolution-resolves-to-file-that-has-same-ambient-module-and-is-also-module.js index 8cd0bada0c782..2bd9071b01162 100644 --- a/tests/baselines/reference/tsserver/projectErrors/correct-errors-when-resolution-resolves-to-file-that-has-same-ambient-module-and-is-also-module.js +++ b/tests/baselines/reference/tsserver/projectErrors/correct-errors-when-resolution-resolves-to-file-that-has-same-ambient-module-and-is-also-module.js @@ -18,8 +18,8 @@ DirectoryWatcher:: Added:: WatchInfo: /users/username/projects/myproject/src 1 u Elapsed:: *ms DirectoryWatcher:: Added:: WatchInfo: /users/username/projects/myproject/src 1 undefined Config: /users/username/projects/myproject/tsconfig.json WatchType: Wild card directory Plugins were requested but not running in environment that supports 'require'. Nothing will be loaded Starting updateGraphWorker: Project: /users/username/projects/myproject/tsconfig.json -DirectoryWatcher:: Added:: WatchInfo: /users/username/projects/myproject/node_modules 1 undefined WatchType: node_modules for closed script infos in them -Elapsed:: *ms DirectoryWatcher:: Added:: WatchInfo: /users/username/projects/myproject/node_modules 1 undefined WatchType: node_modules for closed script infos in them +DirectoryWatcher:: Added:: WatchInfo: /users/username/projects/myproject/node_modules 1 undefined WatchType: node_modules for closed script infos and package.jsons affecting module specifier cache +Elapsed:: *ms DirectoryWatcher:: Added:: WatchInfo: /users/username/projects/myproject/node_modules 1 undefined WatchType: node_modules for closed script infos and package.jsons affecting module specifier cache DirectoryWatcher:: Added:: WatchInfo: /users/username/projects/myproject/node_modules 1 undefined Project: /users/username/projects/myproject/tsconfig.json WatchType: Failed Lookup Locations Elapsed:: *ms DirectoryWatcher:: Added:: WatchInfo: /users/username/projects/myproject/node_modules 1 undefined Project: /users/username/projects/myproject/tsconfig.json WatchType: Failed Lookup Locations FileWatcher:: Added:: WatchInfo: /a/lib/lib.d.ts 500 undefined WatchType: Closed Script info diff --git a/tests/baselines/reference/tsserver/projectErrors/npm-install-when-timeout-occurs-after-installation.js b/tests/baselines/reference/tsserver/projectErrors/npm-install-when-timeout-occurs-after-installation.js index 61a0843e82382..419c68f410458 100644 --- a/tests/baselines/reference/tsserver/projectErrors/npm-install-when-timeout-occurs-after-installation.js +++ b/tests/baselines/reference/tsserver/projectErrors/npm-install-when-timeout-occurs-after-installation.js @@ -222,8 +222,8 @@ Scheduled: /user/username/projects/myproject/tsconfig.json, Cancelled earlier on Scheduled: *ensureProjectForOpenFiles*, Cancelled earlier one Running: /user/username/projects/myproject/tsconfig.json Starting updateGraphWorker: Project: /user/username/projects/myproject/tsconfig.json -DirectoryWatcher:: Added:: WatchInfo: /user/username/projects/myproject/node_modules 1 undefined WatchType: node_modules for closed script infos in them -Elapsed:: *ms DirectoryWatcher:: Added:: WatchInfo: /user/username/projects/myproject/node_modules 1 undefined WatchType: node_modules for closed script infos in them +DirectoryWatcher:: Added:: WatchInfo: /user/username/projects/myproject/node_modules 1 undefined WatchType: node_modules for closed script infos and package.jsons affecting module specifier cache +Elapsed:: *ms DirectoryWatcher:: Added:: WatchInfo: /user/username/projects/myproject/node_modules 1 undefined WatchType: node_modules for closed script infos and package.jsons affecting module specifier cache Finishing updateGraphWorker: Project: /user/username/projects/myproject/tsconfig.json Version: 3 structureChanged: true structureIsReused:: SafeModules Elapsed:: *ms Project '/user/username/projects/myproject/tsconfig.json' (Configured) Files (3) diff --git a/tests/baselines/reference/tsserver/projectErrors/npm-install-when-timeout-occurs-inbetween-installation.js b/tests/baselines/reference/tsserver/projectErrors/npm-install-when-timeout-occurs-inbetween-installation.js index 8c22a7cd8e362..b17e03d859139 100644 --- a/tests/baselines/reference/tsserver/projectErrors/npm-install-when-timeout-occurs-inbetween-installation.js +++ b/tests/baselines/reference/tsserver/projectErrors/npm-install-when-timeout-occurs-inbetween-installation.js @@ -245,8 +245,8 @@ Scheduled: /user/username/projects/myproject/tsconfig.json, Cancelled earlier on Scheduled: *ensureProjectForOpenFiles*, Cancelled earlier one Running: /user/username/projects/myproject/tsconfig.json Starting updateGraphWorker: Project: /user/username/projects/myproject/tsconfig.json -DirectoryWatcher:: Added:: WatchInfo: /user/username/projects/myproject/node_modules 1 undefined WatchType: node_modules for closed script infos in them -Elapsed:: *ms DirectoryWatcher:: Added:: WatchInfo: /user/username/projects/myproject/node_modules 1 undefined WatchType: node_modules for closed script infos in them +DirectoryWatcher:: Added:: WatchInfo: /user/username/projects/myproject/node_modules 1 undefined WatchType: node_modules for closed script infos and package.jsons affecting module specifier cache +Elapsed:: *ms DirectoryWatcher:: Added:: WatchInfo: /user/username/projects/myproject/node_modules 1 undefined WatchType: node_modules for closed script infos and package.jsons affecting module specifier cache Finishing updateGraphWorker: Project: /user/username/projects/myproject/tsconfig.json Version: 3 structureChanged: true structureIsReused:: SafeModules Elapsed:: *ms Project '/user/username/projects/myproject/tsconfig.json' (Configured) Files (3) diff --git a/tests/baselines/reference/tsserver/projectReferences/auto-import-with-referenced-project-when-built-with-disableSourceOfProjectReferenceRedirect.js b/tests/baselines/reference/tsserver/projectReferences/auto-import-with-referenced-project-when-built-with-disableSourceOfProjectReferenceRedirect.js index e7dda323106b0..c257c3c0f2e73 100644 --- a/tests/baselines/reference/tsserver/projectReferences/auto-import-with-referenced-project-when-built-with-disableSourceOfProjectReferenceRedirect.js +++ b/tests/baselines/reference/tsserver/projectReferences/auto-import-with-referenced-project-when-built-with-disableSourceOfProjectReferenceRedirect.js @@ -96,4 +96,6 @@ Open files: Projects: /user/username/projects/myproject/app/src/program/tsconfig.json response:{"responseRequired":false} request:{"command":"getCodeFixes","arguments":{"file":"/user/username/projects/myproject/app/src/program/index.ts","startLine":1,"startOffset":1,"endLine":1,"endOffset":4,"errorCodes":[2304]},"seq":1,"type":"request"} +DirectoryWatcher:: Added:: WatchInfo: /user/username/projects/myproject/node_modules 1 undefined WatchType: node_modules for closed script infos and package.jsons affecting module specifier cache +Elapsed:: *ms DirectoryWatcher:: Added:: WatchInfo: /user/username/projects/myproject/node_modules 1 undefined WatchType: node_modules for closed script infos and package.jsons affecting module specifier cache response:{"response":[{"fixName":"import","description":"Import 'foo' from module \"shared\"","changes":[{"fileName":"/user/username/projects/myproject/app/src/program/index.ts","textChanges":[{"start":{"line":1,"offset":1},"end":{"line":1,"offset":1},"newText":"import { foo } from \"shared\";\n\n"}]}]}],"responseRequired":true} \ No newline at end of file diff --git a/tests/baselines/reference/tsserver/projectReferences/auto-import-with-referenced-project-when-built.js b/tests/baselines/reference/tsserver/projectReferences/auto-import-with-referenced-project-when-built.js index c76675540b864..6f7e0f4c7773c 100644 --- a/tests/baselines/reference/tsserver/projectReferences/auto-import-with-referenced-project-when-built.js +++ b/tests/baselines/reference/tsserver/projectReferences/auto-import-with-referenced-project-when-built.js @@ -95,4 +95,6 @@ Open files: Projects: /user/username/projects/myproject/app/src/program/tsconfig.json response:{"responseRequired":false} request:{"command":"getCodeFixes","arguments":{"file":"/user/username/projects/myproject/app/src/program/index.ts","startLine":1,"startOffset":1,"endLine":1,"endOffset":4,"errorCodes":[2304]},"seq":1,"type":"request"} +DirectoryWatcher:: Added:: WatchInfo: /user/username/projects/myproject/node_modules 1 undefined WatchType: node_modules for closed script infos and package.jsons affecting module specifier cache +Elapsed:: *ms DirectoryWatcher:: Added:: WatchInfo: /user/username/projects/myproject/node_modules 1 undefined WatchType: node_modules for closed script infos and package.jsons affecting module specifier cache response:{"response":[{"fixName":"import","description":"Import 'foo' from module \"shared\"","changes":[{"fileName":"/user/username/projects/myproject/app/src/program/index.ts","textChanges":[{"start":{"line":1,"offset":1},"end":{"line":1,"offset":1},"newText":"import { foo } from \"shared\";\n\n"}]}]}],"responseRequired":true} \ No newline at end of file diff --git a/tests/baselines/reference/tsserver/projectReferences/auto-import-with-referenced-project.js b/tests/baselines/reference/tsserver/projectReferences/auto-import-with-referenced-project.js index c76675540b864..6f7e0f4c7773c 100644 --- a/tests/baselines/reference/tsserver/projectReferences/auto-import-with-referenced-project.js +++ b/tests/baselines/reference/tsserver/projectReferences/auto-import-with-referenced-project.js @@ -95,4 +95,6 @@ Open files: Projects: /user/username/projects/myproject/app/src/program/tsconfig.json response:{"responseRequired":false} request:{"command":"getCodeFixes","arguments":{"file":"/user/username/projects/myproject/app/src/program/index.ts","startLine":1,"startOffset":1,"endLine":1,"endOffset":4,"errorCodes":[2304]},"seq":1,"type":"request"} +DirectoryWatcher:: Added:: WatchInfo: /user/username/projects/myproject/node_modules 1 undefined WatchType: node_modules for closed script infos and package.jsons affecting module specifier cache +Elapsed:: *ms DirectoryWatcher:: Added:: WatchInfo: /user/username/projects/myproject/node_modules 1 undefined WatchType: node_modules for closed script infos and package.jsons affecting module specifier cache response:{"response":[{"fixName":"import","description":"Import 'foo' from module \"shared\"","changes":[{"fileName":"/user/username/projects/myproject/app/src/program/index.ts","textChanges":[{"start":{"line":1,"offset":1},"end":{"line":1,"offset":1},"newText":"import { foo } from \"shared\";\n\n"}]}]}],"responseRequired":true} \ No newline at end of file diff --git a/tests/baselines/reference/tsserver/resolutionCache/npm-install-@types-works.js b/tests/baselines/reference/tsserver/resolutionCache/npm-install-@types-works.js index c9833a1189cd4..a8baf59905f81 100644 --- a/tests/baselines/reference/tsserver/resolutionCache/npm-install-@types-works.js +++ b/tests/baselines/reference/tsserver/resolutionCache/npm-install-@types-works.js @@ -64,8 +64,8 @@ Elapsed:: *ms DirectoryWatcher:: Triggered with /a/b/projects/temp/node_modules/ Running: /dev/null/inferredProject1* Scheduled: *ensureProjectForOpenFiles*, Cancelled earlier one Starting updateGraphWorker: Project: /dev/null/inferredProject1* -DirectoryWatcher:: Added:: WatchInfo: /a/b/projects/temp/node_modules 1 undefined WatchType: node_modules for closed script infos in them -Elapsed:: *ms DirectoryWatcher:: Added:: WatchInfo: /a/b/projects/temp/node_modules 1 undefined WatchType: node_modules for closed script infos in them +DirectoryWatcher:: Added:: WatchInfo: /a/b/projects/temp/node_modules 1 undefined WatchType: node_modules for closed script infos and package.jsons affecting module specifier cache +Elapsed:: *ms DirectoryWatcher:: Added:: WatchInfo: /a/b/projects/temp/node_modules 1 undefined WatchType: node_modules for closed script infos and package.jsons affecting module specifier cache Finishing updateGraphWorker: Project: /dev/null/inferredProject1* Version: 2 structureChanged: true structureIsReused:: SafeModules Elapsed:: *ms Project '/dev/null/inferredProject1*' (Inferred) Files (3) From 1eaf8548401f2eedff35ea3dc6e2934792986f6f Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Tue, 8 Jun 2021 10:15:13 -0500 Subject: [PATCH 07/11] Store isAutoImportable separately from modulePaths --- src/compiler/moduleSpecifiers.ts | 18 +++-- src/compiler/types.ts | 22 +++--- src/services/utilities.ts | 70 ++++++++++++------- .../tsserver/moduleSpecifierCache.ts | 7 +- 4 files changed, 69 insertions(+), 48 deletions(-) diff --git a/src/compiler/moduleSpecifiers.ts b/src/compiler/moduleSpecifiers.ts index af82c7579b4ec..2c07cdfd5fb5c 100644 --- a/src/compiler/moduleSpecifiers.ts +++ b/src/compiler/moduleSpecifiers.ts @@ -110,14 +110,12 @@ namespace ts.moduleSpecifiers { const cache = host.getModuleSpecifierCache?.(); const cached = cache?.get(importingSourceFile.path, moduleSourceFile.path); let modulePaths; - if (typeof cached === "object") { - if (!isArray(cached)) return cached.moduleSpecifiers; - modulePaths = cached; - } - else { - modulePaths = getAllModulePathsWorker(importingSourceFile.path, moduleSourceFile.originalFileName, host); + if (cached) { + if (cached.moduleSpecifiers) return cached.moduleSpecifiers; + modulePaths = cached.modulePaths; } + modulePaths ||= getAllModulePathsWorker(importingSourceFile.path, moduleSourceFile.originalFileName, host); const preferences = getPreferences(userPreferences, compilerOptions, importingSourceFile); const existingSpecifier = forEach(modulePaths, modulePath => forEach( host.getFileIncludeReasons().get(toPath(modulePath.path, host.getCurrentDirectory(), info.getCanonicalFileName)), @@ -152,7 +150,7 @@ namespace ts.moduleSpecifiers { if (specifier && modulePath.isRedirect) { // If we got a specifier for a redirect, it was a bare package specifier (e.g. "@foo/bar", // not "@foo/bar/path/to/file"). No other specifier will be this good, so stop looking. - cache?.set(importingSourceFile.path, moduleSourceFile.path, modulePaths, nodeModulesSpecifiers); + cache?.set(importingSourceFile.path, moduleSourceFile.path, modulePaths, nodeModulesSpecifiers!); return nodeModulesSpecifiers!; } @@ -357,12 +355,12 @@ namespace ts.moduleSpecifiers { ) { const cache = host.getModuleSpecifierCache?.(); if (cache) { - const cached = cache.getModulePaths(importingFilePath, importedFilePath); - if (typeof cached === "object") return cached; + const cached = cache.get(importingFilePath, importedFilePath); + if (cached?.modulePaths) return cached.modulePaths; } const modulePaths = getAllModulePathsWorker(importingFilePath, importedFileName, host); if (cache) { - cache.set(importingFilePath, importedFilePath, modulePaths); + cache.setModulePaths(importingFilePath, importedFilePath, modulePaths); } return modulePaths; } diff --git a/src/compiler/types.ts b/src/compiler/types.ts index fc14a0bb65d41..d8aa7affafdd9 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -8155,19 +8155,19 @@ namespace ts { isRedirect: boolean; } + /*@internal*/ + export interface ResolvedModuleSpecifierInfo { + modulePaths: readonly ModulePath[] | undefined; + moduleSpecifiers: readonly string[] | undefined; + isAutoImportable: boolean | undefined; + } + /* @internal */ export interface ModuleSpecifierCache { - getModulePaths(fromFileName: Path, toFileName: Path): boolean | readonly ModulePath[] | undefined; - /** - * @returns - * - true if it's known that at least one module path is importable but the full array hasn't been computed ({@see isImportableFile}) - * - false if it's known that no module paths are importable - * - an array of ModulePaths if module paths have been computed but final module specifiers have not - * - an object containing ModulePaths and module specifiers if they've both been computed - */ - get(fromFileName: Path, toFileName: Path): readonly ModulePath[] | { modulePaths: readonly ModulePath[], moduleSpecifiers: readonly string[] } | boolean | undefined; - set(fromFileName: Path, toFileName: Path, modulePaths: readonly ModulePath[] | boolean): void; - set(fromFileName: Path, toFileName: Path, modulePaths: readonly ModulePath[], moduleSpecifiers?: readonly string[]): void; + get(fromFileName: Path, toFileName: Path): Readonly | undefined; + set(fromFileName: Path, toFileName: Path, modulePaths: readonly ModulePath[], moduleSpecifiers: readonly string[]): void; + setIsAutoImportable(fromFileName: Path, toFileName: Path, isAutoImportable: boolean): void; + setModulePaths(fromFileName: Path, toFileName: Path, modulePaths: readonly ModulePath[]): void; clear(): void; count(): number; } diff --git a/src/services/utilities.ts b/src/services/utilities.ts index 8fd2dbd607ace..406dcda4dcaaf 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -3282,23 +3282,15 @@ namespace ts { export function createModuleSpecifierCache(host: ModuleSpecifierResolutionCacheHost): ModuleSpecifierCache { let containedNodeModulesWatchers: ESMap | undefined; - let cache: ESMap | undefined; + let cache: ESMap | undefined; let importingFileName: Path | undefined; - const wrapped: ModuleSpecifierCache = { + const result: ModuleSpecifierCache = { get(fromFileName, toFileName) { if (!cache || fromFileName !== importingFileName) return undefined; return cache.get(toFileName); }, - set(fromFileName: Path, toFileName: Path, modulePaths: readonly ModulePath[] | boolean, moduleSpecifiers?: readonly string[]) { - if (cache && fromFileName !== importingFileName) { - this.clear(); - } - importingFileName = fromFileName; - if (typeof modulePaths === "boolean") { - (cache ||= new Map()).set(toFileName, modulePaths); - return; - } - (cache ||= new Map()).set(toFileName, moduleSpecifiers ? { modulePaths, moduleSpecifiers } : modulePaths); + set(fromFileName, toFileName, modulePaths, moduleSpecifiers) { + ensureCache(fromFileName).set(toFileName, createInfo(modulePaths, moduleSpecifiers, /*isAutoImportable*/ true)); // If any module specifiers were generated based off paths in node_modules, // a package.json file in that package was read and is an input to the cached. @@ -3320,10 +3312,25 @@ namespace ts { } } }, - getModulePaths(fromFileName, toFileName) { - if (!cache || fromFileName !== importingFileName) return undefined; - const cached = cache.get(toFileName); - return (!cached || typeof cached === "boolean" || isArray(cached)) ? cached : cached.modulePaths; + setModulePaths(fromFileName, toFileName, modulePaths) { + const cache = ensureCache(fromFileName); + const info = cache.get(toFileName); + if (info) { + info.modulePaths = modulePaths; + } + else { + cache.set(toFileName, createInfo(modulePaths, /*moduleSpecifiers*/ undefined, /*isAutoImportable*/ undefined)); + } + }, + setIsAutoImportable(fromFileName, toFileName, isAutoImportable) { + const cache = ensureCache(fromFileName); + const info = cache.get(toFileName); + if (info) { + info.isAutoImportable = isAutoImportable; + } + else { + cache.set(toFileName, createInfo(/*modulePaths*/ undefined, /*moduleSpecifiers*/ undefined, isAutoImportable)); + } }, clear() { containedNodeModulesWatchers?.forEach(watcher => watcher.close()); @@ -3336,9 +3343,25 @@ namespace ts { } }; if (Debug.isDebugging) { - Object.defineProperty(wrapped, "__cache", { get: () => cache }); + Object.defineProperty(result, "__cache", { get: () => cache }); + } + return result; + + function ensureCache(fromFileName: Path) { + if (cache && fromFileName !== importingFileName) { + result.clear(); + } + importingFileName = fromFileName; + return cache ||= new Map(); + } + + function createInfo( + modulePaths: readonly ModulePath[] | undefined, + moduleSpecifiers: readonly string[] | undefined, + isAutoImportable: boolean | undefined, + ): ResolvedModuleSpecifierInfo { + return { modulePaths, moduleSpecifiers, isAutoImportable }; } - return wrapped; } export function isImportableFile( @@ -3350,9 +3373,9 @@ namespace ts { moduleSpecifierCache: ModuleSpecifierCache | undefined, ): boolean { if (from === to) return false; - const cachedResult = moduleSpecifierCache?.getModulePaths(from.path, to.path); - if (cachedResult !== undefined) { - return !!cachedResult; + const cachedResult = moduleSpecifierCache?.get(from.path, to.path); + if (cachedResult?.isAutoImportable !== undefined) { + return cachedResult.isAutoImportable; } const getCanonicalFileName = hostGetCanonicalFileName(moduleSpecifierResolutionHost); @@ -3372,9 +3395,8 @@ namespace ts { ); if (packageJsonFilter) { - const isImportable = hasImportablePath && packageJsonFilter.allowsImportingSourceFile(to, moduleSpecifierResolutionHost); - moduleSpecifierCache?.set(from.path, to.path, isImportable); - return isImportable; + const isAutoImportable = hasImportablePath && packageJsonFilter.allowsImportingSourceFile(to, moduleSpecifierResolutionHost); + moduleSpecifierCache?.setIsAutoImportable(from.path, to.path, isAutoImportable); } return hasImportablePath; diff --git a/src/testRunner/unittests/tsserver/moduleSpecifierCache.ts b/src/testRunner/unittests/tsserver/moduleSpecifierCache.ts index d094dfe72cffe..b29724fa6dccd 100644 --- a/src/testRunner/unittests/tsserver/moduleSpecifierCache.ts +++ b/src/testRunner/unittests/tsserver/moduleSpecifierCache.ts @@ -35,7 +35,7 @@ namespace ts.projectSystem { describe("unittests:: tsserver:: moduleSpecifierCache", () => { it("caches importability within a file", () => { const { moduleSpecifierCache } = setup(); - assert.isTrue(moduleSpecifierCache.getModulePaths(bTs.path as Path, aTs.path as Path)); + assert.isTrue(moduleSpecifierCache.get(bTs.path as Path, aTs.path as Path)?.isAutoImportable); }); it("caches module specifiers within a file", () => { @@ -49,7 +49,8 @@ namespace ts.projectSystem { isInNodeModules: true, isRedirect: false }], - moduleSpecifiers: ["mobx"] + moduleSpecifiers: ["mobx"], + isAutoImportable: true, }); }); @@ -67,7 +68,7 @@ namespace ts.projectSystem { const { host, moduleSpecifierCache } = setup(); host.writeFile("/src/a2.ts", aTs.content); host.runQueuedTimeoutCallbacks(); - assert.isTrue(moduleSpecifierCache.getModulePaths(bTs.path as Path, aTs.path as Path)); + assert.isTrue(moduleSpecifierCache.get(bTs.path as Path, aTs.path as Path)?.isAutoImportable); }); it("invalidates the cache when symlinks are added or removed", () => { From 24c07ac0eacc9913cd59cbeb9dda3fb619772f23 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Tue, 8 Jun 2021 10:48:30 -0500 Subject: [PATCH 08/11] Add back missing return --- src/services/utilities.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/services/utilities.ts b/src/services/utilities.ts index 406dcda4dcaaf..d3ca113bd0ea4 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -3397,6 +3397,7 @@ namespace ts { if (packageJsonFilter) { const isAutoImportable = hasImportablePath && packageJsonFilter.allowsImportingSourceFile(to, moduleSpecifierResolutionHost); moduleSpecifierCache?.setIsAutoImportable(from.path, to.path, isAutoImportable); + return isAutoImportable; } return hasImportablePath; From 90a6c190d06bf0f3cf898b743aec571376fe3fcd Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Tue, 8 Jun 2021 12:19:07 -0500 Subject: [PATCH 09/11] Return wrapped watcher instead of underlying one --- src/server/editorServices.ts | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index c24e386d41a3c..95a28b891ab27 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -2629,7 +2629,7 @@ namespace ts.server { } } - private createNodeModulesWatcher(dir: Path, affectedModuleSpecifierCacheProject?: Project) { + private createNodeModulesWatcher(dir: Path) { const watcher = this.watchFactory.watchDirectory( dir, fileOrDirectory => { @@ -2671,11 +2671,8 @@ namespace ts.server { WatchType.NodeModules ); const result: NodeModulesWatcher = { - refreshScriptInfoRefCount: affectedModuleSpecifierCacheProject ? 0 : 1, - affectedModuleSpecifierCacheProjects: new Set( - affectedModuleSpecifierCacheProject - ? [affectedModuleSpecifierCacheProject.getProjectName()] - : emptyArray), + refreshScriptInfoRefCount: 0, + affectedModuleSpecifierCacheProjects: undefined, close: () => { if (!result.refreshScriptInfoRefCount && !result.affectedModuleSpecifierCacheProjects?.size) { watcher.close(); @@ -2689,13 +2686,9 @@ namespace ts.server { /*@internal*/ watchPackageJsonsInNodeModules(dir: Path, project: Project): FileWatcher { - const existing = this.nodeModulesWatchers.get(dir); - if (existing) { - (existing.affectedModuleSpecifierCacheProjects ||= new Set()).add(project.getProjectName()); - return existing; - } + const watcher = this.nodeModulesWatchers.get(dir) || this.createNodeModulesWatcher(dir); + (watcher.affectedModuleSpecifierCacheProjects ||= new Set()).add(project.getProjectName()); - const watcher = this.createNodeModulesWatcher(dir, project); return { close: () => { watcher.affectedModuleSpecifierCacheProjects?.delete(project.getProjectName()); @@ -2706,13 +2699,9 @@ namespace ts.server { private watchClosedScriptInfoInNodeModules(dir: Path): FileWatcher { const watchDir = dir + "/node_modules" as Path; - const existing = this.nodeModulesWatchers.get(watchDir); - if (existing) { - existing.refreshScriptInfoRefCount++; - return existing; - } + const watcher = this.nodeModulesWatchers.get(watchDir) || this.createNodeModulesWatcher(watchDir); + watcher.refreshScriptInfoRefCount++; - const watcher = this.createNodeModulesWatcher(watchDir); return { close: () => { watcher.refreshScriptInfoRefCount--; From c7b6f1f9156e29a838063e00a2ad663687f77544 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Tue, 8 Jun 2021 13:39:44 -0500 Subject: [PATCH 10/11] Make user preferences part of the cache key instead of implicitly clearing in editor services --- src/compiler/moduleSpecifiers.ts | 26 +++++++------ src/compiler/transformers/declarations.ts | 1 - src/compiler/types.ts | 8 ++-- src/server/editorServices.ts | 7 ---- src/services/codefixes/importFixes.ts | 31 ++++++++------- src/services/completions.ts | 3 +- src/services/utilities.ts | 39 +++++++++++-------- .../tsserver/moduleSpecifierCache.ts | 28 ++++++++----- 8 files changed, 77 insertions(+), 66 deletions(-) diff --git a/src/compiler/moduleSpecifiers.ts b/src/compiler/moduleSpecifiers.ts index 2c07cdfd5fb5c..007afa9a54d67 100644 --- a/src/compiler/moduleSpecifiers.ts +++ b/src/compiler/moduleSpecifiers.ts @@ -47,7 +47,7 @@ namespace ts.moduleSpecifiers { host: ModuleSpecifierResolutionHost, oldImportSpecifier: string, ): string | undefined { - const res = getModuleSpecifierWorker(compilerOptions, importingSourceFileName, toFileName, host, getPreferencesForUpdate(compilerOptions, oldImportSpecifier)); + const res = getModuleSpecifierWorker(compilerOptions, importingSourceFileName, toFileName, host, getPreferencesForUpdate(compilerOptions, oldImportSpecifier), {}); if (res === oldImportSpecifier) return undefined; return res; } @@ -59,9 +59,8 @@ namespace ts.moduleSpecifiers { importingSourceFileName: Path, toFileName: string, host: ModuleSpecifierResolutionHost, - preferences: UserPreferences = {}, ): string { - return getModuleSpecifierWorker(compilerOptions, importingSourceFileName, toFileName, host, getPreferences(preferences, compilerOptions, importingSourceFile)); + return getModuleSpecifierWorker(compilerOptions, importingSourceFileName, toFileName, host, getPreferences({}, compilerOptions, importingSourceFile), {}); } export function getNodeModulesPackageName( @@ -69,9 +68,10 @@ namespace ts.moduleSpecifiers { importingSourceFileName: Path, nodeModulesFileName: string, host: ModuleSpecifierResolutionHost, + preferences: UserPreferences, ): string | undefined { const info = getInfo(importingSourceFileName, host); - const modulePaths = getAllModulePaths(importingSourceFileName, nodeModulesFileName, host); + const modulePaths = getAllModulePaths(importingSourceFileName, nodeModulesFileName, host, preferences); return firstDefined(modulePaths, modulePath => tryGetModuleNameAsNodeModule(modulePath, info, host, compilerOptions, /*packageNameOnly*/ true)); } @@ -81,10 +81,11 @@ namespace ts.moduleSpecifiers { importingSourceFileName: Path, toFileName: string, host: ModuleSpecifierResolutionHost, - preferences: Preferences + preferences: Preferences, + userPreferences: UserPreferences, ): string { const info = getInfo(importingSourceFileName, host); - const modulePaths = getAllModulePaths(importingSourceFileName, toFileName, host); + const modulePaths = getAllModulePaths(importingSourceFileName, toFileName, host, userPreferences); return firstDefined(modulePaths, modulePath => tryGetModuleNameAsNodeModule(modulePath, info, host, compilerOptions)) || getLocalModuleSpecifier(toFileName, info, compilerOptions, host, preferences); } @@ -108,7 +109,7 @@ namespace ts.moduleSpecifiers { } const cache = host.getModuleSpecifierCache?.(); - const cached = cache?.get(importingSourceFile.path, moduleSourceFile.path); + const cached = cache?.get(importingSourceFile.path, moduleSourceFile.path, userPreferences); let modulePaths; if (cached) { if (cached.moduleSpecifiers) return cached.moduleSpecifiers; @@ -130,7 +131,7 @@ namespace ts.moduleSpecifiers { )); if (existingSpecifier) { const moduleSpecifiers = [existingSpecifier]; - cache?.set(importingSourceFile.path, moduleSourceFile.path, modulePaths, moduleSpecifiers); + cache?.set(importingSourceFile.path, moduleSourceFile.path, userPreferences, modulePaths, moduleSpecifiers); return moduleSpecifiers; } @@ -150,7 +151,7 @@ namespace ts.moduleSpecifiers { if (specifier && modulePath.isRedirect) { // If we got a specifier for a redirect, it was a bare package specifier (e.g. "@foo/bar", // not "@foo/bar/path/to/file"). No other specifier will be this good, so stop looking. - cache?.set(importingSourceFile.path, moduleSourceFile.path, modulePaths, nodeModulesSpecifiers!); + cache?.set(importingSourceFile.path, moduleSourceFile.path, userPreferences, modulePaths, nodeModulesSpecifiers!); return nodeModulesSpecifiers!; } @@ -177,7 +178,7 @@ namespace ts.moduleSpecifiers { const moduleSpecifiers = pathsSpecifiers?.length ? pathsSpecifiers : nodeModulesSpecifiers?.length ? nodeModulesSpecifiers : Debug.checkDefined(relativeSpecifiers); - cache?.set(importingSourceFile.path, moduleSourceFile.path, modulePaths, moduleSpecifiers); + cache?.set(importingSourceFile.path, moduleSourceFile.path, userPreferences, modulePaths, moduleSpecifiers); return moduleSpecifiers; } @@ -351,16 +352,17 @@ namespace ts.moduleSpecifiers { importingFilePath: Path, importedFileName: string, host: ModuleSpecifierResolutionHost, + preferences: UserPreferences, importedFilePath = toPath(importedFileName, host.getCurrentDirectory(), hostGetCanonicalFileName(host)) ) { const cache = host.getModuleSpecifierCache?.(); if (cache) { - const cached = cache.get(importingFilePath, importedFilePath); + const cached = cache.get(importingFilePath, importedFilePath, preferences); if (cached?.modulePaths) return cached.modulePaths; } const modulePaths = getAllModulePathsWorker(importingFilePath, importedFileName, host); if (cache) { - cache.setModulePaths(importingFilePath, importedFilePath, modulePaths); + cache.setModulePaths(importingFilePath, importedFilePath, preferences, modulePaths); } return modulePaths; } diff --git a/src/compiler/transformers/declarations.ts b/src/compiler/transformers/declarations.ts index 6a9cb08fa7cec..cc58530f46633 100644 --- a/src/compiler/transformers/declarations.ts +++ b/src/compiler/transformers/declarations.ts @@ -380,7 +380,6 @@ namespace ts { toPath(outputFilePath, host.getCurrentDirectory(), host.getCanonicalFileName), toPath(declFileName, host.getCurrentDirectory(), host.getCanonicalFileName), host, - /*preferences*/ undefined, ); if (!pathIsRelative(specifier)) { // If some compiler option/symlink/whatever allows access to the file containing the ambient module declaration diff --git a/src/compiler/types.ts b/src/compiler/types.ts index d8aa7affafdd9..5dd981db7ead3 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -8164,10 +8164,10 @@ namespace ts { /* @internal */ export interface ModuleSpecifierCache { - get(fromFileName: Path, toFileName: Path): Readonly | undefined; - set(fromFileName: Path, toFileName: Path, modulePaths: readonly ModulePath[], moduleSpecifiers: readonly string[]): void; - setIsAutoImportable(fromFileName: Path, toFileName: Path, isAutoImportable: boolean): void; - setModulePaths(fromFileName: Path, toFileName: Path, modulePaths: readonly ModulePath[]): void; + get(fromFileName: Path, toFileName: Path, preferences: UserPreferences): Readonly | undefined; + set(fromFileName: Path, toFileName: Path, preferences: UserPreferences, modulePaths: readonly ModulePath[], moduleSpecifiers: readonly string[]): void; + setIsAutoImportable(fromFileName: Path, toFileName: Path, preferences: UserPreferences, isAutoImportable: boolean): void; + setModulePaths(fromFileName: Path, toFileName: Path, preferences: UserPreferences, modulePaths: readonly ModulePath[]): void; clear(): void; count(): number; } diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 95a28b891ab27..7378656c00115 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -2988,8 +2988,6 @@ namespace ts.server { const { lazyConfiguredProjectsFromExternalProject, includePackageJsonAutoImports, - importModuleSpecifierEnding, - importModuleSpecifierPreference, } = this.hostConfiguration.preferences; this.hostConfiguration.preferences = { ...this.hostConfiguration.preferences, ...args.preferences }; @@ -3006,11 +3004,6 @@ namespace ts.server { if (includePackageJsonAutoImports !== args.preferences.includePackageJsonAutoImports) { this.invalidateProjectPackageJson(/*packageJsonPath*/ undefined); } - if (importModuleSpecifierEnding !== args.preferences.importModuleSpecifierEnding || importModuleSpecifierPreference !== args.preferences.importModuleSpecifierPreference) { - this.configuredProjects.forEach(p => p.getModuleSpecifierCache().clear()); - this.inferredProjects.forEach(p => p.getModuleSpecifierCache().clear()); - this.externalProjects.forEach(p => p.getModuleSpecifierCache().clear()); - } } if (args.extraFileExtensions) { this.hostConfiguration.extraFileExtensions = args.extraFileExtensions; diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 061aeaaca7733..5d96408b6265a 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -62,7 +62,7 @@ namespace ts.codefix { const symbolName = getNameForExportedSymbol(exportedSymbol, getEmitScriptTarget(compilerOptions)); const checker = program.getTypeChecker(); const symbol = checker.getMergedSymbol(skipAlias(exportedSymbol, checker)); - const exportInfos = getAllReExportingModules(sourceFile, symbol, moduleSymbol, symbolName, host, program, useAutoImportProvider); + const exportInfos = getAllReExportingModules(sourceFile, symbol, moduleSymbol, symbolName, host, program, preferences, useAutoImportProvider); const preferTypeOnlyImport = !!usageIsTypeOnly && compilerOptions.importsNotUsedAsValues === ImportsNotUsedAsValues.Error; const useRequire = shouldUseRequire(sourceFile, program); const fix = getImportFixForSymbol(sourceFile, exportInfos, moduleSymbol, symbolName, program, /*position*/ undefined, preferTypeOnlyImport, useRequire, host, preferences); @@ -200,7 +200,7 @@ namespace ts.codefix { const compilerOptions = program.getCompilerOptions(); const exportInfos = pathIsBareSpecifier(stripQuotes(moduleSymbol.name)) ? [getSymbolExportInfoForSymbol(exportedSymbol, moduleSymbol, program, host)] - : getAllReExportingModules(sourceFile, exportedSymbol, moduleSymbol, symbolName, host, program, /*useAutoImportProvider*/ true); + : getAllReExportingModules(sourceFile, exportedSymbol, moduleSymbol, symbolName, host, program, preferences, /*useAutoImportProvider*/ true); const useRequire = shouldUseRequire(sourceFile, program); const preferTypeOnlyImport = compilerOptions.importsNotUsedAsValues === ImportsNotUsedAsValues.Error && !isSourceFileJS(sourceFile) && isValidTypeOnlyAliasUseSite(getTokenAtPosition(sourceFile, position)); const fix = getImportFixForSymbol(sourceFile, exportInfos, moduleSymbol, symbolName, program, position, preferTypeOnlyImport, useRequire, host, preferences); @@ -209,7 +209,7 @@ namespace ts.codefix { function getImportFixForSymbol(sourceFile: SourceFile, exportInfos: readonly SymbolExportInfo[], moduleSymbol: Symbol, symbolName: string, program: Program, position: number | undefined, preferTypeOnlyImport: boolean, useRequire: boolean, host: LanguageServiceHost, preferences: UserPreferences) { Debug.assert(exportInfos.some(info => info.moduleSymbol === moduleSymbol), "Some exportInfo should match the specified moduleSymbol"); - return getBestFix(getImportFixes(exportInfos, symbolName, position, preferTypeOnlyImport, useRequire, program, sourceFile, host, preferences), sourceFile, host); + return getBestFix(getImportFixes(exportInfos, symbolName, position, preferTypeOnlyImport, useRequire, program, sourceFile, host, preferences), sourceFile, host, preferences); } function codeFixActionToCodeAction({ description, changes, commands }: CodeFixAction): CodeAction { @@ -237,7 +237,7 @@ namespace ts.codefix { } } - function getAllReExportingModules(importingFile: SourceFile, exportedSymbol: Symbol, exportingModuleSymbol: Symbol, symbolName: string, host: LanguageServiceHost, program: Program, useAutoImportProvider: boolean): readonly SymbolExportInfo[] { + function getAllReExportingModules(importingFile: SourceFile, exportedSymbol: Symbol, exportingModuleSymbol: Symbol, symbolName: string, host: LanguageServiceHost, program: Program, preferences: UserPreferences, useAutoImportProvider: boolean): readonly SymbolExportInfo[] { const result: SymbolExportInfo[] = []; const compilerOptions = program.getCompilerOptions(); const getModuleSpecifierResolutionHost = memoizeOne((isFromPackageJson: boolean) => { @@ -265,7 +265,7 @@ namespace ts.codefix { return result; function isImportable(program: Program, moduleFile: SourceFile | undefined, isFromPackageJson: boolean) { - return !moduleFile || isImportableFile(program, importingFile, moduleFile, /*packageJsonFilter*/ undefined, getModuleSpecifierResolutionHost(isFromPackageJson), host.getModuleSpecifierCache?.()); + return !moduleFile || isImportableFile(program, importingFile, moduleFile, preferences, /*packageJsonFilter*/ undefined, getModuleSpecifierResolutionHost(isFromPackageJson), host.getModuleSpecifierCache?.()); } } @@ -275,7 +275,7 @@ namespace ts.codefix { host: LanguageServiceHost, preferences: UserPreferences ): { exportInfo?: SymbolExportInfo, moduleSpecifier: string } { - return getBestFix(getNewImportFixes(program, importingFile, /*position*/ undefined, /*preferTypeOnlyImport*/ false, /*useRequire*/ false, exportInfo, host, preferences), importingFile, host); + return getBestFix(getNewImportFixes(program, importingFile, /*position*/ undefined, /*preferTypeOnlyImport*/ false, /*useRequire*/ false, exportInfo, host, preferences), importingFile, host, preferences); } export function getSymbolToExportInfoMap(importingFile: SourceFile, host: LanguageServiceHost, program: Program) { @@ -521,20 +521,20 @@ namespace ts.codefix { const info = errorCode === Diagnostics._0_refers_to_a_UMD_global_but_the_current_file_is_a_module_Consider_adding_an_import_instead.code ? getFixesInfoForUMDImport(context, symbolToken) : isIdentifier(symbolToken) ? getFixesInfoForNonUMDImport(context, symbolToken, useAutoImportProvider) : undefined; - return info && { ...info, fixes: sortFixes(info.fixes, context.sourceFile, context.host) }; + return info && { ...info, fixes: sortFixes(info.fixes, context.sourceFile, context.host, context.preferences) }; } - function sortFixes(fixes: readonly ImportFix[], sourceFile: SourceFile, host: LanguageServiceHost): readonly ImportFix[] { - const { allowsImportingSpecifier } = createPackageJsonImportFilter(sourceFile, host); + function sortFixes(fixes: readonly ImportFix[], sourceFile: SourceFile, host: LanguageServiceHost, preferences: UserPreferences): readonly ImportFix[] { + const { allowsImportingSpecifier } = createPackageJsonImportFilter(sourceFile, preferences, host); return sort(fixes, (a, b) => compareValues(a.kind, b.kind) || compareModuleSpecifiers(a, b, allowsImportingSpecifier)); } - function getBestFix(fixes: readonly T[], sourceFile: SourceFile, host: LanguageServiceHost): T { + function getBestFix(fixes: readonly T[], sourceFile: SourceFile, host: LanguageServiceHost, preferences: UserPreferences): T { // These will always be placed first if available, and are better than other kinds if (fixes[0].kind === ImportFixKind.UseNamespace || fixes[0].kind === ImportFixKind.AddToExisting) { return fixes[0]; } - const { allowsImportingSpecifier } = createPackageJsonImportFilter(sourceFile, host); + const { allowsImportingSpecifier } = createPackageJsonImportFilter(sourceFile, preferences, host); return fixes.reduce((best, fix) => compareModuleSpecifiers(fix, best, allowsImportingSpecifier) === Comparison.LessThan ? fix : best ); @@ -618,7 +618,7 @@ namespace ts.codefix { const preferTypeOnlyImport = compilerOptions.importsNotUsedAsValues === ImportsNotUsedAsValues.Error && isValidTypeOnlyAliasUseSite(symbolToken); const useRequire = shouldUseRequire(sourceFile, program); - const exportInfos = getExportInfos(symbolName, getMeaningFromLocation(symbolToken), cancellationToken, sourceFile, program, useAutoImportProvider, host); + const exportInfos = getExportInfos(symbolName, getMeaningFromLocation(symbolToken), cancellationToken, sourceFile, program, useAutoImportProvider, host, preferences); const fixes = arrayFrom(flatMapIterator(exportInfos.entries(), ([_, exportInfos]) => getImportFixes(exportInfos, symbolName, symbolToken.getStart(sourceFile), preferTypeOnlyImport, useRequire, program, sourceFile, host, preferences))); return { fixes, symbolName }; @@ -643,19 +643,20 @@ namespace ts.codefix { fromFile: SourceFile, program: Program, useAutoImportProvider: boolean, - host: LanguageServiceHost + host: LanguageServiceHost, + preferences: UserPreferences, ): ReadonlyESMap { // For each original symbol, keep all re-exports of that symbol together so we can call `getCodeActionsForImport` on the whole group at once. // Maps symbol id to info for modules providing that symbol (original export + re-exports). const originalSymbolToExportInfos = createMultiMap(); - const packageJsonFilter = createPackageJsonImportFilter(fromFile, host); + const packageJsonFilter = createPackageJsonImportFilter(fromFile, preferences, host); const moduleSpecifierCache = host.getModuleSpecifierCache?.(); const getModuleSpecifierResolutionHost = memoizeOne((isFromPackageJson: boolean) => { return createModuleSpecifierResolutionHost(isFromPackageJson ? host.getPackageJsonAutoImportProvider!()! : program, host); }); function addSymbol(moduleSymbol: Symbol, toFile: SourceFile | undefined, exportedSymbol: Symbol, exportKind: ExportKind, program: Program, isFromPackageJson: boolean): void { const moduleSpecifierResolutionHost = getModuleSpecifierResolutionHost(isFromPackageJson); - if (toFile && isImportableFile(program, fromFile, toFile, packageJsonFilter, moduleSpecifierResolutionHost, moduleSpecifierCache) || + if (toFile && isImportableFile(program, fromFile, toFile, preferences, packageJsonFilter, moduleSpecifierResolutionHost, moduleSpecifierCache) || !toFile && packageJsonFilter.allowsImportingAmbientModule(moduleSymbol, moduleSpecifierResolutionHost) ) { const checker = program.getTypeChecker(); diff --git a/src/services/completions.ts b/src/services/completions.ts index e97753908f7c4..4a5d58f5f6506 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -1753,7 +1753,7 @@ namespace ts.Completions { const lowerCaseTokenText = previousToken && isIdentifier(previousToken) ? previousToken.text.toLowerCase() : ""; const exportInfo = codefix.getSymbolToExportInfoMap(sourceFile, host, program); const packageJsonAutoImportProvider = host.getPackageJsonAutoImportProvider?.(); - const packageJsonFilter = detailsEntryId ? undefined : createPackageJsonImportFilter(sourceFile, host); + const packageJsonFilter = detailsEntryId ? undefined : createPackageJsonImportFilter(sourceFile, preferences, host); exportInfo.forEach((info, key) => { const symbolName = key.substring(0, key.indexOf("|")); if (!detailsEntryId && isStringANonContextualKeyword(symbolName)) return; @@ -1802,6 +1802,7 @@ namespace ts.Completions { info.isFromPackageJson ? packageJsonAutoImportProvider! : program, sourceFile, moduleFile, + preferences, packageJsonFilter, getModuleSpecifierResolutionHost(info.isFromPackageJson), moduleSpecifierCache); diff --git a/src/services/utilities.ts b/src/services/utilities.ts index d3ca113bd0ea4..e9b32df9949d9 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -2895,7 +2895,7 @@ namespace ts { allowsImportingSpecifier: (moduleSpecifier: string) => boolean; } - export function createPackageJsonImportFilter(fromFile: SourceFile, host: LanguageServiceHost): PackageJsonImportFilter { + export function createPackageJsonImportFilter(fromFile: SourceFile, preferences: UserPreferences, host: LanguageServiceHost): PackageJsonImportFilter { const packageJsons = ( (host.getPackageJsonsVisibleToFile && host.getPackageJsonsVisibleToFile(fromFile.fileName)) || getPackageJsonsVisibleToFile(fromFile.fileName, host) ).filter(p => p.parseable); @@ -2981,6 +2981,7 @@ namespace ts { fromFile.path, importedFileName, moduleSpecifierResolutionHost, + preferences, ); if (!specifier) { @@ -3283,14 +3284,14 @@ namespace ts { export function createModuleSpecifierCache(host: ModuleSpecifierResolutionCacheHost): ModuleSpecifierCache { let containedNodeModulesWatchers: ESMap | undefined; let cache: ESMap | undefined; - let importingFileName: Path | undefined; + let currentKey: string | undefined; const result: ModuleSpecifierCache = { - get(fromFileName, toFileName) { - if (!cache || fromFileName !== importingFileName) return undefined; + get(fromFileName, toFileName, preferences) { + if (!cache || currentKey !== key(fromFileName, preferences)) return undefined; return cache.get(toFileName); }, - set(fromFileName, toFileName, modulePaths, moduleSpecifiers) { - ensureCache(fromFileName).set(toFileName, createInfo(modulePaths, moduleSpecifiers, /*isAutoImportable*/ true)); + set(fromFileName, toFileName, preferences, modulePaths, moduleSpecifiers) { + ensureCache(fromFileName, preferences).set(toFileName, createInfo(modulePaths, moduleSpecifiers, /*isAutoImportable*/ true)); // If any module specifiers were generated based off paths in node_modules, // a package.json file in that package was read and is an input to the cached. @@ -3312,8 +3313,8 @@ namespace ts { } } }, - setModulePaths(fromFileName, toFileName, modulePaths) { - const cache = ensureCache(fromFileName); + setModulePaths(fromFileName, toFileName, preferences, modulePaths) { + const cache = ensureCache(fromFileName, preferences); const info = cache.get(toFileName); if (info) { info.modulePaths = modulePaths; @@ -3322,8 +3323,8 @@ namespace ts { cache.set(toFileName, createInfo(modulePaths, /*moduleSpecifiers*/ undefined, /*isAutoImportable*/ undefined)); } }, - setIsAutoImportable(fromFileName, toFileName, isAutoImportable) { - const cache = ensureCache(fromFileName); + setIsAutoImportable(fromFileName, toFileName, preferences, isAutoImportable) { + const cache = ensureCache(fromFileName, preferences); const info = cache.get(toFileName); if (info) { info.isAutoImportable = isAutoImportable; @@ -3336,7 +3337,7 @@ namespace ts { containedNodeModulesWatchers?.forEach(watcher => watcher.close()); cache?.clear(); containedNodeModulesWatchers?.clear(); - importingFileName = undefined; + currentKey = undefined; }, count() { return cache ? cache.size : 0; @@ -3347,14 +3348,19 @@ namespace ts { } return result; - function ensureCache(fromFileName: Path) { - if (cache && fromFileName !== importingFileName) { + function ensureCache(fromFileName: Path, preferences: UserPreferences) { + const newKey = key(fromFileName, preferences); + if (cache && (currentKey !== newKey)) { result.clear(); } - importingFileName = fromFileName; + currentKey = newKey; return cache ||= new Map(); } + function key(fromFileName: Path, preferences: UserPreferences) { + return `${fromFileName},${preferences.importModuleSpecifierEnding},${preferences.importModuleSpecifierPreference}`; + } + function createInfo( modulePaths: readonly ModulePath[] | undefined, moduleSpecifiers: readonly string[] | undefined, @@ -3368,12 +3374,13 @@ namespace ts { program: Program, from: SourceFile, to: SourceFile, + preferences: UserPreferences, packageJsonFilter: PackageJsonImportFilter | undefined, moduleSpecifierResolutionHost: ModuleSpecifierResolutionHost, moduleSpecifierCache: ModuleSpecifierCache | undefined, ): boolean { if (from === to) return false; - const cachedResult = moduleSpecifierCache?.get(from.path, to.path); + const cachedResult = moduleSpecifierCache?.get(from.path, to.path, preferences); if (cachedResult?.isAutoImportable !== undefined) { return cachedResult.isAutoImportable; } @@ -3396,7 +3403,7 @@ namespace ts { if (packageJsonFilter) { const isAutoImportable = hasImportablePath && packageJsonFilter.allowsImportingSourceFile(to, moduleSpecifierResolutionHost); - moduleSpecifierCache?.setIsAutoImportable(from.path, to.path, isAutoImportable); + moduleSpecifierCache?.setIsAutoImportable(from.path, to.path, preferences, isAutoImportable); return isAutoImportable; } diff --git a/src/testRunner/unittests/tsserver/moduleSpecifierCache.ts b/src/testRunner/unittests/tsserver/moduleSpecifierCache.ts index b29724fa6dccd..fd57438a2691e 100644 --- a/src/testRunner/unittests/tsserver/moduleSpecifierCache.ts +++ b/src/testRunner/unittests/tsserver/moduleSpecifierCache.ts @@ -35,14 +35,14 @@ namespace ts.projectSystem { describe("unittests:: tsserver:: moduleSpecifierCache", () => { it("caches importability within a file", () => { const { moduleSpecifierCache } = setup(); - assert.isTrue(moduleSpecifierCache.get(bTs.path as Path, aTs.path as Path)?.isAutoImportable); + assert.isTrue(moduleSpecifierCache.get(bTs.path as Path, aTs.path as Path, {})?.isAutoImportable); }); it("caches module specifiers within a file", () => { const { moduleSpecifierCache, triggerCompletions } = setup(); // Completion at an import statement will calculate and cache module specifiers triggerCompletions({ file: cTs.path, line: 1, offset: cTs.content.length + 1 }); - const mobxCache = moduleSpecifierCache.get(cTs.path as Path, mobxDts.path as Path); + const mobxCache = moduleSpecifierCache.get(cTs.path as Path, mobxDts.path as Path, {}); assert.deepEqual(mobxCache, { modulePaths: [{ path: mobxDts.path, @@ -68,7 +68,7 @@ namespace ts.projectSystem { const { host, moduleSpecifierCache } = setup(); host.writeFile("/src/a2.ts", aTs.content); host.runQueuedTimeoutCallbacks(); - assert.isTrue(moduleSpecifierCache.get(bTs.path as Path, aTs.path as Path)?.isAutoImportable); + assert.isTrue(moduleSpecifierCache.get(bTs.path as Path, aTs.path as Path, {})?.isAutoImportable); }); it("invalidates the cache when symlinks are added or removed", () => { @@ -94,20 +94,28 @@ namespace ts.projectSystem { it("invalidates the cache when user preferences change", () => { const { moduleSpecifierCache, session, triggerCompletions } = setup(); - executeSessionRequest(session, protocol.CommandTypes.Configure, { - preferences: { importModuleSpecifierPreference: "project-relative" }, - }); - assert.equal(moduleSpecifierCache.count(), 0); + const preferences: UserPreferences = { importModuleSpecifierPreference: "project-relative" }; - // Reset + assert.ok(getWithPreferences({})); + executeSessionRequest(session, protocol.CommandTypes.Configure, { preferences }); + // Nothing changes yet + assert.ok(getWithPreferences({})); + assert.isUndefined(getWithPreferences(preferences)); + // Completions will request (getting nothing) and set the cache with new preferences triggerCompletions({ file: bTs.path, line: 1, offset: 3 }); - assert.equal(moduleSpecifierCache.count(), 1); + assert.isUndefined(getWithPreferences({})); + assert.ok(getWithPreferences(preferences)); // Test other affecting preference executeSessionRequest(session, protocol.CommandTypes.Configure, { preferences: { importModuleSpecifierEnding: "js" }, }); - assert.equal(moduleSpecifierCache.count(), 0); + triggerCompletions({ file: bTs.path, line: 1, offset: 3 }); + assert.isUndefined(getWithPreferences(preferences)); + + function getWithPreferences(preferences: UserPreferences) { + return moduleSpecifierCache.get(bTs.path as Path, aTs.path as Path, preferences); + } }); }); From 4604dde1e7d905badde552c18bb19fafe07b2b85 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Tue, 8 Jun 2021 14:43:55 -0500 Subject: [PATCH 11/11] Fix missed merge conflict --- src/services/codefixes/importFixes.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 1ecef1d204c8e..a76e70c7b9fb2 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -531,12 +531,8 @@ namespace ts.codefix { return sort(fixes, (a, b) => compareValues(a.kind, b.kind) || compareModuleSpecifiers(a, b, allowsImportingSpecifier)); } -<<<<<<< HEAD - function getBestFix(fixes: readonly T[], sourceFile: SourceFile, host: LanguageServiceHost, preferences: UserPreferences): T { -======= - function getBestFix(fixes: readonly T[], sourceFile: SourceFile, host: LanguageServiceHost): T | undefined { + function getBestFix(fixes: readonly T[], sourceFile: SourceFile, host: LanguageServiceHost, preferences: UserPreferences): T | undefined { if (!some(fixes)) return; ->>>>>>> main // These will always be placed first if available, and are better than other kinds if (fixes[0].kind === ImportFixKind.UseNamespace || fixes[0].kind === ImportFixKind.AddToExisting) { return fixes[0];