Merge pull request microsoft#30971 from Microsoft/dtsSignatureChange

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

Author: sheetalkamat

src/compiler/builder.ts

@@ -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>();
         }
@@ -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);
@@ -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;
         }
@@ -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;
         }
@@ -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;
         }
@@ -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;
         }
@@ -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;
         }
@@ -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.
@@ -808,11 +844,6 @@ namespace ts {
                 }
             }
 
-            // Mark seen emitted files if there are pending files to be emitted
-            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)
@@ -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;
@@ -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>();

src/compiler/builderState.ts

@@ -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");
 

src/testRunner/tsconfig.json

@@ -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",

src/testRunner/unittests/tsbuild/inferredTypeFromTransitiveModule.ts

@@ -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
+        });
+    });
+}