Skip to content

Do not create multiple sourceFile to single outputFile with project redirect as the output file is included by default #30721

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
sheetalkamat merged 3 commits into from
Apr 3, 2019
Merged

Do not create multiple sourceFile to single outputFile with project redirect as the output file is included by default #30721

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3fdd66b
Report program's source files even when there are errors when buildin…
sheetalkamat Apr 2, 2019
b559e81
Add test when module resolution resolves to original file of referenc…
sheetalkamat Apr 2, 2019
602aec2
Never create redirect for sourceFiles that get emitted to single outp…
sheetalkamat Apr 2, 2019
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
23 changes: 17 additions & 6 deletions src/compiler/program.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2266,8 +2266,13 @@ namespace ts {

let redirectedPath: Path | undefined;
if (refFile) {
Copy link
Member

@andrewbranch andrewbranch Apr 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is refFile in this function?

Copy link
Member Author

@sheetalkamat sheetalkamat Apr 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

refFile is normally passed in if its trying to resolve imports/ or reference directives etc. thats the file in which import exists and hence the file is being added. The root files passed to program, when processed have refile as undefined.

const redirect = getProjectReferenceRedirect(fileName);
if (redirect) {
const redirectProject = getProjectReferenceRedirectProject(fileName);
if (redirectProject) {
if (redirectProject.commandLine.options.outFile || redirectProject.commandLine.options.out) {
// Shouldnt create many to 1 mapping file in --out scenario
return undefined;
Copy link
Member

@andrewbranch andrewbranch Apr 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trying to see if I understand:

Normally, if you call findSourceFile for say subproject/a.ts which is part of a referenced project, you'd return (after updating caches) the source file for subproject/a.d.ts. But if that referenced project has an outFile option, there won't be a subproject/a.d.ts, there will be a single concatenated definition file, let's say subproject/built.d.ts.

So you return early here because subproject/built.d.ts has already been processed, and doing it again causes problems. That makes sense to me, but why not return the SourceFile for subproject/built.d.ts?

Copy link
Member Author

@sheetalkamat sheetalkamat Apr 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You do not want to return subproject/built.d.ts because normally file is 1:1 mapping and breaking that could lead to more severe breaks down the line somewhere. Technically there is no single file with subproject/a.ts but its mix of multiple files in subproject

}
const redirect = getProjectReferenceOutputName(redirectProject, fileName);
fileName = redirect;
// Once we start redirecting to a file, we can potentially come back to it
// via a back-reference from another file in the .d.ts folder. If that happens we'll
Expand Down Expand Up @@ -2364,17 +2369,23 @@ namespace ts {
}

function getProjectReferenceRedirect(fileName: string): string | undefined {
const referencedProject = getProjectReferenceRedirectProject(fileName);
return referencedProject && getProjectReferenceOutputName(referencedProject, fileName);
}

function getProjectReferenceRedirectProject(fileName: string) {
// Ignore dts or any of the non ts files
if (!resolvedProjectReferences || !resolvedProjectReferences.length || fileExtensionIs(fileName, Extension.Dts) || !fileExtensionIsOneOf(fileName, supportedTSExtensions)) {
return undefined;
}

// If this file is produced by a referenced project, we need to rewrite it to
// look in the output folder of the referenced project rather than the input
const referencedProject = getResolvedProjectReferenceToRedirect(fileName);
if (!referencedProject) {
return undefined;
}
return getResolvedProjectReferenceToRedirect(fileName);
}


function getProjectReferenceOutputName(referencedProject: ResolvedProjectReference, fileName: string) {
const out = referencedProject.commandLine.options.outFile || referencedProject.commandLine.options.out;
return out ?
changeExtension(out, Extension.Dts) :
Expand Down
35 changes: 25 additions & 10 deletions src/compiler/tsbuild.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ namespace ts {
const projectStatus = createFileMap<UpToDateStatus>(toPath);
const missingRoots = createMap<true>();
let globalDependencyGraph: DependencyGraph | undefined;
const writeFileName = (s: string) => host.trace && host.trace(s);
const writeFileName = host.trace ? (s: string) => host.trace!(s) : undefined;
let readFileWithCache = (f: string) => host.readFile(f);
let projectCompilerOptions = baseCompilerOptions;
const compilerHost = createCompilerHostFromProgramHost(host, () => projectCompilerOptions);
Expand Down Expand Up @@ -1129,7 +1129,7 @@ namespace ts {
let declDiagnostics: Diagnostic[] | undefined;
const reportDeclarationDiagnostics = (d: Diagnostic) => (declDiagnostics || (declDiagnostics = [])).push(d);
const outputFiles: OutputFile[] = [];
emitFilesAndReportErrors(program, reportDeclarationDiagnostics, writeFileName, /*reportSummary*/ undefined, (name, text, writeByteOrderMark) => outputFiles.push({ name, text, writeByteOrderMark }));
emitFilesAndReportErrors(program, reportDeclarationDiagnostics, /*writeFileName*/ undefined, /*reportSummary*/ undefined, (name, text, writeByteOrderMark) => outputFiles.push({ name, text, writeByteOrderMark }));
// Don't emit .d.ts if there are decl file errors
if (declDiagnostics) {
program.restoreState();
Expand All @@ -1138,7 +1138,7 @@ namespace ts {

// Actual Emit
const emitterDiagnostics = createDiagnosticCollection();
const emittedOutputs = createFileMap<true>(toPath as ToPath);
const emittedOutputs = createFileMap<string>(toPath as ToPath);
outputFiles.forEach(({ name, text, writeByteOrderMark }) => {
let priorChangeTime: Date | undefined;
if (!anyDtsChanged && isDeclarationFile(name)) {
Expand All @@ -1152,7 +1152,7 @@ namespace ts {
}
}

emittedOutputs.setValue(name, true);
emittedOutputs.setValue(name, name);
writeFile(compilerHost, emitterDiagnostics, name, text, writeByteOrderMark);
if (priorChangeTime !== undefined) {
newestDeclarationFileContentChangedTime = newer(priorChangeTime, newestDeclarationFileContentChangedTime);
Expand All @@ -1165,6 +1165,11 @@ namespace ts {
return buildErrors(emitDiagnostics, BuildResultFlags.EmitErrors, "Emit");
}

if (writeFileName) {
emittedOutputs.forEach(name => listEmittedFile(configFile, name));
listFiles(program, writeFileName);
}

// Update time stamps for rest of the outputs
newestDeclarationFileContentChangedTime = updateOutputTimestampsWorker(configFile, newestDeclarationFileContentChangedTime, Diagnostics.Updating_unchanged_output_timestamps_of_project_0, emittedOutputs);

Expand All @@ -1182,13 +1187,21 @@ namespace ts {
function buildErrors(diagnostics: ReadonlyArray<Diagnostic>, errorFlags: BuildResultFlags, errorType: string) {
resultFlags |= errorFlags;
reportAndStoreErrors(proj, diagnostics);
// List files if any other build error using program (emit errors already report files)
if (writeFileName) listFiles(program, writeFileName);
projectStatus.setValue(proj, { type: UpToDateStatusType.Unbuildable, reason: `${errorType} errors` });
afterProgramCreate(proj, program);
projectCompilerOptions = baseCompilerOptions;
return resultFlags;
}
}

function listEmittedFile(proj: ParsedCommandLine, file: string) {
if (writeFileName && proj.options.listEmittedFiles) {
writeFileName(`TSFILE: ${file}`);
}
}

function afterProgramCreate(proj: ResolvedConfigFileName, program: T) {
if (host.afterProgramEmitAndDiagnostics) {
host.afterProgramEmitAndDiagnostics(program);
Expand Down Expand Up @@ -1229,9 +1242,9 @@ namespace ts {
// Actual Emit
Debug.assert(!!outputFiles.length);
const emitterDiagnostics = createDiagnosticCollection();
const emittedOutputs = createFileMap<true>(toPath as ToPath);
const emittedOutputs = createFileMap<string>(toPath as ToPath);
outputFiles.forEach(({ name, text, writeByteOrderMark }) => {
emittedOutputs.setValue(name, true);
emittedOutputs.setValue(name, name);
writeFile(compilerHost, emitterDiagnostics, name, text, writeByteOrderMark);
});
const emitDiagnostics = emitterDiagnostics.getDiagnostics();
Expand All @@ -1242,6 +1255,10 @@ namespace ts {
return BuildResultFlags.DeclarationOutputUnchanged | BuildResultFlags.EmitErrors;
}

if (writeFileName) {
emittedOutputs.forEach(name => listEmittedFile(config, name));
}

// Update timestamps for dts
const newestDeclarationFileContentChangedTime = updateOutputTimestampsWorker(config, minimumDate, Diagnostics.Updating_unchanged_output_timestamps_of_project_0, emittedOutputs);

Expand Down Expand Up @@ -1270,7 +1287,7 @@ namespace ts {
projectStatus.setValue(proj.options.configFilePath as ResolvedConfigFilePath, status);
}

function updateOutputTimestampsWorker(proj: ParsedCommandLine, priorNewestUpdateTime: Date, verboseMessage: DiagnosticMessage, skipOutputs?: FileMap<true>) {
function updateOutputTimestampsWorker(proj: ParsedCommandLine, priorNewestUpdateTime: Date, verboseMessage: DiagnosticMessage, skipOutputs?: FileMap<string>) {
const outputs = getAllProjectOutputs(proj, !host.useCaseSensitiveFileNames());
if (!skipOutputs || outputs.length !== skipOutputs.getSize()) {
if (options.verbose) {
Expand All @@ -1287,9 +1304,7 @@ namespace ts {
}

host.setModifiedTime(file, now);
if (proj.options.listEmittedFiles) {
writeFileName(`TSFILE: ${file}`);
}
listEmittedFile(proj, file);
}
}

Expand Down
15 changes: 9 additions & 6 deletions src/compiler/watch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,14 @@ namespace ts {
emit(targetSourceFile?: SourceFile, writeFile?: WriteFileCallback): EmitResult;
}

export function listFiles(program: ProgramToEmitFilesAndReportErrors, writeFileName: (s: string) => void) {
if (program.getCompilerOptions().listFiles) {
forEach(program.getSourceFiles(), file => {
writeFileName(file.fileName);
});
}
}

/**
* Helper that emit files, report diagnostics and lists emitted and/or source files depending on compiler options
*/
Expand Down Expand Up @@ -152,12 +160,7 @@ namespace ts {
const filepath = getNormalizedAbsolutePath(file, currentDir);
writeFileName(`TSFILE: ${filepath}`);
});

if (program.getCompilerOptions().listFiles) {
forEach(program.getSourceFiles(), file => {
writeFileName(file.fileName);
});
}
listFiles(program, writeFileName);
}

if (reportSummary) {
Expand Down
52 changes: 52 additions & 0 deletions src/testRunner/unittests/tsbuild/amdModulesWithOut.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,58 @@ ${internal} export enum internalEnum { a, b, c }`);
modifyAgainFs: fs => replaceText(fs, sources[project.lib][source.ts][1], `export const`, `/*@internal*/ export const`),
});
});

describe("when the module resolution finds original source file", () => {
function modifyFs(fs: vfs.FileSystem) {
// Make lib to output to parent dir
replaceText(fs, sources[project.lib][source.config], `"outFile": "module.js"`, `"outFile": "../module.js", "rootDir": "../"`);
// Change reference to file1 module to resolve to lib/file1
replaceText(fs, sources[project.app][source.ts][0], "file1", "lib/file1");
}

const libOutputFile: OutputFile = [
"/src/lib/module.js",
"/src/lib/module.js.map",
"/src/lib/module.d.ts",
"/src/lib/module.d.ts.map",
"/src/lib/module.tsbuildinfo"
];
verifyTsbuildOutput({
scenario: "when the module resolution finds original source file",
projFs: () => outFileFs,
time,
tick,
proj: "amdModulesWithOut",
rootNames: ["/src/app"],
expectedMapFileNames: [
libOutputFile[ext.jsmap],
libOutputFile[ext.dtsmap],
outputFiles[project.app][ext.jsmap],
outputFiles[project.app][ext.dtsmap],
],
expectedBuildInfoFilesForSectionBaselines: [
[libOutputFile[ext.buildinfo], libOutputFile[ext.js], libOutputFile[ext.dts]],
[outputFiles[project.app][ext.buildinfo], outputFiles[project.app][ext.js], outputFiles[project.app][ext.dts]]
],
lastProjectOutputJs: outputFiles[project.app][ext.js],
initialBuild: {
modifyFs,
expectedDiagnostics: [
getExpectedDiagnosticForProjectsInBuild("src/lib/tsconfig.json", "src/app/tsconfig.json"),
[Diagnostics.Project_0_is_out_of_date_because_output_file_1_does_not_exist, "src/lib/tsconfig.json", "src/module.js"],
[Diagnostics.Building_project_0, sources[project.lib][source.config]],
[Diagnostics.Project_0_is_out_of_date_because_output_file_1_does_not_exist, "src/app/tsconfig.json", "src/app/module.js"],
[Diagnostics.Building_project_0, sources[project.app][source.config]],
]
},
outputFiles: [
...libOutputFile,
...outputFiles[project.app]
],
baselineOnly: true,
verifyDiagnostics: true
});
});
});
});
}
5 changes: 3 additions & 2 deletions src/testRunner/unittests/tsbuild/helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,10 +234,11 @@ Mismatch Actual(path, actual, expected): ${JSON.stringify(arrayFrom(mapDefinedIt
incrementalDtsUnchangedBuild?: BuildState;
incrementalHeaderChangedBuild?: BuildState;
baselineOnly?: true;
verifyDiagnostics?: true;
}

export function verifyTsbuildOutput({
scenario, projFs, time, tick, proj, rootNames, outputFiles, baselineOnly,
scenario, projFs, time, tick, proj, rootNames, outputFiles, baselineOnly, verifyDiagnostics,
expectedMapFileNames, expectedBuildInfoFilesForSectionBaselines, lastProjectOutputJs,
initialBuild, incrementalDtsChangedBuild, incrementalDtsUnchangedBuild, incrementalHeaderChangedBuild
}: VerifyTsBuildInput) {
Expand All @@ -264,7 +265,7 @@ Mismatch Actual(path, actual, expected): ${JSON.stringify(arrayFrom(mapDefinedIt
host = undefined!;
});
describe("initialBuild", () => {
if (!baselineOnly) {
if (!baselineOnly || verifyDiagnostics) {
it(`verify diagnostics`, () => {
host.assertDiagnosticMessages(...(initialBuild.expectedDiagnostics || emptyArray));
});
Expand Down
6 changes: 3 additions & 3 deletions src/testRunner/unittests/tsbuild/sample.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -427,14 +427,14 @@ export class cNew {}`);
builder.buildAllProjects();
assert.deepEqual(host.traces, [
"TSFILE: /src/core/anotherModule.js",
"TSFILE: /src/core/anotherModule.d.ts",
"TSFILE: /src/core/anotherModule.d.ts.map",
"TSFILE: /src/core/anotherModule.d.ts",
"TSFILE: /src/core/index.js",
"TSFILE: /src/core/index.d.ts",
"TSFILE: /src/core/index.d.ts.map",
"TSFILE: /src/core/index.d.ts",
"TSFILE: /src/core/tsconfig.tsbuildinfo",
"TSFILE: /src/logic/index.js",
"TSFILE: /src/logic/index.js.map",
"TSFILE: /src/logic/index.js",
"TSFILE: /src/logic/index.d.ts",
"TSFILE: /src/logic/tsconfig.tsbuildinfo",
"TSFILE: /src/tests/index.js",
Expand Down
2 changes: 2 additions & 0 deletions src/testRunner/unittests/tsbuild/transitiveReferences.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ export const b = new A();`);
const expectedFileTraces = [
...getLibs(),
"/src/a.ts",
...getLibs(),
"/src/b.ts"
];
verifyBuild(fs => modifyFsBTsToNonRelativeImport(fs, "node"),
allExpectedOutputs,
Expand Down
Loading