Skip to content

Commit cdd7ffd

Browse files
authored
Make handleDtsMayChangeOf void-returning (#44322)
* Make handleDtsMayChangeOf void-returning Right now, it always returns false. This seems important, since otherwise it would stop graph traversals prematurely. It took me a while to figure that out though and I thought it might be clearer if it were simply void-returning. I've kept the behavior the same, except in `forEachReferencingModulesOfExportOfAffectedFile`, where it seemed like never enqueueing new references was a mistake. * Make forEachFileAndExportsOfFile void-returning As far as I can tell, it could only return `false`.
1 parent 41f7887 commit cdd7ffd

File tree

1 file changed

+16
-26
lines changed

1 file changed

+16
-26
lines changed

src/compiler/builder.ts

+16-26
Original file line numberDiff line numberDiff line change
@@ -491,8 +491,6 @@ namespace ts {
491491
}
492492
}
493493
}
494-
495-
return false;
496494
}
497495

498496
/**
@@ -517,7 +515,7 @@ namespace ts {
517515
/**
518516
* Iterate on referencing modules that export entities from affected file
519517
*/
520-
function forEachReferencingModulesOfExportOfAffectedFile(state: BuilderProgramState, affectedFile: SourceFile, fn: (state: BuilderProgramState, filePath: Path) => boolean) {
518+
function forEachReferencingModulesOfExportOfAffectedFile(state: BuilderProgramState, affectedFile: SourceFile, fn: (state: BuilderProgramState, filePath: Path) => void) {
521519
// If there was change in signature (dts output) for the changed file,
522520
// then only we need to handle pending file emit
523521
if (!state.exportedModulesMap || !state.changedFilesSet.has(affectedFile.resolvedPath)) {
@@ -536,8 +534,8 @@ namespace ts {
536534
const currentPath = queue.pop()!;
537535
if (!seenFileNamesMap.has(currentPath)) {
538536
seenFileNamesMap.set(currentPath, true);
539-
const result = fn(state, currentPath);
540-
if (result && isChangedSignature(state, currentPath)) {
537+
fn(state, currentPath);
538+
if (isChangedSignature(state, currentPath)) {
541539
const currentSourceFile = Debug.checkDefined(state.program).getSourceFileByPath(currentPath)!;
542540
queue.push(...BuilderState.getReferencedByPaths(state, currentSourceFile.resolvedPath));
543541
}
@@ -549,13 +547,11 @@ namespace ts {
549547
const seenFileAndExportsOfFile = new Set<string>();
550548
// Go through exported modules from cache first
551549
// If exported modules has path, all files referencing file exported from are affected
552-
if (forEachEntry(state.currentAffectedFilesExportedModulesMap, (exportedModules, exportedFromPath) =>
550+
forEachEntry(state.currentAffectedFilesExportedModulesMap, (exportedModules, exportedFromPath) =>
553551
exportedModules &&
554552
exportedModules.has(affectedFile.resolvedPath) &&
555553
forEachFilesReferencingPath(state, exportedFromPath, seenFileAndExportsOfFile, fn)
556-
)) {
557-
return;
558-
}
554+
);
559555

560556
// If exported from path is not from cache and exported modules has path, all files referencing file exported from are affected
561557
forEachEntry(state.exportedModulesMap, (exportedModules, exportedFromPath) =>
@@ -568,47 +564,41 @@ namespace ts {
568564
/**
569565
* Iterate on files referencing referencedPath
570566
*/
571-
function forEachFilesReferencingPath(state: BuilderProgramState, referencedPath: Path, seenFileAndExportsOfFile: Set<string>, fn: (state: BuilderProgramState, filePath: Path) => boolean) {
572-
return forEachEntry(state.referencedMap!, (referencesInFile, filePath) =>
567+
function forEachFilesReferencingPath(state: BuilderProgramState, referencedPath: Path, seenFileAndExportsOfFile: Set<string>, fn: (state: BuilderProgramState, filePath: Path) => void) {
568+
forEachEntry(state.referencedMap!, (referencesInFile, filePath) =>
573569
referencesInFile.has(referencedPath) && forEachFileAndExportsOfFile(state, filePath, seenFileAndExportsOfFile, fn)
574570
);
575571
}
576572

577573
/**
578574
* fn on file and iterate on anything that exports this file
579575
*/
580-
function forEachFileAndExportsOfFile(state: BuilderProgramState, filePath: Path, seenFileAndExportsOfFile: Set<string>, fn: (state: BuilderProgramState, filePath: Path) => boolean): boolean {
576+
function forEachFileAndExportsOfFile(state: BuilderProgramState, filePath: Path, seenFileAndExportsOfFile: Set<string>, fn: (state: BuilderProgramState, filePath: Path) => void) {
581577
if (!tryAddToSet(seenFileAndExportsOfFile, filePath)) {
582-
return false;
578+
return;
583579
}
584580

585-
if (fn(state, filePath)) {
586-
// If there are no more diagnostics from old cache, done
587-
return true;
588-
}
581+
fn(state, filePath);
589582

590583
Debug.assert(!!state.currentAffectedFilesExportedModulesMap);
591584
// Go through exported modules from cache first
592585
// If exported modules has path, all files referencing file exported from are affected
593-
if (forEachEntry(state.currentAffectedFilesExportedModulesMap, (exportedModules, exportedFromPath) =>
586+
forEachEntry(state.currentAffectedFilesExportedModulesMap, (exportedModules, exportedFromPath) =>
594587
exportedModules &&
595588
exportedModules.has(filePath) &&
596589
forEachFileAndExportsOfFile(state, exportedFromPath, seenFileAndExportsOfFile, fn)
597-
)) {
598-
return true;
599-
}
590+
);
600591

601592
// If exported from path is not from cache and exported modules has path, all files referencing file exported from are affected
602-
if (forEachEntry(state.exportedModulesMap!, (exportedModules, exportedFromPath) =>
593+
forEachEntry(state.exportedModulesMap!, (exportedModules, exportedFromPath) =>
603594
!state.currentAffectedFilesExportedModulesMap!.has(exportedFromPath) && // If we already iterated this through cache, ignore it
604595
exportedModules.has(filePath) &&
605596
forEachFileAndExportsOfFile(state, exportedFromPath, seenFileAndExportsOfFile, fn)
606-
)) {
607-
return true;
608-
}
597+
);
609598

610599
// Remove diagnostics of files that import this file (without going to exports of referencing files)
611-
return !!forEachEntry(state.referencedMap!, (referencesInFile, referencingFilePath) =>
600+
601+
forEachEntry(state.referencedMap!, (referencesInFile, referencingFilePath) =>
612602
referencesInFile.has(filePath) &&
613603
!seenFileAndExportsOfFile.has(referencingFilePath) && // Not already removed diagnostic file
614604
fn(state, referencingFilePath) // Dont add to seen since this is not yet done with the export removal

0 commit comments

Comments
 (0)