Instead of computing signatures during building, compute them afterwards for verification (#51718)

This helps with finding issues with d.ts emit because of caching.

Author: sheetalkamat

src/compiler/builder.ts

@@ -286,8 +286,8 @@ function hasSameKeys(map1: ReadonlyCollection<string> | undefined, map2: Readonl
 /**
  * Create the state so that we can iterate on changedFiles/affected files
  */
-function createBuilderProgramState(newProgram: Program, oldState: Readonly<ReusableBuilderProgramState> | undefined, disableUseFileVersionAsSignature: boolean | undefined): BuilderProgramState {
-    const state = BuilderState.create(newProgram, oldState, disableUseFileVersionAsSignature) as BuilderProgramState;
+function createBuilderProgramState(newProgram: Program, oldState: Readonly<ReusableBuilderProgramState> | undefined): BuilderProgramState {
+    const state = BuilderState.create(newProgram, oldState, /*disableUseFileVersionAsSignature*/ false) as BuilderProgramState;
     state.program = newProgram;
     const compilerOptions = newProgram.getCompilerOptions();
     state.compilerOptions = compilerOptions;
@@ -699,7 +699,7 @@ function handleDtsMayChangeOf(
                 sourceFile,
                 cancellationToken,
                 host,
-                !host.disableUseFileVersionAsSignature
+                /*useFileVersionAsSignature*/ true
             );
             // If not dts emit, nothing more to do
             if (getEmitDeclarations(state.compilerOptions)) {
@@ -1312,7 +1312,7 @@ export function createBuilderProgram(kind: BuilderProgramKind, { newProgram, hos
         return oldProgram;
     }
 
-    const state = createBuilderProgramState(newProgram, oldState, host.disableUseFileVersionAsSignature);
+    const state = createBuilderProgramState(newProgram, oldState);
     newProgram.getBuildInfo = bundle => getBuildInfo(state, bundle);
 
     // To ensure that we arent storing any references to old program or new program without state

src/compiler/builderPublic.ts

@@ -30,12 +30,6 @@ export interface BuilderProgramHost {
      * this callback if present would be used to write files
      */
     writeFile?: WriteFileCallback;
-    /**
-     * disable using source file version as signature for testing
-     *
-     * @internal
-     */
-    disableUseFileVersionAsSignature?: boolean;
     /**
      * Store the list of files that update signature during the emit
      *

src/compiler/builderState.ts

@@ -295,7 +295,7 @@ export namespace BuilderState {
     /**
      * Creates the state of file references and signature for the new program from oldState if it is safe
      */
-    export function create(newProgram: Program, oldState?: Readonly<BuilderState>, disableUseFileVersionAsSignature?: boolean): BuilderState {
+    export function create(newProgram: Program, oldState: Readonly<BuilderState> | undefined, disableUseFileVersionAsSignature: boolean): BuilderState {
         const fileInfos = new Map<Path, FileInfo>();
         const options = newProgram.getCompilerOptions();
         const isOutFile = outFile(options);
@@ -402,6 +402,32 @@ export namespace BuilderState {
         (state.hasCalledUpdateShapeSignature ||= new Set()).add(path);
     }
 
+    export function computeDtsSignature(
+        programOfThisState: Program,
+        sourceFile: SourceFile,
+        cancellationToken: CancellationToken | undefined,
+        host: HostForComputeHash,
+        onNewSignature: (signature: string, sourceFiles: readonly SourceFile[]) => void,
+    ) {
+        programOfThisState.emit(
+            sourceFile,
+            (fileName, text, _writeByteOrderMark, _onError, sourceFiles, data) => {
+                Debug.assert(isDeclarationFileName(fileName), `File extension for signature expected to be dts: Got:: ${fileName}`);
+                onNewSignature(computeSignatureWithDiagnostics(
+                    programOfThisState,
+                    sourceFile,
+                    text,
+                    host,
+                    data,
+                ), sourceFiles!);
+            },
+            cancellationToken,
+            /*emitOnlyDtsFiles*/ true,
+            /*customTransformers*/ undefined,
+            /*forceDtsEmit*/ true
+        );
+    }
+
     /**
      * Returns if the shape of the signature has changed since last emit
      */
@@ -420,26 +446,12 @@ export namespace BuilderState {
         const prevSignature = info.signature;
         let latestSignature: string | undefined;
         if (!sourceFile.isDeclarationFile && !useFileVersionAsSignature) {
-            programOfThisState.emit(
-                sourceFile,
-                (fileName, text, _writeByteOrderMark, _onError, sourceFiles, data) => {
-                    Debug.assert(isDeclarationFileName(fileName), `File extension for signature expected to be dts: Got:: ${fileName}`);
-                    latestSignature = computeSignatureWithDiagnostics(
-                        programOfThisState,
-                        sourceFile,
-                        text,
-                        host,
-                        data,
-                    );
-                    if (latestSignature !== prevSignature) {
-                        updateExportedModules(state, sourceFile, sourceFiles![0].exportedModulesFromDeclarationEmit);
-                    }
-                },
-                cancellationToken,
-                /*emitOnlyDtsFiles*/ true,
-                /*customTransformers*/ undefined,
-                /*forceDtsEmit*/ true
-            );
+            computeDtsSignature(programOfThisState, sourceFile, cancellationToken, host, (signature, sourceFiles) => {
+                latestSignature = signature;
+                if (latestSignature !== prevSignature) {
+                    updateExportedModules(state, sourceFile, sourceFiles[0].exportedModulesFromDeclarationEmit);
+                }
+            });
         }
         // Default is to use file version as signature
         if (latestSignature === undefined) {
@@ -468,28 +480,23 @@ export namespace BuilderState {
     export function updateExportedModules(state: BuilderState, sourceFile: SourceFile, exportedModulesFromDeclarationEmit: ExportedModulesFromDeclarationEmit | undefined) {
         if (!state.exportedModulesMap) return;
         (state.oldExportedModulesMap ||= new Map()).set(sourceFile.resolvedPath, state.exportedModulesMap.getValues(sourceFile.resolvedPath) || false);
-        if (!exportedModulesFromDeclarationEmit) {
-            state.exportedModulesMap.deleteKey(sourceFile.resolvedPath);
-            return;
-        }
-
-        let exportedModules: Set<Path> | undefined;
-        exportedModulesFromDeclarationEmit.forEach(symbol => addExportedModule(getReferencedFilesFromImportedModuleSymbol(symbol)));
+        const exportedModules = getExportedModules(exportedModulesFromDeclarationEmit);
         if (exportedModules) {
             state.exportedModulesMap.set(sourceFile.resolvedPath, exportedModules);
         }
         else {
             state.exportedModulesMap.deleteKey(sourceFile.resolvedPath);
         }
+    }
 
-        function addExportedModule(exportedModulePaths: Path[] | undefined) {
-            if (exportedModulePaths?.length) {
-                if (!exportedModules) {
-                    exportedModules = new Set();
-                }
-                exportedModulePaths.forEach(path => exportedModules!.add(path));
-            }
-        }
+    export function getExportedModules(exportedModulesFromDeclarationEmit: ExportedModulesFromDeclarationEmit | undefined) {
+        let exportedModules: Set<Path> | undefined;
+        exportedModulesFromDeclarationEmit?.forEach(
+            symbol => getReferencedFilesFromImportedModuleSymbol(symbol).forEach(
+                path => (exportedModules ??= new Set()).add(path)
+            )
+        );
+        return exportedModules;
     }
 
     /**

src/compiler/sys.ts

@@ -1433,7 +1433,6 @@ export interface System {
 
     // For testing
     /** @internal */ now?(): Date;
-    /** @internal */ disableUseFileVersionAsSignature?: boolean;
     /** @internal */ storeFilesChangingSignatureDuringEmit?: boolean;
 }
 

src/compiler/types.ts

@@ -7380,7 +7380,6 @@ export interface CompilerHost extends ModuleResolutionHost {
     /** @internal */getSymlinkCache?(): SymlinkCache;
 
     // For testing:
-    /** @internal */ disableUseFileVersionAsSignature?: boolean;
     /** @internal */ storeFilesChangingSignatureDuringEmit?: boolean;
     /** @internal */ getBuildInfo?(fileName: string, configFilePath: string | undefined): BuildInfo | undefined;
 }

src/compiler/watch.ts

@@ -765,7 +765,6 @@ export function createCompilerHostFromProgramHost(host: ProgramHost<any>, getCom
         getEnvironmentVariable: maybeBind(host, host.getEnvironmentVariable) || (() => ""),
         createHash: maybeBind(host, host.createHash),
         readDirectory: maybeBind(host, host.readDirectory),
-        disableUseFileVersionAsSignature: host.disableUseFileVersionAsSignature,
         storeFilesChangingSignatureDuringEmit: host.storeFilesChangingSignatureDuringEmit,
     };
     return compilerHost;
@@ -847,7 +846,6 @@ export function createProgramHost<T extends BuilderProgram = EmitAndSemanticDiag
         writeFile: (path, data, writeByteOrderMark) => system.writeFile(path, data, writeByteOrderMark),
         createHash: maybeBind(system, system.createHash),
         createProgram: createProgram || createEmitAndSemanticDiagnosticsBuilderProgram as any as CreateProgram<T>,
-        disableUseFileVersionAsSignature: system.disableUseFileVersionAsSignature,
         storeFilesChangingSignatureDuringEmit: system.storeFilesChangingSignatureDuringEmit,
         now: maybeBind(system, system.now),
     };

src/compiler/watchPublic.ts

@@ -121,7 +121,6 @@ export function readBuilderProgram(compilerOptions: CompilerOptions, host: ReadB
 export function createIncrementalCompilerHost(options: CompilerOptions, system = sys): CompilerHost {
     const host = createCompilerHostWorker(options, /*setParentNodes*/ undefined, system);
     host.createHash = maybeBind(system, system.createHash);
-    host.disableUseFileVersionAsSignature = system.disableUseFileVersionAsSignature;
     host.storeFilesChangingSignatureDuringEmit = system.storeFilesChangingSignatureDuringEmit;
     setGetSourceFileAsHashVersioned(host);
     changeCompilerHostLikeToUseCache(host, fileName => toPath(fileName, host.getCurrentDirectory(), host.getCanonicalFileName));
@@ -248,7 +247,6 @@ export interface ProgramHost<T extends BuilderProgram> {
     createDirectory?(path: string): void;
     writeFile?(path: string, data: string, writeByteOrderMark?: boolean): void;
     // For testing
-    disableUseFileVersionAsSignature?: boolean;
     storeFilesChangingSignatureDuringEmit?: boolean;
     now?(): Date;
 }

src/testRunner/unittests/tsbuild/commandLine.ts

@@ -4,7 +4,6 @@ import {
     compilerOptionsToConfigJson,
     loadProjectFromFiles,
     noChangeRun,
-    noChangeWithExportsDiscrepancyRun,
     replaceText,
     TestTscEdit,
     verifyTscWithEdits,
@@ -41,12 +40,6 @@ describe("unittests:: tsbuild:: commandLine::", () => {
             ];
             return edit;
         }
-        function withOptionChangeAndExportExplanation(subScenario: string, ...options: readonly string[]): TestTscEdit {
-            return {
-                ...withOptionChange(subScenario, ...options),
-                discrepancyExplanation: noChangeWithExportsDiscrepancyRun.discrepancyExplanation,
-            };
-        }
         function nochangeWithIncrementalDeclarationFromBeforeExplaination(): TestTscEdit {
             return {
                 ...noChangeRun,
@@ -129,8 +122,8 @@ describe("unittests:: tsbuild:: commandLine::", () => {
             fs: () => fs({ incremental: true }),
             commandLineArgs: ["--b", "/src/project", "--verbose"],
             edits: [
-                withOptionChangeAndExportExplanation("with sourceMap", "--sourceMap"),
-                withOptionChangeAndExportExplanation("should re-emit only js so they dont contain sourcemap"),
+                withOptionChange("with sourceMap", "--sourceMap"),
+                withOptionChange("should re-emit only js so they dont contain sourcemap"),
                 withOptionChange("with declaration, emit Dts and should not emit js", "--declaration"),
                 withOptionChange("with declaration and declarationMap", "--declaration", "--declarationMap"),
                 nochangeWithIncrementalDeclarationFromBeforeExplaination(),

src/testRunner/unittests/tsbuild/noEmitOnError.ts

@@ -1,7 +1,6 @@
 import {
     loadProjectFromDisk,
     noChangeRun,
-    noChangeWithExportsDiscrepancyRun,
     verifyTscWithEdits,
 } from "../tsc/helpers";
 import * as vfs from "../../_namespaces/vfs";
@@ -80,7 +79,7 @@ const a: string = "hello";`, "utf-8"),
 const a: string = 10;`, "utf-8"),
         commandLineArgs: ["--b", "/src/tsconfig.json", "--incremental"],
         edits: [
-            noChangeWithExportsDiscrepancyRun,
+            noChangeRun,
             {
                 subScenario: "Fix error",
                 modifyFs: fs => fs.writeFileSync("/src/src/main.ts", `import { A } from "../shared/types/db";