Skip to content

Correctly set filesByName map when reusing program to ensure it is same as old program #35784

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 20, 2019
Merged

Correctly set filesByName map when reusing program to ensure it is same as old program #35784

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 17 additions & 12 deletions src/compiler/program.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -933,6 +933,7 @@ namespace ts {
getSourceFiles: () => files,
getMissingFilePaths: () => missingFilePaths!, // TODO: GH#18217
getRefFileMap: () => refFileMap,
getFilesByNameMap: () => filesByName,
getCompilerOptions: () => options,
getSyntacticDiagnostics,
getOptionsDiagnostics,
Expand Down Expand Up @@ -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();
Expand Down
2 changes: 2 additions & 0 deletions src/compiler/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3131,6 +3131,8 @@ namespace ts {
getMissingFilePaths(): readonly Path[];
/* @internal */
getRefFileMap(): MultiMap<RefFile> | undefined;
/* @internal */
getFilesByNameMap(): Map<SourceFile | false | undefined>;

/**
* Emits the JavaScript and declaration files. If targetSourceFile is not specified, then
Expand Down
101 changes: 68 additions & 33 deletions src/testRunner/unittests/tsserver/projectReferences.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1433,6 +1433,7 @@ function foo() {
aTest: File;
bFoo: File;
bBar: File;
bSymlink: SymLink;
}
function verifySymlinkScenario(packages: () => Packages) {
describe("when solution is not built", () => {
Expand All @@ -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]) :
Expand All @@ -1485,6 +1482,26 @@ function foo() {
{ file: aTest, syntax: [], semantic: [], suggestion: [] }
]
});
session.executeCommandSeq<protocol.UpdateOpenRequest>({
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 {
Expand All @@ -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/");
});
});

Expand Down