Skip to content

If we are updating dts of any of the file and it affects global scope, everything needs update in signature and dts emit #48600

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

Merged
merged 4 commits into from
Apr 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
198 changes: 130 additions & 68 deletions src/compiler/builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -433,25 +433,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 @@ -466,20 +476,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 @@ -528,16 +540,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 @@ -550,7 +607,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 @@ -560,65 +618,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
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