Skip to content

Commit

Permalink
If we are updating dts of any of the file and it affects global scope…
Browse files Browse the repository at this point in the history 
…, everything needs update in signature and dts emit (#48600)

* Add test

* Renames

* More refactoring

* If we are updating dts of any of the file and it affects global scope, everything needs update in signature and dts emit
Fixes #42769
  • Loading branch information
@sheetalkamat
sheetalkamat authored Apr 21, 2022
1 parent 273a567 commit 45faac7
Show file tree
Hide file tree
Showing 4 changed files with 681 additions and 68 deletions.
198 changes: 130 additions & 68 deletions src/compiler/builder.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -434,25 +434,35 @@ namespace ts {
return undefined;
}

function removeDiagnosticsOfLibraryFiles(state: BuilderProgramState) {
if (!state.cleanedDiagnosticsOfLibFiles) {
state.cleanedDiagnosticsOfLibFiles = true;
const program = Debug.checkDefined(state.program);
const options = program.getCompilerOptions();
forEach(program.getSourceFiles(), f =>
program.isSourceFileDefaultLibrary(f) &&
!skipTypeChecking(f, options, program) &&
removeSemanticDiagnosticsOf(state, f.resolvedPath)
);
}
}

/**
* 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 handleDtsMayChangeOfAffectedFile(state: BuilderProgramState, affectedFile: SourceFile, cancellationToken: CancellationToken | undefined, computeHash: BuilderState.ComputeHash, host: BuilderProgramHost) {
function handleDtsMayChangeOfAffectedFile(
state: BuilderProgramState,
affectedFile: SourceFile,
cancellationToken: CancellationToken | undefined,
computeHash: BuilderState.ComputeHash,
host: BuilderProgramHost,
) {
removeSemanticDiagnosticsOf(state, affectedFile.resolvedPath);

// 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.checkDefined(state.program);
const options = program.getCompilerOptions();
forEach(program.getSourceFiles(), f =>
program.isSourceFileDefaultLibrary(f) &&
!skipTypeChecking(f, options, program) &&
removeSemanticDiagnosticsOf(state, f.resolvedPath)
);
}
removeDiagnosticsOfLibraryFiles(state);
// When a change affects the global scope, all files are considered to be affected without updating their signature
// That means when affected file is handled, its signature can be out of date
// To avoid this, ensure that we update the signature for any affected file in this scenario.
Expand All @@ -467,20 +477,22 @@ namespace ts {
);
return;
}
else {
Debug.assert(state.hasCalledUpdateShapeSignature.has(affectedFile.resolvedPath) || state.currentAffectedFilesSignatures?.has(affectedFile.resolvedPath), `Signature not updated for affected file: ${affectedFile.fileName}`);
}

if (!state.compilerOptions.assumeChangesOnlyAffectDirectDependencies) {
forEachReferencingModulesOfExportOfAffectedFile(state, affectedFile, (state, path) => handleDtsMayChangeOf(state, path, cancellationToken, computeHash, host));
}
Debug.assert(state.hasCalledUpdateShapeSignature.has(affectedFile.resolvedPath) || state.currentAffectedFilesSignatures?.has(affectedFile.resolvedPath), `Signature not updated for affected file: ${affectedFile.fileName}`);
if (state.compilerOptions.assumeChangesOnlyAffectDirectDependencies) return;
handleDtsMayChangeOfReferencingExportOfAffectedFile(state, affectedFile, cancellationToken, computeHash, host);
}

/**
* 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, host: BuilderProgramHost): void {
function handleDtsMayChangeOf(
state: BuilderProgramState,
path: Path,
cancellationToken: CancellationToken | undefined,
computeHash: BuilderState.ComputeHash,
host: BuilderProgramHost
): void {
removeSemanticDiagnosticsOf(state, path);

if (!state.changedFilesSet.has(path)) {
Expand Down Expand Up @@ -529,16 +541,61 @@ namespace ts {
return newSignature !== oldSignature;
}

function forEachKeyOfExportedModulesMap<T>(
state: BuilderProgramState,
filePath: Path,
fn: (exportedFromPath: Path) => T | undefined,
): T | undefined {
// Go through exported modules from cache first
let keys = state.currentAffectedFilesExportedModulesMap!.getKeys(filePath);
const result = keys && forEachKey(keys, fn);
if (result) return result;

// If exported from path is not from cache and exported modules has path, all files referencing file exported from are affected
keys = state.exportedModulesMap!.getKeys(filePath);
return keys && forEachKey(keys, exportedFromPath =>
// If the cache had an updated value, skip
!state.currentAffectedFilesExportedModulesMap!.hasKey(exportedFromPath) &&
!state.currentAffectedFilesExportedModulesMap!.deletedKeys()?.has(exportedFromPath) ?
fn(exportedFromPath) :
undefined
);
}

function handleDtsMayChangeOfGlobalScope(
state: BuilderProgramState,
filePath: Path,
cancellationToken: CancellationToken | undefined,
computeHash: BuilderState.ComputeHash,
host: BuilderProgramHost,
): boolean {
if (!state.fileInfos.get(filePath)?.affectsGlobalScope) return false;
// Every file needs to be handled
BuilderState.getAllFilesExcludingDefaultLibraryFile(state, state.program!, /*firstSourceFile*/ undefined)
.forEach(file => handleDtsMayChangeOf(
state,
file.resolvedPath,
cancellationToken,
computeHash,
host,
));
removeDiagnosticsOfLibraryFiles(state);
return true;
}

