From 60b19f5782b87af71a2ca567da9c3f9d50638df2 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Thu, 12 Apr 2018 16:47:40 -0700 Subject: [PATCH] Invalidate the unresolved import resolutions when typing files are set This has 3 changes: 1. In updateGraph when enqueue the typing installation request (depending on unresolved imports) 2. When ActionSet event is received, invalidate only files with unresolved imports and resolve those. 3. When ActionInvalidate event is received, typing installer has detected some change in global typing cache location, so just enqueue a new typing installation request. This will repeat the cycle of setting correct typings and pickiing unresolved imports --- src/compiler/resolutionCache.ts | 20 ++++- .../unittests/tsserverProjectSystem.ts | 1 - src/harness/unittests/typingsInstaller.ts | 75 ++++++++++++++++++- src/server/editorServices.ts | 10 +-- src/server/project.ts | 71 ++++++++++++------ src/server/typingsCache.ts | 16 ++-- .../reference/api/tsserverlibrary.d.ts | 5 +- 7 files changed, 153 insertions(+), 45 deletions(-) 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;