Skip to content

Commit 85285bc

Browse files
sheetalkamatsheetalkamat
authored
Merge pull request microsoft#19688 from Microsoft/npmInstallAtTypes
Handle cases when npm install doesnt get triggered with actual failed lookup location but instead the trigger is some folder in the node_modules
2 parents 9623257 + 2d5331e commit 85285bc

4 files changed

Lines changed: 144 additions & 11 deletions

File tree

‎src/compiler/resolutionCache.ts‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,10 @@ namespace ts {
320320
return endsWith(dirPath, "/node_modules");
321321
}
322322

323+
function isNodeModulesAtTypesDirectory(dirPath: Path) {
324+
return endsWith(dirPath, "/node_modules/@types");
325+
}
326+
323327
function isDirectoryAtleastAtLevelFromFSRoot(dirPath: Path, minLevels: number) {
324328
for (let searchIndex = getRootLength(dirPath); minLevels > 0; minLevels--) {
325329
searchIndex = dirPath.indexOf(directorySeparator, searchIndex) + 1;
@@ -560,11 +564,21 @@ namespace ts {
560564
else {
561565
// Some file or directory in the watching directory is created
562566
// Return early if it does not have any of the watching extension or not the custom failed lookup path
563-
if (!isPathWithDefaultFailedLookupExtension(fileOrDirectoryPath) && !customFailedLookupPaths.has(fileOrDirectoryPath)) {
564-
return false;
567+
const dirOfFileOrDirectory = getDirectoryPath(fileOrDirectoryPath);
568+
if (isNodeModulesAtTypesDirectory(dirOfFileOrDirectory) || isNodeModulesDirectory(dirOfFileOrDirectory)) {
569+
// Invalidate any resolution from this directory
570+
isChangedFailedLookupLocation = location => {
571+
const locationPath = resolutionHost.toPath(location);
572+
return locationPath === fileOrDirectoryPath || startsWith(resolutionHost.toPath(location), fileOrDirectoryPath);
573+
};
574+
}
575+
else {
576+
if (!isPathWithDefaultFailedLookupExtension(fileOrDirectoryPath) && !customFailedLookupPaths.has(fileOrDirectoryPath)) {
577+
return false;
578+
}
579+
// Resolution need to be invalidated if failed lookup location is same as the file or directory getting created
580+
isChangedFailedLookupLocation = location => resolutionHost.toPath(location) === fileOrDirectoryPath;
565581
}
566-
// Resolution need to be invalidated if failed lookup location is same as the file or directory getting created
567-
isChangedFailedLookupLocation = location => resolutionHost.toPath(location) === fileOrDirectoryPath;
568582
}
569583
const hasChangedFailedLookupLocation = (resolution: ResolutionWithFailedLookupLocations) => some(resolution.failedLookupLocations, isChangedFailedLookupLocation);
570584
const invalidatedFilesCount = filesWithInvalidatedResolutions && filesWithInvalidatedResolutions.size;

‎src/harness/unittests/tscWatchMode.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1988,7 +1988,7 @@ declare module "fs" {
19881988

19891989
checkProgramActualFiles(watch(), mapDefined(files, f => f === configFile ? undefined : f.path));
19901990
file1.content = "var zz30 = 100;";
1991-
host.reloadFS(files, /*invokeDirectoryWatcherInsteadOfFileChanged*/ true);
1991+
host.reloadFS(files, { invokeDirectoryWatcherInsteadOfFileChanged: true });
19921992
host.runQueuedTimeoutCallbacks();
19931993

19941994
checkProgramActualFiles(watch(), mapDefined(files, f => f === configFile ? undefined : f.path));

‎src/harness/unittests/tsserverProjectSystem.ts‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3445,6 +3445,117 @@ namespace ts.projectSystem {
34453445
diags = session.executeCommand(getErrRequest).response as server.protocol.Diagnostic[];
34463446
verifyNoDiagnostics(diags);
34473447
});
3448+
3449+
function assertEvent(actualOutput: string, expectedEvent: protocol.Event, host: TestServerHost) {
3450+
assert.equal(actualOutput, server.formatMessage(expectedEvent, nullLogger, Utils.byteLength, host.newLine));
3451+
}
3452+
3453+
function checkErrorMessage(host: TestServerHost, eventName: "syntaxDiag" | "semanticDiag", diagnostics: protocol.DiagnosticEventBody) {
3454+
const outputs = host.getOutput();
3455+
assert.isTrue(outputs.length >= 1, outputs.toString());
3456+
const event: protocol.Event = {
3457+
seq: 0,
3458+
type: "event",
3459+
event: eventName,
3460+
body: diagnostics
3461+
};
3462+
assertEvent(outputs[0], event, host);
3463+
}
3464+
3465+
function checkCompleteEvent(host: TestServerHost, numberOfCurrentEvents: number, expectedSequenceId: number) {
3466+
const outputs = host.getOutput();
3467+
assert.equal(outputs.length, numberOfCurrentEvents, outputs.toString());
3468+
const event: protocol.RequestCompletedEvent = {
3469+
seq: 0,
3470+
type: "event",
3471+
event: "requestCompleted",
3472+
body: {
3473+
request_seq: expectedSequenceId
3474+
}
3475+
};
3476+
assertEvent(outputs[numberOfCurrentEvents - 1], event, host);
3477+
}
3478+
3479+
function checkProjectUpdatedInBackgroundEvent(host: TestServerHost, openFiles: string[]) {
3480+
const outputs = host.getOutput();
3481+
assert.equal(outputs.length, 1, outputs.toString());
3482+
const event: protocol.ProjectsUpdatedInBackgroundEvent = {
3483+
seq: 0,
3484+
type: "event",
3485+
event: "projectsUpdatedInBackground",
3486+
body: {
3487+
openFiles
3488+
}
3489+
};
3490+
assertEvent(outputs[0], event, host);
3491+
}
3492+
3493+
it("npm install @types works", () => {
3494+
const folderPath = "/a/b/projects/temp";
3495+
const file1: FileOrFolder = {
3496+
path: `${folderPath}/a.ts`,
3497+
content: 'import f = require("pad")'
3498+
};
3499+
const files = [file1, libFile];
3500+
const host = createServerHost(files);
3501+
const session = createSession(host, { canUseEvents: true });
3502+
const service = session.getProjectService();
3503+
session.executeCommandSeq<protocol.OpenRequest>({
3504+
command: server.CommandNames.Open,
3505+
arguments: {
3506+
file: file1.path,
3507+
fileContent: file1.content,
3508+
scriptKindName: "TS",
3509+
projectRootPath: folderPath
3510+
}
3511+
});
3512+
checkNumberOfProjects(service, { inferredProjects: 1 });
3513+
host.clearOutput();
3514+
const expectedSequenceId = session.getNextSeq();
3515+
session.executeCommandSeq<protocol.GeterrRequest>({
3516+
command: server.CommandNames.Geterr,
3517+
arguments: {
3518+
delay: 0,
3519+
files: [file1.path]
3520+
}
3521+
});
3522+
3523+
host.checkTimeoutQueueLengthAndRun(1);
3524+
checkErrorMessage(host, "syntaxDiag", { file: file1.path, diagnostics: [] });
3525+
host.clearOutput();
3526+
3527+
host.runQueuedImmediateCallbacks();
3528+
const moduleNotFound = Diagnostics.Cannot_find_module_0;
3529+
const startOffset = file1.content.indexOf('"') + 1;
3530+
checkErrorMessage(host, "semanticDiag", {
3531+
file: file1.path, diagnostics: [{
3532+
start: { line: 1, offset: startOffset },
3533+
end: { line: 1, offset: startOffset + '"pad"'.length },
3534+
text: formatStringFromArgs(moduleNotFound.message, ["pad"]),
3535+
code: moduleNotFound.code,
3536+
category: DiagnosticCategory[moduleNotFound.category].toLowerCase()
3537+
}]
3538+
});
3539+
checkCompleteEvent(host, 2, expectedSequenceId);
3540+
host.clearOutput();
3541+
3542+
const padIndex: FileOrFolder = {
3543+
path: `${folderPath}/node_modules/@types/pad/index.d.ts`,
3544+
content: "export = pad;declare function pad(length: number, text: string, char ?: string): string;"
3545+
};
3546+
files.push(padIndex);
3547+
host.reloadFS(files, { ignoreWatchInvokedWithTriggerAsFileCreate: true });
3548+
host.runQueuedTimeoutCallbacks();
3549+
checkProjectUpdatedInBackgroundEvent(host, [file1.path]);
3550+
host.clearOutput();
3551+
3552+
host.runQueuedTimeoutCallbacks();
3553+
checkErrorMessage(host, "syntaxDiag", { file: file1.path, diagnostics: [] });
3554+
host.clearOutput();
3555+
3556+
host.runQueuedImmediateCallbacks();
3557+
checkErrorMessage(host, "semanticDiag", { file: file1.path, diagnostics: [] });
3558+
});
34483559
});
34493560

34503561
describe("Configure file diagnostics events", () => {

‎src/harness/virtualFileSystemWithWatch.ts‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,11 @@ interface Array<T> {}`
226226
directoryName: string;
227227
}
228228

229+
export interface ReloadWatchInvokeOptions {
230+
invokeDirectoryWatcherInsteadOfFileChanged: boolean;
231+
ignoreWatchInvokedWithTriggerAsFileCreate: boolean;
232+
}
233+
229234
export class TestServerHost implements server.ServerHost, FormatDiagnosticsHost {
230235
args: string[] = [];
231236

@@ -270,7 +275,7 @@ interface Array<T> {}`
270275
return s;
271276
}
272277

273-
reloadFS(fileOrFolderList: ReadonlyArray<FileOrFolder>, invokeDirectoryWatcherInsteadOfFileChanged?: boolean) {
278+
reloadFS(fileOrFolderList: ReadonlyArray<FileOrFolder>, options?: Partial<ReloadWatchInvokeOptions>) {
274279
const mapNewLeaves = createMap<true>();
275280
const isNewFs = this.fs.size === 0;
276281
fileOrFolderList = fileOrFolderList.concat(this.withSafeList ? safeList : []);
@@ -291,7 +296,7 @@ interface Array<T> {}`
291296
// Update file
292297
if (currentEntry.content !== fileOrDirectory.content) {
293298
currentEntry.content = fileOrDirectory.content;
294-
if (invokeDirectoryWatcherInsteadOfFileChanged) {
299+
if (options && options.invokeDirectoryWatcherInsteadOfFileChanged) {
295300
this.invokeDirectoryWatcher(getDirectoryPath(currentEntry.fullPath), currentEntry.fullPath);
296301
}
297302
else {
@@ -314,7 +319,7 @@ interface Array<T> {}`
314319
}
315320
}
316321
else {
317-
this.ensureFileOrFolder(fileOrDirectory);
322+
this.ensureFileOrFolder(fileOrDirectory, options && options.ignoreWatchInvokedWithTriggerAsFileCreate);
318323
}
319324
}
320325

@@ -331,12 +336,12 @@ interface Array<T> {}`
331336
}
332337
}
333338

334-
ensureFileOrFolder(fileOrDirectory: FileOrFolder) {
339+
ensureFileOrFolder(fileOrDirectory: FileOrFolder, ignoreWatchInvokedWithTriggerAsFileCreate?: boolean) {
335340
if (isString(fileOrDirectory.content)) {
336341
const file = this.toFile(fileOrDirectory);
337342
Debug.assert(!this.fs.get(file.path));
338343
const baseFolder = this.ensureFolder(getDirectoryPath(file.fullPath));
339-
this.addFileOrFolderInFolder(baseFolder, file);
344+
this.addFileOrFolderInFolder(baseFolder, file, ignoreWatchInvokedWithTriggerAsFileCreate);
340345
}
341346
else {
342347
const fullPath = getNormalizedAbsolutePath(fileOrDirectory.path, this.currentDirectory);
@@ -365,10 +370,13 @@ interface Array<T> {}`
365370
return folder;
366371
}
367372

368-
private addFileOrFolderInFolder(folder: Folder, fileOrDirectory: File | Folder) {
373+
private addFileOrFolderInFolder(folder: Folder, fileOrDirectory: File | Folder, ignoreWatch?: boolean) {
369374
folder.entries.push(fileOrDirectory);
370375
this.fs.set(fileOrDirectory.path, fileOrDirectory);
371376

377+
if (ignoreWatch) {
378+
return;
379+
}
372380
if (isFile(fileOrDirectory)) {
373381
this.invokeFileWatcher(fileOrDirectory.fullPath, FileWatcherEventKind.Created);
374382
}

0 commit comments

Comments
 (0)