Skip to content
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.

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

Handle when change in type of dts may result in only dts emit but not js emit #30971

Merged
merged 8 commits into from Apr 24, 2019
151 changes: 93 additions & 58 deletions src/compiler/builder.ts
Expand Up @@ -238,10 +238,7 @@ namespace ts {

if (oldCompilerOptions && compilerOptionsAffectEmit(compilerOptions, oldCompilerOptions)) {
// Add all files to affectedFilesPendingEmit since emit changed
state.affectedFilesPendingEmit = concatenate(state.affectedFilesPendingEmit, newProgram.getSourceFiles().map(f => f.path));
if (state.affectedFilesPendingEmitIndex === undefined) {
state.affectedFilesPendingEmitIndex = 0;
}
addToAffectedFilesPendingEmit(state, newProgram.getSourceFiles().map(f => f.path));
Debug.assert(state.seenAffectedFiles === undefined);
state.seenAffectedFiles = createMap<true>();
}
Expand Down Expand Up @@ -339,7 +336,7 @@ namespace ts {
if (!seenAffectedFiles.has(affectedFile.path)) {
// Set the next affected file as seen and remove the cached semantic diagnostics
state.affectedFilesIndex = affectedFilesIndex;
cleanSemanticDiagnosticsOfAffectedFile(state, affectedFile);
handleDtsMayChangeOfAffectedFile(state, affectedFile, cancellationToken, computeHash);
return affectedFile;
}
seenAffectedFiles.set(affectedFile.path, true);
Expand Down Expand Up @@ -406,31 +403,83 @@ namespace ts {
}

/**
* Remove the semantic diagnostics cached from old state for affected File and the files that are referencing modules that export entities from affected file
* Handles semantic diagnostics and dts emit for affectedFile and files, that are referencing modules that export entities from affected file
* This is because even though js emit doesnt change, dts emit / type used can change resulting in need for dts emit and js change
*/
function cleanSemanticDiagnosticsOfAffectedFile(state: BuilderProgramState, affectedFile: SourceFile) {
if (removeSemanticDiagnosticsOf(state, affectedFile.path)) {
// If there are no more diagnostics from old cache, done
function handleDtsMayChangeOfAffectedFile(state: BuilderProgramState, affectedFile: SourceFile, cancellationToken: CancellationToken | undefined, computeHash: BuilderState.ComputeHash) {
removeSemanticDiagnosticsOf(state, affectedFile.path);

// If affected files is everything except default library, then nothing more to do
if (state.allFilesExcludingDefaultLibraryFile === state.affectedFiles) {
if (!state.cleanedDiagnosticsOfLibFiles) {
state.cleanedDiagnosticsOfLibFiles = true;
const program = Debug.assertDefined(state.program);
const options = program.getCompilerOptions();
forEach(program.getSourceFiles(), f =>
program.isSourceFileDefaultLibrary(f) &&
!skipTypeChecking(f, options) &&
removeSemanticDiagnosticsOf(state, f.path)
);
}
return;
}

// Clean lib file diagnostics if its all files excluding default files to emit
if (state.allFilesExcludingDefaultLibraryFile === state.affectedFiles && !state.cleanedDiagnosticsOfLibFiles) {
state.cleanedDiagnosticsOfLibFiles = true;
forEachReferencingModulesOfExportOfAffectedFile(state, affectedFile, (state, path) => handleDtsMayChangeOf(state, path, cancellationToken, computeHash));
}

/**
* Handle the dts may change, so they need to be added to pending emit if dts emit is enabled,
* Also we need to make sure signature is updated for these files
*/
function handleDtsMayChangeOf(state: BuilderProgramState, path: Path, cancellationToken: CancellationToken | undefined, computeHash: BuilderState.ComputeHash) {
removeSemanticDiagnosticsOf(state, path);

if (!state.changedFilesSet.has(path)) {
const program = Debug.assertDefined(state.program);
const options = program.getCompilerOptions();
if (forEach(program.getSourceFiles(), f =>
program.isSourceFileDefaultLibrary(f) &&
!skipTypeChecking(f, options) &&
removeSemanticDiagnosticsOf(state, f.path)
)) {
return;
const sourceFile = program.getSourceFileByPath(path);
if (sourceFile) {
// Even though the js emit doesnt change and we are already handling dts emit and semantic diagnostics
// we need to update the signature to reflect correctness of the signature(which is output d.ts emit) of this file
// This ensures that we dont later during incremental builds considering wrong signature.
// Eg where this also is needed to ensure that .tsbuildinfo generated by incremental build should be same as if it was first fresh build
BuilderState.updateShapeSignature(
state,
program,
sourceFile,
Debug.assertDefined(state.currentAffectedFilesSignatures),
cancellationToken,
computeHash,
state.currentAffectedFilesExportedModulesMap
);
// If not dts emit, nothing more to do
if (getEmitDeclarations(state.compilerOptions)) {
addToAffectedFilesPendingEmit(state, [path]);
}
}
}

// If there was change in signature for the changed file,
// then delete the semantic diagnostics for files that are affected by using exports of this module
return false;
}

/**
* Removes semantic diagnostics for path and
* returns true if there are no more semantic diagnostics from the old state
*/
function removeSemanticDiagnosticsOf(state: BuilderProgramState, path: Path) {
if (!state.semanticDiagnosticsFromOldState) {
return true;
}
state.semanticDiagnosticsFromOldState.delete(path);
state.semanticDiagnosticsPerFile!.delete(path);
return !state.semanticDiagnosticsFromOldState.size;
}

/**
* Iterate on referencing modules that export entities from affected file
*/
function forEachReferencingModulesOfExportOfAffectedFile(state: BuilderProgramState, affectedFile: SourceFile, fn: (state: BuilderProgramState, filePath: Path) => boolean) {
// If there was change in signature (dts output) for the changed file,
// then only we need to handle pending file emit
if (!state.exportedModulesMap || state.affectedFiles!.length === 1 || !state.changedFilesSet.has(affectedFile.path)) {
return;
}
Expand All @@ -442,7 +491,7 @@ namespace ts {
if (forEachEntry(state.currentAffectedFilesExportedModulesMap!, (exportedModules, exportedFromPath) =>
exportedModules &&
exportedModules.has(affectedFile.path) &&
removeSemanticDiagnosticsOfFilesReferencingPath(state, exportedFromPath as Path, seenFileAndExportsOfFile)
forEachFilesReferencingPath(state, exportedFromPath as Path, seenFileAndExportsOfFile, fn)
)) {
return;
}
Expand All @@ -451,29 +500,28 @@ namespace ts {
forEachEntry(state.exportedModulesMap, (exportedModules, exportedFromPath) =>
!state.currentAffectedFilesExportedModulesMap!.has(exportedFromPath) && // If we already iterated this through cache, ignore it
exportedModules.has(affectedFile.path) &&
removeSemanticDiagnosticsOfFilesReferencingPath(state, exportedFromPath as Path, seenFileAndExportsOfFile)
forEachFilesReferencingPath(state, exportedFromPath as Path, seenFileAndExportsOfFile, fn)
);
}

/**
* removes the semantic diagnostics of files referencing referencedPath and
* returns true if there are no more semantic diagnostics from old state
* Iterate on files referencing referencedPath
*/
function removeSemanticDiagnosticsOfFilesReferencingPath(state: BuilderProgramState, referencedPath: Path, seenFileAndExportsOfFile: Map<true>) {
function forEachFilesReferencingPath(state: BuilderProgramState, referencedPath: Path, seenFileAndExportsOfFile: Map<true>, fn: (state: BuilderProgramState, filePath: Path) => boolean) {
return forEachEntry(state.referencedMap!, (referencesInFile, filePath) =>
referencesInFile.has(referencedPath) && removeSemanticDiagnosticsOfFileAndExportsOfFile(state, filePath as Path, seenFileAndExportsOfFile)
referencesInFile.has(referencedPath) && forEachFileAndExportsOfFile(state, filePath as Path, seenFileAndExportsOfFile, fn)
);
}

/**
* Removes semantic diagnostics of file and anything that exports this file
* fn on file and iterate on anything that exports this file
*/
function removeSemanticDiagnosticsOfFileAndExportsOfFile(state: BuilderProgramState, filePath: Path, seenFileAndExportsOfFile: Map<true>): boolean {
function forEachFileAndExportsOfFile(state: BuilderProgramState, filePath: Path, seenFileAndExportsOfFile: Map<true>, fn: (state: BuilderProgramState, filePath: Path) => boolean): boolean {
if (!addToSeen(seenFileAndExportsOfFile, filePath)) {
return false;
}

if (removeSemanticDiagnosticsOf(state, filePath)) {
if (fn(state, filePath)) {
// If there are no more diagnostics from old cache, done
return true;
}
Expand All @@ -484,7 +532,7 @@ namespace ts {
if (forEachEntry(state.currentAffectedFilesExportedModulesMap!, (exportedModules, exportedFromPath) =>
exportedModules &&
exportedModules.has(filePath) &&
removeSemanticDiagnosticsOfFileAndExportsOfFile(state, exportedFromPath as Path, seenFileAndExportsOfFile)
forEachFileAndExportsOfFile(state, exportedFromPath as Path, seenFileAndExportsOfFile, fn)
)) {
return true;
}
Expand All @@ -493,7 +541,7 @@ namespace ts {
if (forEachEntry(state.exportedModulesMap!, (exportedModules, exportedFromPath) =>
!state.currentAffectedFilesExportedModulesMap!.has(exportedFromPath) && // If we already iterated this through cache, ignore it
exportedModules.has(filePath) &&
removeSemanticDiagnosticsOfFileAndExportsOfFile(state, exportedFromPath as Path, seenFileAndExportsOfFile)
forEachFileAndExportsOfFile(state, exportedFromPath as Path, seenFileAndExportsOfFile, fn)
)) {
return true;
}
Expand All @@ -502,22 +550,10 @@ namespace ts {
return !!forEachEntry(state.referencedMap!, (referencesInFile, referencingFilePath) =>
referencesInFile.has(filePath) &&
!seenFileAndExportsOfFile.has(referencingFilePath) && // Not already removed diagnostic file
removeSemanticDiagnosticsOf(state, referencingFilePath as Path) // Dont add to seen since this is not yet done with the export removal
fn(state, referencingFilePath as Path) // Dont add to seen since this is not yet done with the export removal
);
}

/**
* Removes semantic diagnostics for path and
* returns true if there are no more semantic diagnostics from the old state
*/
function removeSemanticDiagnosticsOf(state: BuilderProgramState, path: Path) {
if (!state.semanticDiagnosticsFromOldState) {
return true;
}
state.semanticDiagnosticsFromOldState.delete(path);
state.semanticDiagnosticsPerFile!.delete(path);
return !state.semanticDiagnosticsFromOldState.size;
}

/**
* This is called after completing operation on the next affected file.
Expand Down Expand Up @@ -808,11 +844,6 @@ namespace ts {
}
}

// Mark seen emitted files if there are pending files to be emitted
sheetalkamat marked this conversation as resolved.
Show resolved Hide resolved
if (state.affectedFilesPendingEmit && state.program !== affected) {
(state.seenEmittedFiles || (state.seenEmittedFiles = createMap())).set((affected as SourceFile).path, true);
}

return toAffectedFileResult(
state,
// When whole program is affected, do emit only once (eg when --out or --outFile is specified)
Expand Down Expand Up @@ -931,14 +962,7 @@ namespace ts {

// In case of emit builder, cache the files to be emitted
if (affectedFilesPendingEmit) {
state.affectedFilesPendingEmit = concatenate(state.affectedFilesPendingEmit, affectedFilesPendingEmit);
// affectedFilesPendingEmitIndex === undefined
// - means the emit state.affectedFilesPendingEmit was undefined before adding current affected files
// so start from 0 as array would be affectedFilesPendingEmit
// else, continue to iterate from existing index, the current set is appended to existing files
if (state.affectedFilesPendingEmitIndex === undefined) {
state.affectedFilesPendingEmitIndex = 0;
}
addToAffectedFilesPendingEmit(state, affectedFilesPendingEmit);
}

let diagnostics: Diagnostic[] | undefined;
Expand All @@ -949,6 +973,17 @@ namespace ts {
}
}

function addToAffectedFilesPendingEmit(state: BuilderProgramState, affectedFilesPendingEmit: readonly Path[]) {
state.affectedFilesPendingEmit = concatenate(state.affectedFilesPendingEmit, affectedFilesPendingEmit);
// affectedFilesPendingEmitIndex === undefined
// - means the emit state.affectedFilesPendingEmit was undefined before adding current affected files
// so start from 0 as array would be affectedFilesPendingEmit
// else, continue to iterate from existing index, the current set is appended to existing files
if (state.affectedFilesPendingEmitIndex === undefined) {
state.affectedFilesPendingEmitIndex = 0;
}
}

function getMapOfReferencedSet(mapLike: MapLike<ReadonlyArray<string>> | undefined): ReadonlyMap<BuilderState.ReferencedSet> | undefined {
if (!mapLike) return undefined;
const map = createMap<BuilderState.ReferencedSet>();
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/builderState.ts
Expand Up @@ -321,7 +321,7 @@ namespace ts.BuilderState {
/**
* Returns if the shape of the signature has changed since last emit
*/
function updateShapeSignature(state: Readonly<BuilderState>, programOfThisState: Program, sourceFile: SourceFile, cacheToUpdateSignature: Map<string>, cancellationToken: CancellationToken | undefined, computeHash: ComputeHash, exportedModulesMapCache?: ComputingExportedModulesMap) {
export function updateShapeSignature(state: Readonly<BuilderState>, programOfThisState: Program, sourceFile: SourceFile, cacheToUpdateSignature: Map<string>, cancellationToken: CancellationToken | undefined, computeHash: ComputeHash, exportedModulesMapCache?: ComputingExportedModulesMap) {
Debug.assert(!!sourceFile);
Debug.assert(!exportedModulesMapCache || !!state.exportedModulesMap, "Compute visible to outside map only if visibleToOutsideReferencedMap present in the state");

Expand Down
1 change: 1 addition & 0 deletions src/testRunner/tsconfig.json
Expand Up @@ -92,6 +92,7 @@
"unittests/tsbuild/amdModulesWithOut.ts",
"unittests/tsbuild/emptyFiles.ts",
"unittests/tsbuild/graphOrdering.ts",
"unittests/tsbuild/inferredTypeFromTransitiveModule.ts",
"unittests/tsbuild/lateBoundSymbol.ts",
"unittests/tsbuild/missingExtendedFile.ts",
"unittests/tsbuild/outFile.ts",
Expand Down
@@ -0,0 +1,49 @@
namespace ts {
describe("unittests:: tsbuild:: inferredTypeFromTransitiveModule::", () => {
let projFs: vfs.FileSystem;
const { time, tick } = getTime();
before(() => {
projFs = loadProjectFromDisk("tests/projects/inferredTypeFromTransitiveModule", time);
});
after(() => {
projFs = undefined!;
});

verifyTsbuildOutput({
scenario: "inferred type from transitive module",
projFs: () => projFs,
time,
tick,
proj: "inferredTypeFromTransitiveModule",
rootNames: ["/src"],
expectedMapFileNames: emptyArray,
lastProjectOutputJs: `/src/obj/index.js`,
outputFiles: [
"/src/obj/bar.js", "/src/obj/bar.d.ts",
"/src/obj/bundling.js", "/src/obj/bundling.d.ts",
"/src/obj/lazyIndex.js", "/src/obj/lazyIndex.d.ts",
"/src/obj/index.js", "/src/obj/index.d.ts",
"/src/obj/tsconfig.tsbuildinfo"
],
initialBuild: {
modifyFs: noop,
expectedDiagnostics: [
getExpectedDiagnosticForProjectsInBuild("src/tsconfig.json"),
[Diagnostics.Project_0_is_out_of_date_because_output_file_1_does_not_exist, "src/tsconfig.json", "src/obj/bar.js"],
[Diagnostics.Building_project_0, "/src/tsconfig.json"]
]
},
incrementalDtsChangedBuild: {
modifyFs: fs => replaceText(fs, "/src/bar.ts", "param: string", ""),
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/obj/bar.js", "src/bar.ts"],
[Diagnostics.Building_project_0, "/src/tsconfig.json"],
[Diagnostics.Updating_unchanged_output_timestamps_of_project_0, "/src/tsconfig.json"]
]
},
baselineOnly: true,
verifyDiagnostics: true
});
});
}