From 2d5331edde612eb3c4936fa5e5dad143cde0bea0 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Thu, 2 Nov 2017 13:45:50 -0700 Subject: [PATCH] Handle cases when npm install doesnt get triggered with the actual file added Fixes #19597 --- src/compiler/resolutionCache.ts | 22 +++- src/harness/unittests/tscWatchMode.ts | 2 +- .../unittests/tsserverProjectSystem.ts | 111 ++++++++++++++++++ src/harness/virtualFileSystemWithWatch.ts | 20 +++- 4 files changed, 144 insertions(+), 11 deletions(-) diff --git a/src/compiler/resolutionCache.ts b/src/compiler/resolutionCache.ts index b988da0fd5f..edc66df8fa3 100644 --- a/src/compiler/resolutionCache.ts +++ b/src/compiler/resolutionCache.ts @@ -320,6 +320,10 @@ namespace ts { return endsWith(dirPath, "/node_modules"); } + function isNodeModulesAtTypesDirectory(dirPath: Path) { + return endsWith(dirPath, "/node_modules/@types"); + } + function isDirectoryAtleastAtLevelFromFSRoot(dirPath: Path, minLevels: number) { for (let searchIndex = getRootLength(dirPath); minLevels > 0; minLevels--) { searchIndex = dirPath.indexOf(directorySeparator, searchIndex) + 1; @@ -560,11 +564,21 @@ namespace ts { else { // Some file or directory in the watching directory is created // Return early if it does not have any of the watching extension or not the custom failed lookup path - if (!isPathWithDefaultFailedLookupExtension(fileOrDirectoryPath) && !customFailedLookupPaths.has(fileOrDirectoryPath)) { - return false; + const dirOfFileOrDirectory = getDirectoryPath(fileOrDirectoryPath); + if (isNodeModulesAtTypesDirectory(dirOfFileOrDirectory) || isNodeModulesDirectory(dirOfFileOrDirectory)) { + // Invalidate any resolution from this directory + isChangedFailedLookupLocation = location => { + const locationPath = resolutionHost.toPath(location); + return locationPath === fileOrDirectoryPath || startsWith(resolutionHost.toPath(location), fileOrDirectoryPath); + }; + } + else { + if (!isPathWithDefaultFailedLookupExtension(fileOrDirectoryPath) && !customFailedLookupPaths.has(fileOrDirectoryPath)) { + return false; + } + // Resolution need to be invalidated if failed lookup location is same as the file or directory getting created + isChangedFailedLookupLocation = location => resolutionHost.toPath(location) === fileOrDirectoryPath; } - // Resolution need to be invalidated if failed lookup location is same as the file or directory getting created - isChangedFailedLookupLocation = location => resolutionHost.toPath(location) === fileOrDirectoryPath; } const hasChangedFailedLookupLocation = (resolution: ResolutionWithFailedLookupLocations) => some(resolution.failedLookupLocations, isChangedFailedLookupLocation); const invalidatedFilesCount = filesWithInvalidatedResolutions && filesWithInvalidatedResolutions.size; diff --git a/src/harness/unittests/tscWatchMode.ts b/src/harness/unittests/tscWatchMode.ts index c5aba73f2ac..cddbfba9dc0 100644 --- a/src/harness/unittests/tscWatchMode.ts +++ b/src/harness/unittests/tscWatchMode.ts @@ -1988,7 +1988,7 @@ declare module "fs" { checkProgramActualFiles(watch(), mapDefined(files, f => f === configFile ? undefined : f.path)); file1.content = "var zz30 = 100;"; - host.reloadFS(files, /*invokeDirectoryWatcherInsteadOfFileChanged*/ true); + host.reloadFS(files, { invokeDirectoryWatcherInsteadOfFileChanged: true }); host.runQueuedTimeoutCallbacks(); checkProgramActualFiles(watch(), mapDefined(files, f => f === configFile ? undefined : f.path)); diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index bc8b476ceb9..be38176fb15 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -3445,6 +3445,117 @@ namespace ts.projectSystem { diags = session.executeCommand(getErrRequest).response as server.protocol.Diagnostic[]; verifyNoDiagnostics(diags); }); + + function assertEvent(actualOutput: string, expectedEvent: protocol.Event, host: TestServerHost) { + assert.equal(actualOutput, server.formatMessage(expectedEvent, nullLogger, Utils.byteLength, host.newLine)); + } + + function checkErrorMessage(host: TestServerHost, eventName: "syntaxDiag" | "semanticDiag", diagnostics: protocol.DiagnosticEventBody) { + const outputs = host.getOutput(); + assert.isTrue(outputs.length >= 1, outputs.toString()); + const event: protocol.Event = { + seq: 0, + type: "event", + event: eventName, + body: diagnostics + }; + assertEvent(outputs[0], event, host); + } + + function checkCompleteEvent(host: TestServerHost, numberOfCurrentEvents: number, expectedSequenceId: number) { + const outputs = host.getOutput(); + assert.equal(outputs.length, numberOfCurrentEvents, outputs.toString()); + const event: protocol.RequestCompletedEvent = { + seq: 0, + type: "event", + event: "requestCompleted", + body: { + request_seq: expectedSequenceId + } + }; + assertEvent(outputs[numberOfCurrentEvents - 1], event, host); + } + + function checkProjectUpdatedInBackgroundEvent(host: TestServerHost, openFiles: string[]) { + const outputs = host.getOutput(); + assert.equal(outputs.length, 1, outputs.toString()); + const event: protocol.ProjectsUpdatedInBackgroundEvent = { + seq: 0, + type: "event", + event: "projectsUpdatedInBackground", + body: { + openFiles + } + }; + assertEvent(outputs[0], event, host); + } + + it("npm install @types works", () => { + const folderPath = "/a/b/projects/temp"; + const file1: FileOrFolder = { + path: `${folderPath}/a.ts`, + content: 'import f = require("pad")' + }; + const files = [file1, libFile]; + const host = createServerHost(files); + const session = createSession(host, { canUseEvents: true }); + const service = session.getProjectService(); + session.executeCommandSeq({ + command: server.CommandNames.Open, + arguments: { + file: file1.path, + fileContent: file1.content, + scriptKindName: "TS", + projectRootPath: folderPath + } + }); + checkNumberOfProjects(service, { inferredProjects: 1 }); + host.clearOutput(); + const expectedSequenceId = session.getNextSeq(); + session.executeCommandSeq({ + command: server.CommandNames.Geterr, + arguments: { + delay: 0, + files: [file1.path] + } + }); + + host.checkTimeoutQueueLengthAndRun(1); + checkErrorMessage(host, "syntaxDiag", { file: file1.path, diagnostics: [] }); + host.clearOutput(); + + host.runQueuedImmediateCallbacks(); + const moduleNotFound = Diagnostics.Cannot_find_module_0; + const startOffset = file1.content.indexOf('"') + 1; + checkErrorMessage(host, "semanticDiag", { + file: file1.path, diagnostics: [{ + start: { line: 1, offset: startOffset }, + end: { line: 1, offset: startOffset + '"pad"'.length }, + text: formatStringFromArgs(moduleNotFound.message, ["pad"]), + code: moduleNotFound.code, + category: DiagnosticCategory[moduleNotFound.category].toLowerCase() + }] + }); + checkCompleteEvent(host, 2, expectedSequenceId); + host.clearOutput(); + + const padIndex: FileOrFolder = { + path: `${folderPath}/node_modules/@types/pad/index.d.ts`, + content: "export = pad;declare function pad(length: number, text: string, char ?: string): string;" + }; + files.push(padIndex); + host.reloadFS(files, { ignoreWatchInvokedWithTriggerAsFileCreate: true }); + host.runQueuedTimeoutCallbacks(); + checkProjectUpdatedInBackgroundEvent(host, [file1.path]); + host.clearOutput(); + + host.runQueuedTimeoutCallbacks(); + checkErrorMessage(host, "syntaxDiag", { file: file1.path, diagnostics: [] }); + host.clearOutput(); + + host.runQueuedImmediateCallbacks(); + checkErrorMessage(host, "semanticDiag", { file: file1.path, diagnostics: [] }); + }); }); describe("Configure file diagnostics events", () => { diff --git a/src/harness/virtualFileSystemWithWatch.ts b/src/harness/virtualFileSystemWithWatch.ts index d6b1fc1a771..fe40cb42844 100644 --- a/src/harness/virtualFileSystemWithWatch.ts +++ b/src/harness/virtualFileSystemWithWatch.ts @@ -226,6 +226,11 @@ interface Array {}` directoryName: string; } + export interface ReloadWatchInvokeOptions { + invokeDirectoryWatcherInsteadOfFileChanged: boolean; + ignoreWatchInvokedWithTriggerAsFileCreate: boolean; + } + export class TestServerHost implements server.ServerHost, FormatDiagnosticsHost { args: string[] = []; @@ -270,7 +275,7 @@ interface Array {}` return s; } - reloadFS(fileOrFolderList: ReadonlyArray, invokeDirectoryWatcherInsteadOfFileChanged?: boolean) { + reloadFS(fileOrFolderList: ReadonlyArray, options?: Partial) { const mapNewLeaves = createMap(); const isNewFs = this.fs.size === 0; fileOrFolderList = fileOrFolderList.concat(this.withSafeList ? safeList : []); @@ -291,7 +296,7 @@ interface Array {}` // Update file if (currentEntry.content !== fileOrDirectory.content) { currentEntry.content = fileOrDirectory.content; - if (invokeDirectoryWatcherInsteadOfFileChanged) { + if (options && options.invokeDirectoryWatcherInsteadOfFileChanged) { this.invokeDirectoryWatcher(getDirectoryPath(currentEntry.fullPath), currentEntry.fullPath); } else { @@ -314,7 +319,7 @@ interface Array {}` } } else { - this.ensureFileOrFolder(fileOrDirectory); + this.ensureFileOrFolder(fileOrDirectory, options && options.ignoreWatchInvokedWithTriggerAsFileCreate); } } @@ -331,12 +336,12 @@ interface Array {}` } } - ensureFileOrFolder(fileOrDirectory: FileOrFolder) { + ensureFileOrFolder(fileOrDirectory: FileOrFolder, ignoreWatchInvokedWithTriggerAsFileCreate?: boolean) { if (isString(fileOrDirectory.content)) { const file = this.toFile(fileOrDirectory); Debug.assert(!this.fs.get(file.path)); const baseFolder = this.ensureFolder(getDirectoryPath(file.fullPath)); - this.addFileOrFolderInFolder(baseFolder, file); + this.addFileOrFolderInFolder(baseFolder, file, ignoreWatchInvokedWithTriggerAsFileCreate); } else { const fullPath = getNormalizedAbsolutePath(fileOrDirectory.path, this.currentDirectory); @@ -365,10 +370,13 @@ interface Array {}` return folder; } - private addFileOrFolderInFolder(folder: Folder, fileOrDirectory: File | Folder) { + private addFileOrFolderInFolder(folder: Folder, fileOrDirectory: File | Folder, ignoreWatch?: boolean) { folder.entries.push(fileOrDirectory); this.fs.set(fileOrDirectory.path, fileOrDirectory); + if (ignoreWatch) { + return; + } if (isFile(fileOrDirectory)) { this.invokeFileWatcher(fileOrDirectory.fullPath, FileWatcherEventKind.Created); }