/**
* Iterate on referencing modules that export entities from affected file
* Iterate on referencing modules that export entities from affected file and delete diagnostics and add pending emit
*/
function forEachReferencingModulesOfExportOfAffectedFile(state: BuilderProgramState, affectedFile: SourceFile, fn: (state: BuilderProgramState, filePath: Path) => void) {
function handleDtsMayChangeOfReferencingExportOfAffectedFile(
state: BuilderProgramState,
affectedFile: SourceFile,
cancellationToken: CancellationToken | undefined,
computeHash: BuilderState.ComputeHash,
host: BuilderProgramHost
) {
// 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.changedFilesSet.has(affectedFile.resolvedPath)) {
return;
}

if (!state.exportedModulesMap || !state.changedFilesSet.has(affectedFile.resolvedPath)) return;
if (!isChangedSignature(state, affectedFile.resolvedPath)) return;

// Since isolated modules dont change js files, files affected by change in signature is itself
Expand All @@ -551,7 +608,8 @@ namespace ts {
const currentPath = queue.pop()!;
if (!seenFileNamesMap.has(currentPath)) {
seenFileNamesMap.set(currentPath, true);
fn(state, currentPath);
if (handleDtsMayChangeOfGlobalScope(state, currentPath, cancellationToken, computeHash, host)) return;
handleDtsMayChangeOf(state, currentPath, cancellationToken, computeHash, host);
if (isChangedSignature(state, currentPath)) {
const currentSourceFile = Debug.checkDefined(state.program).getSourceFileByPath(currentPath)!;
queue.push(...BuilderState.getReferencedByPaths(state, currentSourceFile.resolvedPath));
Expand All @@ -561,65 +619,69 @@ namespace ts {
}

Debug.assert(!!state.currentAffectedFilesExportedModulesMap);

const seenFileAndExportsOfFile = new Set<string>();
// Go through exported modules from cache first
// If exported modules has path, all files referencing file exported from are affected
state.currentAffectedFilesExportedModulesMap.getKeys(affectedFile.resolvedPath)?.forEach(exportedFromPath =>
forEachFilesReferencingPath(state, exportedFromPath, seenFileAndExportsOfFile, fn)
);

// If exported from path is not from cache and exported modules has path, all files referencing file exported from are affected
state.exportedModulesMap.getKeys(affectedFile.resolvedPath)?.forEach(exportedFromPath =>
// If the cache had an updated value, skip
!state.currentAffectedFilesExportedModulesMap!.hasKey(exportedFromPath) &&
!state.currentAffectedFilesExportedModulesMap!.deletedKeys()?.has(exportedFromPath) &&
forEachFilesReferencingPath(state, exportedFromPath, seenFileAndExportsOfFile, fn)
);
}

/**
* Iterate on files referencing referencedPath
*/
function forEachFilesReferencingPath(state: BuilderProgramState, referencedPath: Path, seenFileAndExportsOfFile: Set<string>, fn: (state: BuilderProgramState, filePath: Path) => void): void {
state.referencedMap!.getKeys(referencedPath)?.forEach(filePath =>
forEachFileAndExportsOfFile(state, filePath, seenFileAndExportsOfFile, fn)
);
forEachKeyOfExportedModulesMap(state, affectedFile.resolvedPath, exportedFromPath => {
if (handleDtsMayChangeOfGlobalScope(state, exportedFromPath, cancellationToken, computeHash, host)) return true;
const references = state.referencedMap!.getKeys(exportedFromPath);
return references && forEachKey(references, filePath =>
handleDtsMayChangeOfFileAndExportsOfFile(
state,
filePath,
seenFileAndExportsOfFile,
cancellationToken,
computeHash,
host,
)
);
});
}

/**
* fn on file and iterate on anything that exports this file
* handle dts and semantic diagnostics on file and iterate on anything that exports this file
* return true when all work is done and we can exit handling dts emit and semantic diagnostics
*/
function forEachFileAndExportsOfFile(state: BuilderProgramState, filePath: Path, seenFileAndExportsOfFile: Set<string>, fn: (state: BuilderProgramState, filePath: Path) => void): void {
if (!tryAddToSet(seenFileAndExportsOfFile, filePath)) {
return;
}

fn(state, filePath);

function handleDtsMayChangeOfFileAndExportsOfFile(
state: BuilderProgramState,
filePath: Path,
seenFileAndExportsOfFile: Set<string>,
cancellationToken: CancellationToken | undefined,
computeHash: BuilderState.ComputeHash,
host: BuilderProgramHost,
): boolean | undefined {
if (!tryAddToSet(seenFileAndExportsOfFile, filePath)) return undefined;

if (handleDtsMayChangeOfGlobalScope(state, filePath, cancellationToken, computeHash, host)) return true;
handleDtsMayChangeOf(state, filePath, cancellationToken, computeHash, host);
Debug.assert(!!state.currentAffectedFilesExportedModulesMap);
// Go through exported modules from cache first
// If exported modules has path, all files referencing file exported from are affected
state.currentAffectedFilesExportedModulesMap.getKeys(filePath)?.forEach(exportedFromPath =>
forEachFileAndExportsOfFile(state, exportedFromPath, seenFileAndExportsOfFile, fn)
);

// If exported from path is not from cache and exported modules has path, all files referencing file exported from are affected
state.exportedModulesMap!.getKeys(filePath)?.forEach(exportedFromPath =>
// If the cache had an updated value, skip
!state.currentAffectedFilesExportedModulesMap!.hasKey(exportedFromPath) &&
!state.currentAffectedFilesExportedModulesMap!.deletedKeys()?.has(exportedFromPath) &&
forEachFileAndExportsOfFile(state, exportedFromPath, seenFileAndExportsOfFile, fn)
// If exported modules has path, all files referencing file exported from are affected
forEachKeyOfExportedModulesMap(state, filePath, exportedFromPath =>
handleDtsMayChangeOfFileAndExportsOfFile(
state,
exportedFromPath,
seenFileAndExportsOfFile,
cancellationToken,
computeHash,
host,
)
);

// Remove diagnostics of files that import this file (without going to exports of referencing files)
state.referencedMap!.getKeys(filePath)?.forEach(referencingFilePath =>
!seenFileAndExportsOfFile.has(referencingFilePath) && // Not already removed diagnostic file
fn(state, referencingFilePath) // Dont add to seen since this is not yet done with the export removal
handleDtsMayChangeOf( // Dont add to seen since this is not yet done with the export removal
state,
referencingFilePath,
cancellationToken,
computeHash,
host,
)
);
return undefined;
}


/**
* 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
Expand Down
35 changes: 35 additions & 0 deletions src/testRunner/unittests/tsc/incremental.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -464,5 +464,40 @@ declare global {
fs.writeFileSync("/lib/lib.esnext.d.ts", libContent);
}
});

verifyTscWithEdits({
scenario: "incremental",
subScenario: "change to type that gets used as global through export in another file",
commandLineArgs: ["-p", `src/project`],
fs: () => loadProjectFromFiles({
"/src/project/tsconfig.json": JSON.stringify({ compilerOptions: { composite: true }, }),
"/src/project/class1.ts": `const a: MagicNumber = 1;
console.log(a);`,
"/src/project/constants.ts": "export default 1;",
"/src/project/types.d.ts": `type MagicNumber = typeof import('./constants').default`,
}),
edits: [{
subScenario: "Modify imports used in global file",
modifyFs: fs => fs.writeFileSync("/src/project/constants.ts", "export default 2;"),
}],
});

verifyTscWithEdits({
scenario: "incremental",
subScenario: "change to type that gets used as global through export in another file through indirect import",
commandLineArgs: ["-p", `src/project`],
fs: () => loadProjectFromFiles({
"/src/project/tsconfig.json": JSON.stringify({ compilerOptions: { composite: true }, }),
"/src/project/class1.ts": `const a: MagicNumber = 1;
console.log(a);`,
"/src/project/constants.ts": "export default 1;",
"/src/project/reexport.ts": `export { default as ConstantNumber } from "./constants"`,
"/src/project/types.d.ts": `type MagicNumber = typeof import('./reexport').ConstantNumber`,
}),
edits: [{
subScenario: "Modify imports used in global file",
modifyFs: fs => fs.writeFileSync("/src/project/constants.ts", "export default 2;"),
}],
});
});
}
Loading

0 comments on commit 45faac7

Please sign in to comment.