From 19c404ec510b2036037c5ebfd15c3ee19b370caf Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Thu, 19 Dec 2019 12:25:04 -0800 Subject: [PATCH] Correctly set filesByName map when reusing program to ensure it is same as old It was previously not populated correctly if preserveSymlinks with useSourceOfProjectReference was true, in that case if module was resolved to symlink and we deduced it refers to source of project reference we want to set "resolvedFileName" correctly otherwise it results in incorrect module not found errors. --- src/compiler/program.ts | 29 ++--- src/compiler/types.ts | 2 + .../unittests/tsserver/projectReferences.ts | 101 ++++++++++++------ 3 files changed, 87 insertions(+), 45 deletions(-) diff --git a/src/compiler/program.ts b/src/compiler/program.ts index 847a85ec9f396..891763ddbbd33 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -933,6 +933,7 @@ namespace ts { getSourceFiles: () => files, getMissingFilePaths: () => missingFilePaths!, // TODO: GH#18217 getRefFileMap: () => refFileMap, + getFilesByNameMap: () => filesByName, getCompilerOptions: () => options, getSyntacticDiagnostics, getOptionsDiagnostics, @@ -1422,21 +1423,25 @@ namespace ts { refFileMap = oldProgram.getRefFileMap(); // update fileName -> file mapping + Debug.assert(newSourceFiles.length === oldProgram.getSourceFiles().length); for (const newSourceFile of newSourceFiles) { - const filePath = newSourceFile.path; - addFileToFilesByName(newSourceFile, filePath, newSourceFile.resolvedPath); - if (useSourceOfProjectReferenceRedirect) { - const redirectProject = getProjectReferenceRedirectProject(newSourceFile.fileName); - if (redirectProject && !(redirectProject.commandLine.options.outFile || redirectProject.commandLine.options.out)) { - const redirect = getProjectReferenceOutputName(redirectProject, newSourceFile.fileName); - addFileToFilesByName(newSourceFile, toPath(redirect), /*redirectedPath*/ undefined); - } + filesByName.set(newSourceFile.path, newSourceFile); + } + const oldFilesByNameMap = oldProgram.getFilesByNameMap(); + oldFilesByNameMap.forEach((oldFile, path) => { + if (!oldFile) { + filesByName.set(path, oldFile); + return; } - // Set the file as found during node modules search if it was found that way in old progra, - if (oldProgram.isSourceFileFromExternalLibrary(oldProgram.getSourceFileByPath(newSourceFile.resolvedPath)!)) { - sourceFilesFoundSearchingNodeModules.set(filePath, true); + if (oldFile.path === path) { + // Set the file as found during node modules search if it was found that way in old progra, + if (oldProgram!.isSourceFileFromExternalLibrary(oldFile)) { + sourceFilesFoundSearchingNodeModules.set(oldFile.path, true); + } + return; } - } + filesByName.set(path, filesByName.get(oldFile.path)); + }); files = newSourceFiles; fileProcessingDiagnostics = oldProgram.getFileProcessingDiagnostics(); diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 393e48695a2a6..5fb85679cc5e3 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -3131,6 +3131,8 @@ namespace ts { getMissingFilePaths(): readonly Path[]; /* @internal */ getRefFileMap(): MultiMap | undefined; + /* @internal */ + getFilesByNameMap(): Map; /** * Emits the JavaScript and declaration files. If targetSourceFile is not specified, then diff --git a/src/testRunner/unittests/tsserver/projectReferences.ts b/src/testRunner/unittests/tsserver/projectReferences.ts index b67d04733a576..fa5196bb4bdd4 100644 --- a/src/testRunner/unittests/tsserver/projectReferences.ts +++ b/src/testRunner/unittests/tsserver/projectReferences.ts @@ -1433,6 +1433,7 @@ function foo() { aTest: File; bFoo: File; bBar: File; + bSymlink: SymLink; } function verifySymlinkScenario(packages: () => Packages) { describe("when solution is not built", () => { @@ -1456,13 +1457,9 @@ function foo() { }); } - function verifySession({ bPackageJson, aTest, bFoo, bBar }: Packages, alreadyBuilt: boolean, extraOptions: CompilerOptions) { + function verifySession({ bPackageJson, aTest, bFoo, bBar, bSymlink }: Packages, alreadyBuilt: boolean, extraOptions: CompilerOptions) { const aConfig = config("A", extraOptions, ["../B"]); const bConfig = config("B", extraOptions); - const bSymlink: SymLink = { - path: `${tscWatch.projectRoot}/node_modules/b`, - symLink: `${tscWatch.projectRoot}/packages/B` - }; const files = [libFile, bPackageJson, aConfig, bConfig, aTest, bFoo, bBar, bSymlink]; const host = alreadyBuilt ? createHost(files, [aConfig.path]) : @@ -1485,6 +1482,26 @@ function foo() { { file: aTest, syntax: [], semantic: [], suggestion: [] } ] }); + session.executeCommandSeq({ + command: protocol.CommandTypes.UpdateOpen, + arguments: { + changedFiles: [{ + fileName: aTest.path, + textChanges: [{ + newText: "\n", + start: { line: 5, offset: 1 }, + end: { line: 5, offset: 1 } + }] + }] + } + }); + verifyGetErrRequest({ + host, + session, + expected: [ + { file: aTest, syntax: [], semantic: [], suggestion: [] } + ] + }); } function config(packageName: string, extraOptions: CompilerOptions, references?: string[]): File { @@ -1510,37 +1527,55 @@ function foo() { }; } - describe("when packageJson has types field and has index.ts", () => { - verifySymlinkScenario(() => ({ - bPackageJson: { - path: `${tscWatch.projectRoot}/packages/B/package.json`, - content: JSON.stringify({ - main: "lib/index.js", - types: "lib/index.d.ts" - }) - }, - aTest: file("A", "index.ts", `import { foo } from 'b'; -import { bar } from 'b/lib/bar'; + function verifyMonoRepoLike(scope = "") { + describe("when packageJson has types field and has index.ts", () => { + verifySymlinkScenario(() => ({ + bPackageJson: { + path: `${tscWatch.projectRoot}/packages/B/package.json`, + content: JSON.stringify({ + main: "lib/index.js", + types: "lib/index.d.ts" + }) + }, + aTest: file("A", "index.ts", `import { foo } from '${scope}b'; +import { bar } from '${scope}b/lib/bar'; foo(); -bar();`), - bFoo: file("B", "index.ts", `export function foo() { }`), - bBar: file("B", "bar.ts", `export function bar() { }`) - })); - }); +bar(); +`), + bFoo: file("B", "index.ts", `export function foo() { }`), + bBar: file("B", "bar.ts", `export function bar() { }`), + bSymlink: { + path: `${tscWatch.projectRoot}/node_modules/${scope}b`, + symLink: `${tscWatch.projectRoot}/packages/B` + } + })); + }); - describe("when referencing file from subFolder", () => { - verifySymlinkScenario(() => ({ - bPackageJson: { - path: `${tscWatch.projectRoot}/packages/B/package.json`, - content: "{}" - }, - aTest: file("A", "test.ts", `import { foo } from 'b/lib/foo'; -import { bar } from 'b/lib/bar/foo'; + describe("when referencing file from subFolder", () => { + verifySymlinkScenario(() => ({ + bPackageJson: { + path: `${tscWatch.projectRoot}/packages/B/package.json`, + content: "{}" + }, + aTest: file("A", "test.ts", `import { foo } from '${scope}b/lib/foo'; +import { bar } from '${scope}b/lib/bar/foo'; foo(); -bar();`), - bFoo: file("B", "foo.ts", `export function foo() { }`), - bBar: file("B", "bar/foo.ts", `export function bar() { }`) - })); +bar(); +`), + bFoo: file("B", "foo.ts", `export function foo() { }`), + bBar: file("B", "bar/foo.ts", `export function bar() { }`), + bSymlink: { + path: `${tscWatch.projectRoot}/node_modules/${scope}b`, + symLink: `${tscWatch.projectRoot}/packages/B` + } + })); + }); + } + describe("when package is not scoped", () => { + verifyMonoRepoLike(); + }); + describe("when package is scoped", () => { + verifyMonoRepoLike("@issue/"); }); });