From 674ef177c6061df6a850142a52cad836c1b7f223 Mon Sep 17 00:00:00 2001 From: uniqueiniquity Date: Tue, 19 Dec 2017 17:16:01 -0800 Subject: [PATCH 1/6] Allow dynamic files script info to be created when not opened by client --- src/server/editorServices.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index b10ca156de1..761110c2375 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -1748,9 +1748,9 @@ namespace ts.server { const path = normalizedPathToPath(fileName, currentDirectory, this.toCanonicalFileName); let info = this.getScriptInfoForPath(path); if (!info) { - Debug.assert(isRootedDiskPath(fileName) || openedByClient, "Script info with relative file name can only be open script info"); - Debug.assert(!isRootedDiskPath(fileName) || this.currentDirectory === currentDirectory || !this.openFilesWithNonRootedDiskPath.has(this.toCanonicalFileName(fileName)), "Open script files with non rooted disk path opened with current directory context cannot have same canonical names"); const isDynamic = isDynamicFileName(fileName); + Debug.assert(isRootedDiskPath(fileName) || isDynamic || openedByClient, "Script info with non-dynamic relative file name can only be open script info"); + Debug.assert(!isRootedDiskPath(fileName) || this.currentDirectory === currentDirectory || !this.openFilesWithNonRootedDiskPath.has(this.toCanonicalFileName(fileName)), "Open script files with non rooted disk path opened with current directory context cannot have same canonical names"); // If the file is not opened by client and the file doesnot exist on the disk, return if (!openedByClient && !isDynamic && !(hostToQueryFileExistsOn || this.host).fileExists(fileName)) { return; From 8721495a6d9db6ea9b0b44a3367aa740ef8addce Mon Sep 17 00:00:00 2001 From: Benjamin Lichtman Date: Wed, 27 Dec 2017 13:44:51 -0800 Subject: [PATCH 2/6] Add dynamic file open test --- .../unittests/tsserverProjectSystem.ts | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index 1b2aa481b26..c82a231da53 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -1192,6 +1192,28 @@ namespace ts.projectSystem { checkNumberOfInferredProjects(projectService, 0); }); + it("external project for dynamic file", () => { + const file1 = { + path: "/a/b/f1.ts", + content: "let x =1;" + }; + const externalProjectName = "^ScriptDocument1 file1.ts"; + const externalFiles = toExternalFiles(["^ScriptDocument1 file1.ts"]); + const host = createServerHost([file1]); + const projectService = createProjectService(host); + projectService.openExternalProject({ + rootFiles: externalFiles, + options: {}, + projectFileName: externalProjectName + }); + + checkNumberOfExternalProjects(projectService, 1); + checkNumberOfInferredProjects(projectService, 0); + + externalFiles[0].content = "let x =1;"; + projectService.applyChangesInOpenFiles(externalFiles, [], []); + }); + it("external project that included config files", () => { const file1 = { path: "/a/b/f1.ts", From 226e5c9c39176fabb1f3db793f4ea5cea269cc20 Mon Sep 17 00:00:00 2001 From: Benjamin Lichtman Date: Thu, 28 Dec 2017 11:22:41 -0800 Subject: [PATCH 3/6] Simplify test and add explanatory assertion --- src/harness/unittests/tsserverProjectSystem.ts | 6 +----- src/server/editorServices.ts | 1 + 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index c82a231da53..4941878be20 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -1193,13 +1193,9 @@ namespace ts.projectSystem { }); it("external project for dynamic file", () => { - const file1 = { - path: "/a/b/f1.ts", - content: "let x =1;" - }; const externalProjectName = "^ScriptDocument1 file1.ts"; const externalFiles = toExternalFiles(["^ScriptDocument1 file1.ts"]); - const host = createServerHost([file1]); + const host = createServerHost([]); const projectService = createProjectService(host); projectService.openExternalProject({ rootFiles: externalFiles, diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 761110c2375..24df05a4a79 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -1751,6 +1751,7 @@ namespace ts.server { const isDynamic = isDynamicFileName(fileName); Debug.assert(isRootedDiskPath(fileName) || isDynamic || openedByClient, "Script info with non-dynamic relative file name can only be open script info"); Debug.assert(!isRootedDiskPath(fileName) || this.currentDirectory === currentDirectory || !this.openFilesWithNonRootedDiskPath.has(this.toCanonicalFileName(fileName)), "Open script files with non rooted disk path opened with current directory context cannot have same canonical names"); + Debug.assert(!isDynamic || this.currentDirectory === currentDirectory, "Dynamic files must always have current directory context since containing external project name will always match the script info name."); // If the file is not opened by client and the file doesnot exist on the disk, return if (!openedByClient && !isDynamic && !(hostToQueryFileExistsOn || this.host).fileExists(fileName)) { return; From 1922c9a0105c60cbc6fa95fd24cadccc61d92d05 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Tue, 9 Jan 2018 15:43:50 -0800 Subject: [PATCH 4/6] Add test for failure to use correct current directory in inferred project Test for #21040 --- .../unittests/tsserverProjectSystem.ts | 69 +++++++++++++++++++ src/harness/virtualFileSystemWithWatch.ts | 2 +- 2 files changed, 70 insertions(+), 1 deletion(-) diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index 4941878be20..104943272e0 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -6408,4 +6408,73 @@ namespace ts.projectSystem { verifyWatchedDirectories(/*useProjectAtRoot*/ false); }); }); + + describe("tsserverProjectSystem typingsInstaller on inferred Project", () => { + it("when projectRootPath is provided", () => { + const projects = "/users/username/projects"; + const projectRootPath = `${projects}/san2`; + const file: FileOrFolder = { + path: `${projectRootPath}/x.js`, + content: "const aaaaaaav = 1;" + }; + + const currentDirectory = `${projects}/anotherProject`; + const packageJsonInCurrentDirectory: FileOrFolder = { + path: `${currentDirectory}/package.json`, + content: JSON.stringify({ + devDependencies: { + "pkgcurrentdirectory": "" + }, + }) + }; + const packageJsonOfPkgcurrentdirectory: FileOrFolder = { + path: `${currentDirectory}/node_modules/pkgcurrentdirectory/package.json`, + content: JSON.stringify({ + name: "pkgcurrentdirectory", + main: "index.js", + typings: "index.d.ts" + }) + }; + const indexOfPkgcurrentdirectory: FileOrFolder = { + path: `${currentDirectory}/node_modules/pkgcurrentdirectory/index.d.ts`, + content: "export function foo() { }" + }; + + const typingsCache = `/users/username/Library/Caches/typescript/2.7`; + const typingsCachePackageJson: FileOrFolder = { + path: `${typingsCache}/package.json`, + content: JSON.stringify({ + devDependencies: { + }, + }) + }; + + const files = [file, packageJsonInCurrentDirectory, packageJsonOfPkgcurrentdirectory, indexOfPkgcurrentdirectory, typingsCachePackageJson]; + const host = createServerHost(files, { currentDirectory }); + + const typesRegistry = createMap(); + typesRegistry.set("pkgcurrentdirectory", void 0); + const typingsInstaller = new TestTypingsInstaller(typingsCache, /*throttleLimit*/ 5, host, typesRegistry); + + const projectService = createProjectService(host, { typingsInstaller }); + + projectService.setCompilerOptionsForInferredProjects({ + module: ModuleKind.CommonJS, + target: ScriptTarget.ES2016, + jsx: JsxEmit.Preserve, + experimentalDecorators: true, + allowJs: true, + allowSyntheticDefaultImports: true, + allowNonTsExtensions: true + }); + + projectService.openClientFile(file.path, file.content, ScriptKind.JS, projectRootPath); + + const project = projectService.inferredProjects[0]; + assert.isDefined(project); + + // Ensure that we use result from types cache when getting ls + assert.isDefined(project.getLanguageService()); + }); + }); } diff --git a/src/harness/virtualFileSystemWithWatch.ts b/src/harness/virtualFileSystemWithWatch.ts index 158e460780c..4db75c48386 100644 --- a/src/harness/virtualFileSystemWithWatch.ts +++ b/src/harness/virtualFileSystemWithWatch.ts @@ -539,7 +539,7 @@ namespace ts.TestFSWithWatch { } readDirectory(path: string, extensions?: ReadonlyArray, exclude?: ReadonlyArray, include?: ReadonlyArray, depth?: number): string[] { - return ts.matchFiles(this.toNormalizedAbsolutePath(path), extensions, exclude, include, this.useCaseSensitiveFileNames, this.getCurrentDirectory(), depth, (dir) => { + return ts.matchFiles(path, extensions, exclude, include, this.useCaseSensitiveFileNames, this.getCurrentDirectory(), depth, (dir) => { const directories: string[] = []; const files: string[] = []; const dirEntry = this.fs.get(this.toPath(dir)); From c57f266187ec353b415140d0d1388b1e7ae204a3 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Tue, 9 Jan 2018 15:59:23 -0800 Subject: [PATCH 5/6] When sending typings request use project's current directory as project root path This ensures that we arent picking typings from folder different from the current directory for the project --- src/harness/unittests/tsserverProjectSystem.ts | 5 ++++- src/server/utilities.ts | 15 +-------------- 2 files changed, 5 insertions(+), 15 deletions(-) diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index 104943272e0..66ebdcfaa14 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -6423,7 +6423,7 @@ namespace ts.projectSystem { path: `${currentDirectory}/package.json`, content: JSON.stringify({ devDependencies: { - "pkgcurrentdirectory": "" + pkgcurrentdirectory: "" }, }) }; @@ -6475,6 +6475,9 @@ namespace ts.projectSystem { // Ensure that we use result from types cache when getting ls assert.isDefined(project.getLanguageService()); + + // Verify that the pkgcurrentdirectory from the current directory isnt picked up + checkProjectActualFiles(project, [file.path]); }); }); } diff --git a/src/server/utilities.ts b/src/server/utilities.ts index 69399b672b3..55490fc112d 100644 --- a/src/server/utilities.ts +++ b/src/server/utilities.ts @@ -33,19 +33,6 @@ namespace ts.server { export type Types = Err | Info | Perf; } - function getProjectRootPath(project: Project): Path { - switch (project.projectKind) { - case ProjectKind.Configured: - return getDirectoryPath(project.getProjectName()); - case ProjectKind.Inferred: - // TODO: fixme - return ""; - case ProjectKind.External: - const projectName = normalizeSlashes(project.getProjectName()); - return project.projectService.host.fileExists(projectName) ? getDirectoryPath(projectName) : projectName; - } - } - export function createInstallTypingsRequest(project: Project, typeAcquisition: TypeAcquisition, unresolvedImports: SortedReadonlyArray, cachePath?: string): DiscoverTypings { return { projectName: project.getProjectName(), @@ -53,7 +40,7 @@ namespace ts.server { compilerOptions: project.getCompilationSettings(), typeAcquisition, unresolvedImports, - projectRootPath: getProjectRootPath(project), + projectRootPath: project.getCurrentDirectory() as Path, cachePath, kind: "discover" }; From 0d023917b4c545edff849047841a5b06b8a9fdca Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Thu, 18 Jan 2018 12:53:11 -0800 Subject: [PATCH 6/6] Report more detailed info during script debug failure --- src/server/editorServices.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 24df05a4a79..e57becc76b9 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -1749,9 +1749,9 @@ namespace ts.server { let info = this.getScriptInfoForPath(path); if (!info) { const isDynamic = isDynamicFileName(fileName); - Debug.assert(isRootedDiskPath(fileName) || isDynamic || openedByClient, "Script info with non-dynamic relative file name can only be open script info"); - Debug.assert(!isRootedDiskPath(fileName) || this.currentDirectory === currentDirectory || !this.openFilesWithNonRootedDiskPath.has(this.toCanonicalFileName(fileName)), "Open script files with non rooted disk path opened with current directory context cannot have same canonical names"); - Debug.assert(!isDynamic || this.currentDirectory === currentDirectory, "Dynamic files must always have current directory context since containing external project name will always match the script info name."); + Debug.assert(isRootedDiskPath(fileName) || isDynamic || openedByClient, "", () => `${JSON.stringify({ fileName, currentDirectory, hostCurrentDirectory: this.currentDirectory, openKeys: arrayFrom(this.openFilesWithNonRootedDiskPath.keys()) })}\nScript info with non-dynamic relative file name can only be open script info`); + Debug.assert(!isRootedDiskPath(fileName) || this.currentDirectory === currentDirectory || !this.openFilesWithNonRootedDiskPath.has(this.toCanonicalFileName(fileName)), "", () => `${JSON.stringify({ fileName, currentDirectory, hostCurrentDirectory: this.currentDirectory, openKeys: arrayFrom(this.openFilesWithNonRootedDiskPath.keys()) })}\nOpen script files with non rooted disk path opened with current directory context cannot have same canonical names`); + Debug.assert(!isDynamic || this.currentDirectory === currentDirectory, "", () => `${JSON.stringify({ fileName, currentDirectory, hostCurrentDirectory: this.currentDirectory, openKeys: arrayFrom(this.openFilesWithNonRootedDiskPath.keys()) })}\nDynamic files must always have current directory context since containing external project name will always match the script info name.`); // If the file is not opened by client and the file doesnot exist on the disk, return if (!openedByClient && !isDynamic && !(hostToQueryFileExistsOn || this.host).fileExists(fileName)) { return;