Skip to content

Commit

Permalink
Handle seenEmittedFiles which was not being set when emit of a file w…
Browse files Browse the repository at this point in the history
…as complete (#33145)

* Add test that fails because file is written multiple times
Reported from #33061

* Handle seenEmittedFiles which was not being set when emit of a file was complete.
It was issue only when errors are reported before emitting (which puts the files into pendingEmit that needs to check only in seenEmittedFiles)
If emit happens before semantic diagnostics query this issue is not repro, because the affected files come into play and those are being set correctly
Fixes #31398

* make baselining source map optional

* Handle emitDeclarationOnly in --build scenario

* Ensure we are using d.ts emit as signature even when --declarationMap is on (map files are emitted before d.ts)

* Move module specifiers to verifyTsBuildOutput

* implement create Hash to be default hashing plus data so we can verify it easily in baseline

* Remove failing baseline

* Accept correct baseline name
  • Loading branch information
sheetalkamat committed Aug 30, 2019
1 parent 5ea4257 commit 79bcb3d
Show file tree
Hide file tree
Showing 46 changed files with 1,571 additions and 676 deletions.
24 changes: 17 additions & 7 deletions src/compiler/builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ namespace ts {
*/
emittedBuildInfo?: boolean;
/**
* Already seen affected files
* Already seen emitted files
*/
seenEmittedFiles: Map<true> | undefined;
/**
Expand Down Expand Up @@ -329,7 +329,6 @@ namespace ts {
handleDtsMayChangeOfAffectedFile(state, affectedFile, cancellationToken, computeHash);
return affectedFile;
}
seenAffectedFiles.set(affectedFile.path, true);
affectedFilesIndex++;
}

Expand Down Expand Up @@ -549,7 +548,7 @@ namespace ts {
* This is called after completing operation on the next affected file.
* The operations here are postponed to ensure that cancellation during the iteration is handled correctly
*/
function doneWithAffectedFile(state: BuilderProgramState, affected: SourceFile | Program, isPendingEmit?: boolean, isBuildInfoEmit?: boolean) {
function doneWithAffectedFile(state: BuilderProgramState, affected: SourceFile | Program, isPendingEmit?: boolean, isBuildInfoEmit?: boolean, isEmitResult?: boolean) {
if (isBuildInfoEmit) {
state.emittedBuildInfo = true;
}
Expand All @@ -559,6 +558,9 @@ namespace ts {
}
else {
state.seenAffectedFiles!.set((affected as SourceFile).path, true);
if (isEmitResult) {
(state.seenEmittedFiles || (state.seenEmittedFiles = createMap())).set((affected as SourceFile).path, true);
}
if (isPendingEmit) {
state.affectedFilesPendingEmitIndex!++;
}
Expand All @@ -576,6 +578,14 @@ namespace ts {
return { result, affected };
}

/**
* Returns the result with affected file
*/
function toAffectedFileEmitResult(state: BuilderProgramState, result: EmitResult, affected: SourceFile | Program, isPendingEmit?: boolean, isBuildInfoEmit?: boolean): AffectedFileResult<EmitResult> {
doneWithAffectedFile(state, affected, isPendingEmit, isBuildInfoEmit, /*isEmitResult*/ true);
return { result, affected };
}

/**
* Gets the semantic diagnostics either from cache if present, or otherwise from program and caches it
* Note that it is assumed that the when asked about semantic diagnostics, the file has been taken out of affected files/changed file set
Expand Down Expand Up @@ -849,7 +859,7 @@ namespace ts {
}

const affected = Debug.assertDefined(state.program);
return toAffectedFileResult(
return toAffectedFileEmitResult(
state,
// When whole program is affected, do emit only once (eg when --out or --outFile is specified)
// Otherwise just affected file
Expand All @@ -872,14 +882,14 @@ namespace ts {
}
}

return toAffectedFileResult(
return toAffectedFileEmitResult(
state,
// When whole program is affected, do emit only once (eg when --out or --outFile is specified)
// Otherwise just affected file
Debug.assertDefined(state.program).emit(affected === state.program ? undefined : affected as SourceFile, writeFile || maybeBind(host, host.writeFile), cancellationToken, emitOnlyDtsFiles, customTransformers),
affected,
isPendingEmitFile
);
isPendingEmitFile,
);
}

/**
Expand Down
9 changes: 7 additions & 2 deletions src/compiler/builderState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -345,8 +345,13 @@ namespace ts.BuilderState {
}
else {
const emitOutput = getFileEmitOutput(programOfThisState, sourceFile, /*emitOnlyDtsFiles*/ true, cancellationToken);
if (emitOutput.outputFiles && emitOutput.outputFiles.length > 0) {
latestSignature = computeHash(emitOutput.outputFiles[0].text);
const firstDts = emitOutput.outputFiles &&
programOfThisState.getCompilerOptions().declarationMap ?
emitOutput.outputFiles.length > 1 ? emitOutput.outputFiles[1] : undefined :
emitOutput.outputFiles.length > 0 ? emitOutput.outputFiles[0] : undefined;
if (firstDts) {
Debug.assert(fileExtensionIs(firstDts.name, Extension.Dts), "File extension for signature expected to be dts", () => `Found: ${getAnyExtensionFromPath(firstDts.name)} for ${firstDts.name}:: All output files: ${JSON.stringify(emitOutput.outputFiles.map(f => f.name))}`);
latestSignature = computeHash(firstDts.text);
if (exportedModulesMapCache && latestSignature !== prevSignature) {
updateExportedModules(sourceFile, emitOutput.exportedModulesFromDeclarationEmit, exportedModulesMapCache);
}
Expand Down
7 changes: 6 additions & 1 deletion src/compiler/emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ namespace ts {
}

function getOutputJSFileName(inputFileName: string, configFile: ParsedCommandLine, ignoreCase: boolean) {
if (configFile.options.emitDeclarationOnly) return undefined;
const isJsonFile = fileExtensionIs(inputFileName, Extension.Json);
const outputFileName = changeExtension(
getOutputPathWithoutChangingExt(inputFileName, configFile, ignoreCase, configFile.options.outDir),
Expand Down Expand Up @@ -187,7 +188,7 @@ namespace ts {
const js = getOutputJSFileName(inputFileName, configFile, ignoreCase);
addOutput(js);
if (fileExtensionIs(inputFileName, Extension.Json)) continue;
if (configFile.options.sourceMap) {
if (js && configFile.options.sourceMap) {
addOutput(`${js}.map`);
}
if (getEmitDeclarations(configFile.options) && hasTSFileExtension(inputFileName)) {
Expand All @@ -214,6 +215,10 @@ namespace ts {
if (fileExtensionIs(inputFileName, Extension.Dts)) continue;
const jsFilePath = getOutputJSFileName(inputFileName, configFile, ignoreCase);
if (jsFilePath) return jsFilePath;
if (fileExtensionIs(inputFileName, Extension.Json)) continue;
if (getEmitDeclarations(configFile.options) && hasTSFileExtension(inputFileName)) {
return getOutputDeclarationFileName(inputFileName, configFile, ignoreCase);
}
}
const buildInfoPath = getOutputPathForBuildInfo(configFile.options);
if (buildInfoPath) return buildInfoPath;
Expand Down
13 changes: 13 additions & 0 deletions src/harness/fakes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -545,6 +545,10 @@ ${indentText}${text}`;
super.writeFile(fileName, ts.getBuildInfoText(buildInfo), writeByteOrderMark);
}

createHash(data: string) {
return `${ts.generateDjb2Hash(data)}-${data}`;
}

now() {
return new Date(this.sys.vfs.time());
}
Expand All @@ -571,6 +575,15 @@ Actual: ${JSON.stringify(actual, /*replacer*/ undefined, " ")}
Expected: ${JSON.stringify(expected, /*replacer*/ undefined, " ")}`);
}

assertErrors(...expectedDiagnostics: ExpectedErrorDiagnostic[]) {
const actual = this.diagnostics.filter(d => d.kind === DiagnosticKind.Error).map(diagnosticToText);
const expected = expectedDiagnostics.map(expectedDiagnosticToText);
assert.deepEqual(actual, expected, `Diagnostics arrays did not match:
Actual: ${JSON.stringify(actual, /*replacer*/ undefined, " ")}
Expected: ${JSON.stringify(expected, /*replacer*/ undefined, " ")}
Actual All:: ${JSON.stringify(this.diagnostics.slice().map(diagnosticToText), /*replacer*/ undefined, " ")}`);
}

printDiagnostics(header = "== Diagnostics ==") {
const out = ts.createDiagnosticReporter(ts.sys);
ts.sys.write(header + "\r\n");
Expand Down
1 change: 1 addition & 0 deletions src/testRunner/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@
"unittests/tsbuild/amdModulesWithOut.ts",
"unittests/tsbuild/containerOnlyReferenced.ts",
"unittests/tsbuild/demo.ts",
"unittests/tsbuild/emitDeclarationOnly.ts",
"unittests/tsbuild/emptyFiles.ts",
"unittests/tsbuild/graphOrdering.ts",
"unittests/tsbuild/inferredTypeFromTransitiveModule.ts",
Expand Down
4 changes: 2 additions & 2 deletions src/testRunner/unittests/tsbuild/amdModulesWithOut.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ namespace ts {
[outputFiles[project.lib][ext.buildinfo], outputFiles[project.lib][ext.js], outputFiles[project.lib][ext.dts]],
[outputFiles[project.app][ext.buildinfo], outputFiles[project.app][ext.js], outputFiles[project.app][ext.dts]]
],
lastProjectOutputJs: outputFiles[project.app][ext.js],
lastProjectOutput: outputFiles[project.app][ext.js],
initialBuild: {
modifyFs
},
Expand Down Expand Up @@ -231,7 +231,7 @@ ${internal} export enum internalEnum { a, b, c }`);
[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],
lastProjectOutput: outputFiles[project.app][ext.js],
initialBuild: {
modifyFs,
expectedDiagnostics: [
Expand Down
109 changes: 109 additions & 0 deletions src/testRunner/unittests/tsbuild/emitDeclarationOnly.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
namespace ts {
describe("unittests:: tsbuild:: on project with emitDeclarationOnly set to true", () => {
let projFs: vfs.FileSystem;
const { time, tick } = getTime();
before(() => {
projFs = loadProjectFromDisk("tests/projects/emitDeclarationOnly", time);
});
after(() => {
projFs = undefined!;
});

function verifyEmitDeclarationOnly(disableMap?: true) {
verifyTsbuildOutput({
scenario: `only dts output in circular import project with emitDeclarationOnly${disableMap ? "" : " and declarationMap"}`,
projFs: () => projFs,
time,
tick,
proj: "emitDeclarationOnly",
rootNames: ["/src"],
lastProjectOutput: `/src/lib/index.d.ts`,
outputFiles: [
"/src/lib/a.d.ts",
"/src/lib/b.d.ts",
"/src/lib/c.d.ts",
"/src/lib/index.d.ts",
"/src/tsconfig.tsbuildinfo",
...(disableMap ? emptyArray : [
"/src/lib/a.d.ts.map",
"/src/lib/b.d.ts.map",
"/src/lib/c.d.ts.map",
"/src/lib/index.d.ts.map"
])
],
initialBuild: {
modifyFs: disableMap ?
(fs => replaceText(fs, "/src/tsconfig.json", `"declarationMap": true,`, "")) :
noop,
expectedDiagnostics: [
getExpectedDiagnosticForProjectsInBuild("src/tsconfig.json"),
[Diagnostics.Project_0_is_out_of_date_because_output_file_1_does_not_exist, "src/tsconfig.json", "src/lib/a.d.ts"],
[Diagnostics.Building_project_0, "/src/tsconfig.json"]
]
},
incrementalDtsChangedBuild: {
modifyFs: fs => replaceText(fs, "/src/src/a.ts", "b: B;", "b: B; foo: any;"),
expectedDiagnostics: [
getExpectedDiagnosticForProjectsInBuild("src/tsconfig.json"),
[Diagnostics.Project_0_is_out_of_date_because_oldest_output_1_is_older_than_newest_input_2, "src/tsconfig.json", "src/lib/a.d.ts", "src/src/a.ts"],
[Diagnostics.Building_project_0, "/src/tsconfig.json"]
]
},
baselineOnly: true,
verifyDiagnostics: true
});
}
verifyEmitDeclarationOnly();
verifyEmitDeclarationOnly(/*disableMap*/ true);

verifyTsbuildOutput({
scenario: `only dts output in non circular imports project with emitDeclarationOnly`,
projFs: () => projFs,
time,
tick,
proj: "emitDeclarationOnly",
rootNames: ["/src"],
lastProjectOutput: `/src/lib/a.d.ts`,
outputFiles: [
"/src/lib/a.d.ts",
"/src/lib/b.d.ts",
"/src/lib/c.d.ts",
"/src/tsconfig.tsbuildinfo",
"/src/lib/a.d.ts.map",
"/src/lib/b.d.ts.map",
"/src/lib/c.d.ts.map",
],
initialBuild: {
modifyFs: fs => {
fs.rimrafSync("/src/src/index.ts");
replaceText(fs, "/src/src/a.ts", `import { B } from "./b";`, `export class B { prop = "hello"; }`);
},
expectedDiagnostics: [
getExpectedDiagnosticForProjectsInBuild("src/tsconfig.json"),
[Diagnostics.Project_0_is_out_of_date_because_output_file_1_does_not_exist, "src/tsconfig.json", "src/lib/a.d.ts"],
[Diagnostics.Building_project_0, "/src/tsconfig.json"]
]
},
incrementalDtsChangedBuild: {
modifyFs: fs => replaceText(fs, "/src/src/a.ts", "b: B;", "b: B; foo: any;"),
expectedDiagnostics: [
getExpectedDiagnosticForProjectsInBuild("src/tsconfig.json"),
[Diagnostics.Project_0_is_out_of_date_because_oldest_output_1_is_older_than_newest_input_2, "src/tsconfig.json", "src/lib/a.d.ts", "src/src/a.ts"],
[Diagnostics.Building_project_0, "/src/tsconfig.json"]
]
},
incrementalDtsUnchangedBuild: {
modifyFs: fs => replaceText(fs, "/src/src/a.ts", "export interface A {", `class C { }
export interface A {`),
expectedDiagnostics: [
getExpectedDiagnosticForProjectsInBuild("src/tsconfig.json"),
[Diagnostics.Project_0_is_out_of_date_because_oldest_output_1_is_older_than_newest_input_2, "src/tsconfig.json", "src/lib/a.d.ts", "src/src/a.ts"],
[Diagnostics.Building_project_0, "/src/tsconfig.json"],
[Diagnostics.Updating_unchanged_output_timestamps_of_project_0, "/src/tsconfig.json"]
]
},
baselineOnly: true,
verifyDiagnostics: true
});
});
}

0 comments on commit 79bcb3d

Please sign in to comment.