From 944565718446a0e7a11104afa493e7ba1a4a71ef Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Fri, 20 Dec 2019 07:44:35 -0800 Subject: [PATCH] Correctly set filesByName map when reusing program to ensure it is same as old (#35784) 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 | 31 +++--- src/compiler/types.ts | 2 + .../unittests/tsserver/projectReferences.ts | 101 ++++++++++++------ 3 files changed, 88 insertions(+), 46 deletions(-) diff --git a/src/compiler/program.ts b/src/compiler/program.ts index 847a85ec9f3..891763ddbbd 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); - } - } - // 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); - } + filesByName.set(newSourceFile.path, newSourceFile); } + const oldFilesByNameMap = oldProgram.getFilesByNameMap(); + oldFilesByNameMap.forEach((oldFile, path) => { + if (!oldFile) { + filesByName.set(path, oldFile); + return; + } + 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 393e48695a2..5fb85679cc5 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 b67d04733a5..fa5196bb4bd 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/"); }); });