diff --git a/src/compiler/resolutionCache.ts b/src/compiler/resolutionCache.ts index c2b96649cf2..2cd39068de1 100644 --- a/src/compiler/resolutionCache.ts +++ b/src/compiler/resolutionCache.ts @@ -10,6 +10,7 @@ namespace ts { invalidateResolutionOfFile(filePath: Path): void; removeResolutionsOfFile(filePath: Path): void; + setFilesWithInvalidatedNonRelativeUnresolvedImports(filesWithUnresolvedImports: Map): void; createHasInvalidatedResolution(forceAllFilesAsInvalidated?: boolean): HasInvalidatedResolution; startCachingPerDirectoryResolution(): void; @@ -74,6 +75,7 @@ namespace ts { export function createResolutionCache(resolutionHost: ResolutionCacheHost, rootDirForResolution: string, logChangesWhenResolvingModule: boolean): ResolutionCache { let filesWithChangedSetOfUnresolvedImports: Path[] | undefined; let filesWithInvalidatedResolutions: Map | undefined; + let filesWithInvalidatedNonRelativeUnresolvedImports: Map | undefined; let allFilesHaveInvalidatedResolution = false; const getCurrentDirectory = memoize(() => resolutionHost.getCurrentDirectory()); @@ -122,6 +124,7 @@ namespace ts { resolveTypeReferenceDirectives, removeResolutionsOfFile, invalidateResolutionOfFile, + setFilesWithInvalidatedNonRelativeUnresolvedImports, createHasInvalidatedResolution, updateTypeRootsWatch, closeTypeRootsWatch, @@ -173,7 +176,8 @@ namespace ts { } const collected = filesWithInvalidatedResolutions; filesWithInvalidatedResolutions = undefined; - return path => collected && collected.has(path); + return path => (collected && collected.has(path)) || + (filesWithInvalidatedNonRelativeUnresolvedImports && filesWithInvalidatedNonRelativeUnresolvedImports.has(path)); } function clearPerDirectoryResolutions() { @@ -184,6 +188,7 @@ namespace ts { function finishCachingPerDirectoryResolution() { allFilesHaveInvalidatedResolution = false; + filesWithInvalidatedNonRelativeUnresolvedImports = undefined; directoryWatchesOfFailedLookups.forEach((watcher, path) => { if (watcher.refCount === 0) { directoryWatchesOfFailedLookups.delete(path); @@ -237,13 +242,15 @@ namespace ts { const resolvedModules: R[] = []; const compilerOptions = resolutionHost.getCompilationSettings(); - + const hasInvalidatedNonRelativeUnresolvedImport = logChanges && filesWithInvalidatedNonRelativeUnresolvedImports && filesWithInvalidatedNonRelativeUnresolvedImports.has(path); const seenNamesInFile = createMap(); for (const name of names) { let resolution = resolutionsInFile.get(name); // Resolution is valid if it is present and not invalidated if (!seenNamesInFile.has(name) && - allFilesHaveInvalidatedResolution || !resolution || resolution.isInvalidated) { + allFilesHaveInvalidatedResolution || !resolution || resolution.isInvalidated || + // If the name is unresolved import that was invalidated, recalculate + (hasInvalidatedNonRelativeUnresolvedImport && !isExternalModuleNameRelative(name) && !getResolutionWithResolvedFileName(resolution))) { const existingResolution = resolution; const resolutionInDirectory = perDirectoryResolution.get(name); if (resolutionInDirectory) { @@ -284,7 +291,7 @@ namespace ts { if (oldResolution === newResolution) { return true; } - if (!oldResolution || !newResolution || oldResolution.isInvalidated) { + if (!oldResolution || !newResolution) { return false; } const oldResult = getResolutionWithResolvedFileName(oldResolution); @@ -577,6 +584,11 @@ namespace ts { ); } + function setFilesWithInvalidatedNonRelativeUnresolvedImports(filesMap: Map) { + Debug.assert(filesWithInvalidatedNonRelativeUnresolvedImports === filesMap || filesWithInvalidatedNonRelativeUnresolvedImports === undefined); + filesWithInvalidatedNonRelativeUnresolvedImports = filesMap; + } + function invalidateResolutionOfFailedLookupLocation(fileOrDirectoryPath: Path, isCreatingWatchedDirectory: boolean) { let isChangedFailedLookupLocation: (location: string) => boolean; if (isCreatingWatchedDirectory) { diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index b167ed21b94..10912b4fb87 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -7294,7 +7294,6 @@ namespace ts.projectSystem { const host = createServerHost(files); const session = createSession(host); const projectService = session.getProjectService(); - debugger; session.executeCommandSeq({ command: protocol.CommandTypes.Open, arguments: { diff --git a/src/harness/unittests/typingsInstaller.ts b/src/harness/unittests/typingsInstaller.ts index 5ec7a9d7a17..b6d5a20ef9a 100644 --- a/src/harness/unittests/typingsInstaller.ts +++ b/src/harness/unittests/typingsInstaller.ts @@ -1006,7 +1006,7 @@ namespace ts.projectSystem { installer.installAll(/*expectedCount*/ 1); }); - it("should recompute resolutions after typings are installed", () => { + it("cached unresolved typings are not recomputed if program structure did not change", () => { const host = createServerHost([]); const session = createSession(host); const f = { @@ -1048,7 +1048,7 @@ namespace ts.projectSystem { session.executeCommand(changeRequest); host.checkTimeoutQueueLengthAndRun(2); // This enqueues the updategraph and refresh inferred projects const version2 = proj.lastCachedUnresolvedImportsList; - assert.notEqual(version1, version2, "set of unresolved imports should change"); + assert.strictEqual(version1, version2, "set of unresolved imports should change"); }); it("expired cache entry (inferred project, should install typings)", () => { @@ -1621,4 +1621,75 @@ namespace ts.projectSystem { assert.deepEqual(commands, expectedCommands, "commands"); }); }); + + describe("recomputing resolutions of unresolved imports", () => { + const globalTypingsCacheLocation = "/tmp"; + const appPath = "/a/b/app.js" as Path; + const foooPath = "/a/b/node_modules/fooo/index.d.ts"; + function verifyResolvedModuleOfFooo(project: server.Project) { + const foooResolution = project.getLanguageService().getProgram().getSourceFileByPath(appPath).resolvedModules.get("fooo"); + assert.equal(foooResolution.resolvedFileName, foooPath); + return foooResolution; + } + + function verifyUnresolvedImportResolutions(appContents: string, typingNames: string[], typingFiles: FileOrFolder[]) { + const app: FileOrFolder = { + path: appPath, + content: `${appContents}import * as x from "fooo";` + }; + const fooo: FileOrFolder = { + path: foooPath, + content: `export var x: string;` + }; + const host = createServerHost([app, fooo]); + const installer = new (class extends Installer { + constructor() { + super(host, { globalTypingsCacheLocation, typesRegistry: createTypesRegistry("foo") }); + } + installWorker(_requestId: number, _args: string[], _cwd: string, cb: TI.RequestCompletedAction) { + executeCommand(this, host, typingNames, typingFiles, cb); + } + })(); + const projectService = createProjectService(host, { typingsInstaller: installer }); + projectService.openClientFile(app.path); + projectService.checkNumberOfProjects({ inferredProjects: 1 }); + + const proj = projectService.inferredProjects[0]; + checkProjectActualFiles(proj, [app.path, fooo.path]); + const foooResolution1 = verifyResolvedModuleOfFooo(proj); + + installer.installAll(/*expectedCount*/ 1); + host.checkTimeoutQueueLengthAndRun(2); + checkProjectActualFiles(proj, typingFiles.map(f => f.path).concat(app.path, fooo.path)); + const foooResolution2 = verifyResolvedModuleOfFooo(proj); + assert.strictEqual(foooResolution1, foooResolution2); + } + + it("correctly invalidate the resolutions with typing names", () => { + verifyUnresolvedImportResolutions('import * as a from "foo";', ["foo"], [{ + path: `${globalTypingsCacheLocation}/node_modules/foo/index.d.ts`, + content: "export function a(): void;" + }]); + }); + + it("correctly invalidate the resolutions with typing names that are trimmed", () => { + const fooAA: FileOrFolder = { + path: `${globalTypingsCacheLocation}/node_modules/foo/a/a.d.ts`, + content: "export function a (): void;" + }; + const fooAB: FileOrFolder = { + path: `${globalTypingsCacheLocation}/node_modules/foo/a/b.d.ts`, + content: "export function b (): void;" + }; + const fooAC: FileOrFolder = { + path: `${globalTypingsCacheLocation}/node_modules/foo/a/c.d.ts`, + content: "export function c (): void;" + }; + verifyUnresolvedImportResolutions(` + import * as a from "foo/a/a"; + import * as b from "foo/a/b"; + import * as c from "foo/a/c"; + `, ["foo"], [fooAA, fooAB, fooAC]); + }); + }); } diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index dc2fa086a93..1271b98c1c7 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -524,13 +524,13 @@ namespace ts.server { } switch (response.kind) { case ActionSet: - project.resolutionCache.clear(); - this.typingsCache.updateTypingsForProject(response.projectName, response.compilerOptions, response.typeAcquisition, response.unresolvedImports, response.typings); + // Update the typing files and update the project + project.updateTypingFiles(this.typingsCache.updateTypingsForProject(response.projectName, response.compilerOptions, response.typeAcquisition, response.unresolvedImports, response.typings)); break; case ActionInvalidate: - project.resolutionCache.clear(); - this.typingsCache.deleteTypingsForProject(response.projectName); - break; + // Do not clear resolution cache, there was changes detected in typings, so enque typing request and let it get us correct results + this.typingsCache.enqueueInstallTypingsForProject(project, project.lastCachedUnresolvedImportsList, /*forceRefresh*/ true); + return; } this.delayUpdateProjectGraphAndEnsureProjectStructureForOpenFiles(project); } diff --git a/src/server/project.ts b/src/server/project.ts index 47bfb03a189..67469cc69d8 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -89,7 +89,18 @@ namespace ts.server { private plugins: PluginModule[] = []; /*@internal*/ + /** + * This is map from files to unresolved imports in it + * Maop does not contain entries for files that do not have unresolved imports + * This helps in containing the set of files to invalidate + */ cachedUnresolvedImportsPerFile = createMap>(); + + /** + * This is the set that has entry to true if file doesnt contain any unresolved import + */ + private filesWithNoUnresolvedImports = createMap(); + /*@internal*/ lastCachedUnresolvedImportsList: SortedReadonlyArray; /*@internal*/ @@ -143,7 +154,8 @@ namespace ts.server { /*@internal*/ hasChangedAutomaticTypeDirectiveNames = false; - private typingFiles: SortedReadonlyArray; + /*@internal*/ + typingFiles: SortedReadonlyArray = emptyArray; private readonly cancellationToken: ThrottledCancellationToken; @@ -554,6 +566,7 @@ namespace ts.server { this.resolutionCache.clear(); this.resolutionCache = undefined; this.cachedUnresolvedImportsPerFile = undefined; + this.filesWithNoUnresolvedImports = undefined; this.directoryStructureHost = undefined; // Clean up file watchers waiting for missing files @@ -714,6 +727,7 @@ namespace ts.server { else { this.resolutionCache.invalidateResolutionOfFile(info.path); } + this.filesWithNoUnresolvedImports.delete(info.path); this.cachedUnresolvedImportsPerFile.delete(info.path); if (detachFromProject) { @@ -735,16 +749,21 @@ namespace ts.server { } /* @internal */ - private extractUnresolvedImportsFromSourceFile(file: SourceFile, result: Push, ambientModules: string[]) { + private extractUnresolvedImportsFromSourceFile(file: SourceFile, result: string[] | undefined, ambientModules: string[]): string[] | undefined { + // No unresolve imports in this file + if (this.filesWithNoUnresolvedImports.has(file.path)) { + return result; + } + const cached = this.cachedUnresolvedImportsPerFile.get(file.path); if (cached) { // found cached result - use it and return for (const f of cached) { - result.push(f); + (result || (result = [])).push(f); } - return; + return result; } - let unresolvedImports: string[]; + let unresolvedImports: string[] | undefined; if (file.resolvedModules) { file.resolvedModules.forEach((resolvedModule, name) => { // pick unresolved non-relative names @@ -760,11 +779,17 @@ namespace ts.server { trimmed = trimmed.substr(0, i); } (unresolvedImports || (unresolvedImports = [])).push(trimmed); - result.push(trimmed); + (result || (result = [])).push(trimmed); } }); } - this.cachedUnresolvedImportsPerFile.set(file.path, unresolvedImports || emptyArray); + if (unresolvedImports) { + this.cachedUnresolvedImportsPerFile.set(file.path, unresolvedImports); + } + else { + this.filesWithNoUnresolvedImports.set(file.path, true); + } + return result; function isAmbientlyDeclaredModule(name: string) { return ambientModules.some(m => m === name); @@ -778,7 +803,7 @@ namespace ts.server { updateGraph(): boolean { this.resolutionCache.startRecordingFilesWithChangedResolutions(); - let hasChanges = this.updateGraphWorker(); + const hasChanges = this.updateGraphWorker(); const hasMoreOrLessScriptInfos = this.hasMoreOrLessScriptInfos; this.hasMoreOrLessScriptInfos = false; @@ -787,6 +812,7 @@ namespace ts.server { for (const file of changedFiles) { // delete cached information for changed files this.cachedUnresolvedImportsPerFile.delete(file); + this.filesWithNoUnresolvedImports.delete(file); } // update builder only if language service is enabled @@ -799,20 +825,15 @@ namespace ts.server { // (can reuse cached imports for files that were not changed) // 4. compilation settings were changed in the way that might affect module resolution - drop all caches and collect all data from the scratch if (hasChanges || changedFiles.length) { - const result: string[] = []; + let result: string[] | undefined; const ambientModules = this.program.getTypeChecker().getAmbientModules().map(mod => stripQuotes(mod.getName())); for (const sourceFile of this.program.getSourceFiles()) { - this.extractUnresolvedImportsFromSourceFile(sourceFile, result, ambientModules); + result = this.extractUnresolvedImportsFromSourceFile(sourceFile, result, ambientModules); } - this.lastCachedUnresolvedImportsList = toDeduplicatedSortedArray(result); + this.lastCachedUnresolvedImportsList = result ? toDeduplicatedSortedArray(result) : emptyArray; } - const cachedTypings = this.projectService.typingsCache.getTypingsForProject(this, this.lastCachedUnresolvedImportsList, hasMoreOrLessScriptInfos); - if (!arrayIsEqualTo(this.typingFiles, cachedTypings)) { - this.typingFiles = cachedTypings; - this.markAsDirty(); - hasChanges = this.updateGraphWorker() || hasChanges; - } + this.projectService.typingsCache.enqueueInstallTypingsForProject(this, this.lastCachedUnresolvedImportsList, hasMoreOrLessScriptInfos); } else { this.lastCachedUnresolvedImportsList = undefined; @@ -824,6 +845,13 @@ namespace ts.server { return !hasChanges; } + /*@internal*/ + updateTypingFiles(typingFiles: SortedReadonlyArray) { + this.typingFiles = typingFiles; + // Invalidate files with unresolved imports + this.resolutionCache.setFilesWithInvalidatedNonRelativeUnresolvedImports(this.cachedUnresolvedImportsPerFile); + } + /* @internal */ getCurrentProgram() { return this.program; @@ -959,15 +987,14 @@ namespace ts.server { setCompilerOptions(compilerOptions: CompilerOptions) { if (compilerOptions) { compilerOptions.allowNonTsExtensions = true; - if (changesAffectModuleResolution(this.compilerOptions, compilerOptions)) { - // reset cached unresolved imports if changes in compiler options affected module resolution - this.cachedUnresolvedImportsPerFile.clear(); - this.lastCachedUnresolvedImportsList = undefined; - } const oldOptions = this.compilerOptions; this.compilerOptions = compilerOptions; this.setInternalCompilerOptionsForEmittingJsFiles(); if (changesAffectModuleResolution(oldOptions, compilerOptions)) { + // reset cached unresolved imports if changes in compiler options affected module resolution + this.cachedUnresolvedImportsPerFile.clear(); + this.filesWithNoUnresolvedImports.clear(); + this.lastCachedUnresolvedImportsList = undefined; this.resolutionCache.clear(); } this.markAsDirty(); diff --git a/src/server/typingsCache.ts b/src/server/typingsCache.ts index 6418b5f9cbe..c255757481f 100644 --- a/src/server/typingsCache.ts +++ b/src/server/typingsCache.ts @@ -95,15 +95,14 @@ namespace ts.server { return this.installer.installPackage(options); } - getTypingsForProject(project: Project, unresolvedImports: SortedReadonlyArray, forceRefresh: boolean): SortedReadonlyArray { + enqueueInstallTypingsForProject(project: Project, unresolvedImports: SortedReadonlyArray, forceRefresh: boolean) { const typeAcquisition = project.getTypeAcquisition(); if (!typeAcquisition || !typeAcquisition.enable) { - return emptyArray; + return; } const entry = this.perProjectCache.get(project.getProjectName()); - const result: SortedReadonlyArray = entry ? entry.typings : emptyArray; if (forceRefresh || !entry || typeAcquisitionChanged(typeAcquisition, entry.typeAcquisition) || @@ -114,28 +113,25 @@ namespace ts.server { this.perProjectCache.set(project.getProjectName(), { compilerOptions: project.getCompilationSettings(), typeAcquisition, - typings: result, + typings: entry ? entry.typings : emptyArray, unresolvedImports, poisoned: true }); // something has been changed, issue a request to update typings this.installer.enqueueInstallTypingsRequest(project, typeAcquisition, unresolvedImports); } - return result; } updateTypingsForProject(projectName: string, compilerOptions: CompilerOptions, typeAcquisition: TypeAcquisition, unresolvedImports: SortedReadonlyArray, newTypings: string[]) { + const typings = toSortedArray(newTypings); this.perProjectCache.set(projectName, { compilerOptions, typeAcquisition, - typings: toSortedArray(newTypings), + typings, unresolvedImports, poisoned: false }); - } - - deleteTypingsForProject(projectName: string) { - this.perProjectCache.delete(projectName); + return !typeAcquisition || !typeAcquisition.enable ? emptyArray : typings; } onProjectClosed(project: Project) { diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index 29959a89d02..53c23b1d382 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -7643,6 +7643,10 @@ declare namespace ts.server { private externalFiles; private missingFilesMap; private plugins; + /** + * This is the set that has entry to true if file doesnt contain any unresolved import + */ + private filesWithNoUnresolvedImports; private lastFileExceededProgramSize; protected languageService: LanguageService; languageServiceEnabled: boolean; @@ -7673,7 +7677,6 @@ declare namespace ts.server { * This property is different from projectStructureVersion since in most cases edits don't affect set of files in the project */ private projectStateVersion; - private typingFiles; private readonly cancellationToken; isNonTsProject(): boolean; isJsOnlyProject(): boolean;