Merge pull request #33380 from microsoft/isolatedModules

With --isolatedModules, --declaration emit is now allowed and builder handles it for incremental compilation

Author: sheetalkamat

Diff for: src/compiler/builder.ts

@@ -173,8 +173,7 @@ namespace ts {
         const compilerOptions = newProgram.getCompilerOptions();
         state.compilerOptions = compilerOptions;
         // With --out or --outFile, any change affects all semantic diagnostics so no need to cache them
-        // With --isolatedModules, emitting changed file doesnt emit dependent files so we cant know of dependent files to retrieve errors so dont cache the errors
-        if (!compilerOptions.outFile && !compilerOptions.out && !compilerOptions.isolatedModules) {
+        if (!compilerOptions.outFile && !compilerOptions.out) {
             state.semanticDiagnosticsPerFile = createMap<readonly Diagnostic[]>();
         }
         state.changedFilesSet = createMap<true>();
@@ -483,16 +482,43 @@ namespace ts {
         return !state.semanticDiagnosticsFromOldState.size;
     }
 
+    function isChangedSignagure(state: BuilderProgramState, path: Path) {
+        const newSignature = Debug.assertDefined(state.currentAffectedFilesSignatures).get(path);
+        const oldSignagure = Debug.assertDefined(state.fileInfos.get(path)).signature;
+        return newSignature !== oldSignagure;
+    }
+
     /**
      * 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)) {
+        if (!state.exportedModulesMap || !state.changedFilesSet.has(affectedFile.path)) {
             return;
         }
 
+        if (!isChangedSignagure(state, affectedFile.path)) return;
+
+        // Since isolated modules dont change js files, files affected by change in signature is itself
+        // But we need to cleanup semantic diagnostics and queue dts emit for affected files
+        if (state.compilerOptions.isolatedModules) {
+            const seenFileNamesMap = createMap<true>();
+            seenFileNamesMap.set(affectedFile.path, true);
+            const queue = BuilderState.getReferencedByPaths(state, affectedFile.resolvedPath);
+            while (queue.length > 0) {
+                const currentPath = queue.pop()!;
+                if (!seenFileNamesMap.has(currentPath)) {
+                    seenFileNamesMap.set(currentPath, true);
+                    const result = fn(state, currentPath);
+                    if (result && isChangedSignagure(state, currentPath)) {
+                        const currentSourceFile = Debug.assertDefined(state.program).getSourceFileByPath(currentPath)!;
+                        queue.push(...BuilderState.getReferencedByPaths(state, currentSourceFile.resolvedPath));
+                    }
+                }
+            }
+        }
+
         Debug.assert(!!state.currentAffectedFilesExportedModulesMap);
         const seenFileAndExportsOfFile = createMap<true>();
         // Go through exported modules from cache first

Diff for: src/compiler/builderState.ts

@@ -466,7 +466,7 @@ namespace ts.BuilderState {
     /**
      * Gets the files referenced by the the file path
      */
-    function getReferencedByPaths(state: Readonly<BuilderState>, referencedFilePath: Path) {
+    export function getReferencedByPaths(state: Readonly<BuilderState>, referencedFilePath: Path) {
         return arrayFrom(mapDefinedIterator(state.referencedMap!.entries(), ([filePath, referencesInFile]) =>
             referencesInFile.has(referencedFilePath) ? filePath as Path : undefined
         ));

Diff for: src/compiler/program.ts

@@ -2807,10 +2807,6 @@ namespace ts {
             }
 
             if (options.isolatedModules) {
-                if (getEmitDeclarations(options)) {
-                    createDiagnosticForOptionName(Diagnostics.Option_0_cannot_be_specified_with_option_1, getEmitDeclarationOptionName(options), "isolatedModules");
-                }
-
                 if (options.out) {
                     createDiagnosticForOptionName(Diagnostics.Option_0_cannot_be_specified_with_option_1, "out", "isolatedModules");
                 }

Diff for: src/testRunner/unittests/tsbuild/helpers.ts

@@ -230,7 +230,7 @@ interface Symbol {
         }
     }
 
-    interface BuildInput {
+    export interface BuildInput {
         fs: vfs.FileSystem;
         tick: () => void;
         rootNames: readonly string[];
@@ -239,7 +239,7 @@ interface Symbol {
         baselineBuildInfo?: true;
     }
 
-    function build({ fs, tick, rootNames, modifyFs, baselineSourceMap, baselineBuildInfo }: BuildInput) {
+    export function tscBuild({ fs, tick, rootNames, modifyFs, baselineSourceMap, baselineBuildInfo }: BuildInput) {
         const actualReadFileMap = createMap<number>();
         modifyFs(fs);
         tick();
@@ -344,7 +344,7 @@ Mismatch Actual(path, actual, expected): ${JSON.stringify(arrayFrom(mapDefinedIt
             let host: fakes.SolutionBuilderHost;
             let initialWrittenFiles: Map<true>;
             before(() => {
-                const result = build({
+                const result = tscBuild({
                     fs: projFs().shadow(),
                     tick,
                     rootNames,
@@ -390,7 +390,7 @@ Mismatch Actual(path, actual, expected): ${JSON.stringify(arrayFrom(mapDefinedIt
                         tick();
                         newFs = fs.shadow();
                         tick();
-                        ({ actualReadFileMap, host } = build({
+                        ({ actualReadFileMap, host } = tscBuild({
                             fs: newFs,
                             tick,
                             rootNames,
@@ -429,7 +429,7 @@ Mismatch Actual(path, actual, expected): ${JSON.stringify(arrayFrom(mapDefinedIt
                         });
                     }
                     it(`Verify emit output file text is same when built clean`, () => {
-                        const { fs, writtenFiles } = build({
+                        const { fs, writtenFiles } = tscBuild({
                             fs: newFs.shadow(),
                             tick,
                             rootNames,

Diff for: src/testRunner/unittests/tsbuild/inferredTypeFromTransitiveModule.ts

@@ -25,7 +25,7 @@ namespace ts {
                 ]
             },
             incrementalDtsChangedBuild: {
-                modifyFs: fs => replaceText(fs, "/src/bar.ts", "param: string", ""),
+                modifyFs: changeBarParam,
                 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"],
@@ -36,5 +36,55 @@ namespace ts {
             baselineOnly: true,
             verifyDiagnostics: true
         });
+
+        verifyTsbuildOutput({
+            scenario: "inferred type from transitive module with isolatedModules",
+            projFs: () => projFs,
+            time,
+            tick,
+            proj: "inferredTypeFromTransitiveModule",
+            rootNames: ["/src"],
+            initialBuild: { modifyFs: changeToIsolatedModules },
+            incrementalDtsChangedBuild: { modifyFs: changeBarParam },
+            baselineOnly: true,
+        });
+
+        it("reports errors in files affected by change in signature", () => {
+            const { fs, host } = tscBuild({
+                fs: projFs.shadow(),
+                tick,
+                rootNames: ["/src"],
+                modifyFs: fs => {
+                    changeToIsolatedModules(fs);
+                    appendText(fs, "/src/lazyIndex.ts", `
+import { default as bar } from './bar';
+bar("hello");`);
+                }
+            });
+            host.assertErrors(/*empty*/);
+
+            tick();
+            const { fs: newFs, host: newHost, writtenFiles } = tscBuild({
+                fs: fs.shadow(),
+                tick,
+                rootNames: ["/src"],
+                modifyFs: changeBarParam
+            });
+            // Has errors
+            newHost.assertErrors({
+                message: [Diagnostics.Expected_0_arguments_but_got_1, 0, 1],
+                location: expectedLocationIndexOf(newFs, "/src/lazyIndex.ts", `"hello"`)
+            });
+            // No written files
+            assert.equal(writtenFiles.size, 0);
+        });
     });
+
+    function changeToIsolatedModules(fs: vfs.FileSystem) {
+        replaceText(fs, "/src/tsconfig.json", `"incremental": true`, `"incremental": true, "isolatedModules": true`);
+    }
+
+    function changeBarParam(fs: vfs.FileSystem) {
+        replaceText(fs, "/src/bar.ts", "param: string", "");
+    }
 }