Skip to content

Commit 76ee021

Browse files
sheetalkamatsheetalkamat
authored
Use watch invoked with node_modules/.staging as watch for refreshing complete node_modules, so that npm install is reflected correctly (#36039)
* Add test that demonstrates npm install watch behaviour some times * Use watch invoked with `node_modules/.staging` as watch for refreshing complete node_modules, so that npm install is reflected correctly Fixes #35966
1 parent 0c3019e commit 76ee021

File tree

5 files changed

+142
-16
lines changed

5 files changed

+142
-16
lines changed

‎src/compiler/resolutionCache.ts‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,15 @@ namespace ts {
7373
nonRecursive?: boolean;
7474
}
7575

76-
export function isPathIgnored(path: Path) {
77-
return some(ignoredPaths, searchPath => stringContains(path, searchPath));
76+
export function removeIgnoredPath(path: Path): Path | undefined {
77+
// Consider whole staging folder as if node_modules changed.
78+
if (endsWith(path, "/node_modules/.staging")) {
79+
return removeSuffix(path, "/.staging") as Path;
80+
}
81+
82+
return some(ignoredPaths, searchPath => stringContains(path, searchPath)) ?
83+
undefined :
84+
path;
7885
}
7986

8087
/**
@@ -722,7 +729,9 @@ namespace ts {
722729
}
723730
else {
724731
// If something to do with folder/file starting with "." in node_modules folder, skip it
725-
if (isPathIgnored(fileOrDirectoryPath)) return false;
732+
const updatedPath = removeIgnoredPath(fileOrDirectoryPath);
733+
if (!updatedPath) return false;
734+
fileOrDirectoryPath = updatedPath;
726735

727736
// prevent saving an open file from over-eagerly triggering invalidation
728737
if (resolutionHost.fileIsOpen(fileOrDirectoryPath)) {

‎src/compiler/watchPublic.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -687,15 +687,16 @@ namespace ts {
687687
fileOrDirectory => {
688688
Debug.assert(!!configFileName);
689689

690-
const fileOrDirectoryPath = toPath(fileOrDirectory);
690+
let fileOrDirectoryPath: Path | undefined = toPath(fileOrDirectory);
691691

692692
// Since the file existance changed, update the sourceFiles cache
693693
if (cachedDirectoryStructureHost) {
694694
cachedDirectoryStructureHost.addOrDeleteFileOrDirectory(fileOrDirectory, fileOrDirectoryPath);
695695
}
696696
nextSourceFileVersion(fileOrDirectoryPath);
697697

698-
if (isPathIgnored(fileOrDirectoryPath)) return;
698+
fileOrDirectoryPath = removeIgnoredPath(fileOrDirectoryPath);
699+
if (!fileOrDirectoryPath) return;
699700

700701
// If the the added or created file or directory is not supported file name, ignore the file
701702
// But when watched directory is added/removed, we need to reload the file list

‎src/harness/virtualFileSystemWithWatch.ts‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -611,37 +611,38 @@ interface Array<T> { length: number; [n: number]: T; }`
611611
}
612612
}
613613

614-
ensureFileOrFolder(fileOrDirectoryOrSymLink: FileOrFolderOrSymLink, ignoreWatchInvokedWithTriggerAsFileCreate?: boolean) {
614+
ensureFileOrFolder(fileOrDirectoryOrSymLink: FileOrFolderOrSymLink, ignoreWatchInvokedWithTriggerAsFileCreate?: boolean, ignoreParentWatch?: boolean) {
615615
if (isFile(fileOrDirectoryOrSymLink)) {
616616
const file = this.toFsFile(fileOrDirectoryOrSymLink);
617617
// file may already exist when updating existing type declaration file
618618
if (!this.fs.get(file.path)) {
619-
const baseFolder = this.ensureFolder(getDirectoryPath(file.fullPath));
619+
const baseFolder = this.ensureFolder(getDirectoryPath(file.fullPath), ignoreParentWatch);
620620
this.addFileOrFolderInFolder(baseFolder, file, ignoreWatchInvokedWithTriggerAsFileCreate);
621621
}
622622
}
623623
else if (isSymLink(fileOrDirectoryOrSymLink)) {
624624
const symLink = this.toFsSymLink(fileOrDirectoryOrSymLink);
625625
Debug.assert(!this.fs.get(symLink.path));
626-
const baseFolder = this.ensureFolder(getDirectoryPath(symLink.fullPath));
626+
const baseFolder = this.ensureFolder(getDirectoryPath(symLink.fullPath), ignoreParentWatch);
627627
this.addFileOrFolderInFolder(baseFolder, symLink, ignoreWatchInvokedWithTriggerAsFileCreate);
628628
}
629629
else {
630630
const fullPath = getNormalizedAbsolutePath(fileOrDirectoryOrSymLink.path, this.currentDirectory);
631-
this.ensureFolder(fullPath);
631+
this.ensureFolder(getDirectoryPath(fullPath), ignoreParentWatch);
632+
this.ensureFolder(fullPath, ignoreWatchInvokedWithTriggerAsFileCreate);
632633
}
633634
}
634635

635-
private ensureFolder(fullPath: string): FsFolder {
636+
private ensureFolder(fullPath: string, ignoreWatch: boolean | undefined): FsFolder {
636637
const path = this.toPath(fullPath);
637638
let folder = this.fs.get(path) as FsFolder;
638639
if (!folder) {
639640
folder = this.toFsFolder(fullPath);
640641
const baseFullPath = getDirectoryPath(fullPath);
641642
if (fullPath !== baseFullPath) {
642643
// Add folder in the base folder
643-
const baseFolder = this.ensureFolder(baseFullPath);
644-
this.addFileOrFolderInFolder(baseFolder, folder);
644+
const baseFolder = this.ensureFolder(baseFullPath, ignoreWatch);
645+
this.addFileOrFolderInFolder(baseFolder, folder, ignoreWatch);
645646
}
646647
else {
647648
// root folder

‎src/server/editorServices.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1104,7 +1104,7 @@ namespace ts.server {
11041104
this.host,
11051105
directory,
11061106
fileOrDirectory => {
1107-
const fileOrDirectoryPath = this.toPath(fileOrDirectory);
1107+
let fileOrDirectoryPath: Path | undefined = this.toPath(fileOrDirectory);
11081108
const fsResult = project.getCachedDirectoryStructureHost().addOrDeleteFileOrDirectory(fileOrDirectory, fileOrDirectoryPath);
11091109

11101110
// don't trigger callback on open, existing files
@@ -1115,7 +1115,8 @@ namespace ts.server {
11151115
return;
11161116
}
11171117

1118-
if (isPathIgnored(fileOrDirectoryPath)) return;
1118+
fileOrDirectoryPath = removeIgnoredPath(fileOrDirectoryPath);
1119+
if (!fileOrDirectoryPath) return;
11191120
const configFilename = project.getConfigFilePath();
11201121

11211122
if (getBaseFileName(fileOrDirectoryPath) === "package.json" && !isInsideNodeModules(fileOrDirectoryPath) &&
@@ -2272,8 +2273,8 @@ namespace ts.server {
22722273
this.host,
22732274
watchDir,
22742275
(fileOrDirectory) => {
2275-
const fileOrDirectoryPath = this.toPath(fileOrDirectory);
2276-
if (isPathIgnored(fileOrDirectoryPath)) return;
2276+
const fileOrDirectoryPath = removeIgnoredPath(this.toPath(fileOrDirectory));
2277+
if (!fileOrDirectoryPath) return;
22772278

22782279
// Has extension
22792280
Debug.assert(result.refCount > 0);

‎src/testRunner/unittests/tsserver/projectErrors.ts‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -933,4 +933,118 @@ console.log(blabla);`
933933
});
934934
});
935935
});
936+
937+
describe("unittests:: tsserver:: Project Errors with npm install when", () => {
938+
function verifyNpmInstall(timeoutDuringPartialInstallation: boolean) {
939+
const main: File = {
940+
path: `${tscWatch.projectRoot}/src/main.ts`,
941+
content: "import * as _a from '@angular/core';"
942+
};
943+
const config: File = {
944+
path: `${tscWatch.projectRoot}/tsconfig.json`,
945+
content: "{}"
946+
};
947+
const projectFiles = [main, libFile, config];
948+
const host = createServerHost(projectFiles);
949+
const session = createSession(host, { canUseEvents: true });
950+
const service = session.getProjectService();
951+
openFilesForSession([{ file: main, projectRootPath: tscWatch.projectRoot }], session);
952+
const span = protocolTextSpanFromSubstring(main.content, `'@angular/core'`);
953+
const moduleNotFoundErr: protocol.Diagnostic[] = [
954+
createDiagnostic(
955+
span.start,
956+
span.end,
957+
Diagnostics.Cannot_find_module_0,
958+
["@angular/core"]
959+
)
960+
];
961+
const expectedRecursiveWatches = arrayToMap([`${tscWatch.projectRoot}`, `${tscWatch.projectRoot}/src`, `${tscWatch.projectRoot}/node_modules`, `${tscWatch.projectRoot}/node_modules/@types`], identity, () => 1);
962+
verifyProject();
963+
verifyErrors(moduleNotFoundErr);
964+
965+
let npmInstallComplete = false;
966+
967+
// Simulate npm install
968+
let filesAndFoldersToAdd: (File | Folder)[] = [
969+
{ path: `${tscWatch.projectRoot}/node_modules` }, // This should queue update
970+
{ path: `${tscWatch.projectRoot}/node_modules/.staging` },
971+
{ path: `${tscWatch.projectRoot}/node_modules/.staging/@babel` },
972+
{ path: `${tscWatch.projectRoot}/node_modules/.staging/@babel/helper-plugin-utils-a06c629f` },
973+
{ path: `${tscWatch.projectRoot}/node_modules/.staging/core-js-db53158d` },
974+
];
975+
verifyWhileNpmInstall({ timeouts: 2, semantic: moduleNotFoundErr });
976+
977+
filesAndFoldersToAdd = [
978+
{ path: `${tscWatch.projectRoot}/node_modules/.staging/@angular/platform-browser-dynamic-5efaaa1a` },
979+
{ path: `${tscWatch.projectRoot}/node_modules/.staging/@angular/cli-c1e44b05/models/analytics.d.ts`, content: `export const x = 10;` },
980+
{ path: `${tscWatch.projectRoot}/node_modules/.staging/@angular/core-0963aebf/index.d.ts`, content: `export const y = 10;` },
981+
];
982+
// Since we added/removed in .staging no timeout
983+
verifyWhileNpmInstall({ timeouts: 0, semantic: moduleNotFoundErr });
984+
985+
filesAndFoldersToAdd = [];
986+
// Move things from staging to node_modules without triggering watch
987+
const moduleFile: File = {
988+
path: `${tscWatch.projectRoot}/node_modules/@angular/core/index.d.ts`,
989+
content: `export const y = 10;`
990+
};
991+
host.ensureFileOrFolder(moduleFile, /*ignoreWatchInvokedWithTriggerAsFileCreate*/ true, /*ignoreParentWatch*/ true);
992+
// Since we added/removed in .staging no timeout
993+
verifyWhileNpmInstall({ timeouts: 0, semantic: moduleNotFoundErr });
994+
995+
// Remove staging folder to remove errors
996+
host.deleteFolder(`${tscWatch.projectRoot}/node_modules/.staging`, /*recursive*/ true);
997+
npmInstallComplete = true;
998+
projectFiles.push(moduleFile);
999+
// Additional watch for watching script infos from node_modules
1000+
expectedRecursiveWatches.set(`${tscWatch.projectRoot}/node_modules`, 2);
1001+
verifyWhileNpmInstall({ timeouts: 2, semantic: [] });
1002+
1003+
function verifyWhileNpmInstall({ timeouts, semantic }: { timeouts: number; semantic: protocol.Diagnostic[] }) {
1004+
filesAndFoldersToAdd.forEach(f => host.ensureFileOrFolder(f));
1005+
if (npmInstallComplete || timeoutDuringPartialInstallation) {
1006+
host.checkTimeoutQueueLengthAndRun(timeouts);
1007+
}
1008+
else {
1009+
host.checkTimeoutQueueLength(2);
1010+
}
1011+
verifyProject();
1012+
verifyErrors(semantic, !npmInstallComplete && !timeoutDuringPartialInstallation ? 2 : undefined);
1013+
}
1014+
1015+
function verifyProject() {
1016+
checkNumberOfConfiguredProjects(service, 1);
1017+
1018+
const project = service.configuredProjects.get(config.path)!;
1019+
checkProjectActualFiles(project, map(projectFiles, f => f.path));
1020+
1021+
checkWatchedFilesDetailed(host, mapDefined(projectFiles, f => f === main || f === moduleFile ? undefined : f.path), 1);
1022+
checkWatchedDirectoriesDetailed(host, expectedRecursiveWatches, /*recursive*/ true);
1023+
checkWatchedDirectories(host, [], /*recursive*/ false);
1024+
}
1025+
1026+
function verifyErrors(semantic: protocol.Diagnostic[], existingTimeouts?: number) {
1027+
verifyGetErrRequest({
1028+
session,
1029+
host,
1030+
expected: [{
1031+
file: main,
1032+
syntax: [],
1033+
semantic,
1034+
suggestion: []
1035+
}],
1036+
existingTimeouts
1037+
});
1038+
1039+
}
1040+
}
1041+
1042+
it("timeouts occur inbetween installation", () => {
1043+
verifyNpmInstall(/*timeoutDuringPartialInstallation*/ true);
1044+
});
1045+
1046+
it("timeout occurs after installation", () => {
1047+
verifyNpmInstall(/*timeoutDuringPartialInstallation*/ false);
1048+
});
1049+
});
9361050
}

0 commit comments

Comments
 (0)