fix(@ngtools/webpack): fix rebuilds for transitive and global type deps

Fix #15856

Author: filipesilva

packages/angular_devkit/build_angular/test/browser/rebuild_spec_large.ts

@@ -222,6 +222,52 @@ describe('Browser Builder rebuilds', () => {
       .toPromise();
   });
 
+  it('rebuilds on transitive type-only file changes', async () => {
+    if (veEnabled) {
+      // TODO: https://github.com/angular/angular-cli/issues/15056
+      pending('Only supported in Ivy.');
+
+      return;
+    }
+    host.writeMultipleFiles({
+      'src/interface1.ts': `
+        import { Interface2 } from './interface2';
+        export interface Interface1 extends Interface2 { }
+      `,
+      'src/interface2.ts': `
+        import { Interface3 } from './interface3';
+        export interface Interface2 extends Interface3 { }
+      `,
+      'src/interface3.ts': `export interface Interface3 { nbr: number; }`,
+    });
+    host.appendToFile('src/main.ts', `
+      import { Interface1 } from './interface1';
+      const something: Interface1 = { nbr: 43 };
+    `);
+
+    const overrides = { watch: true };
+    const run = await architect.scheduleTarget(target, overrides);
+    let buildNumber = 0;
+    await run.output
+      .pipe(
+        debounceTime(rebuildDebounceTime),
+        tap(buildEvent => expect(buildEvent.success).toBe(true)),
+        tap(() => {
+          // NOTE: this only works for transitive type deps after the first build, and only if the
+          // typedep file was there on the previous build.
+          // Make sure the first rebuild is triggered on a direct dep (typedep or not).
+          buildNumber++;
+          if (buildNumber < 4) {
+            host.appendToFile(`src/interface${buildNumber}.ts`, `export type MyType = string;`);
+          } else {
+            host.appendToFile(`src/typings.d.ts`, `export type MyType = string;`);
+          }
+        }),
+        take(5),
+      )
+      .toPromise();
+  });
+
   it('rebuilds after errors in JIT', async () => {
     const origContent = virtualFs.fileBufferToString(
       host.scopedSync().read(normalize('src/app/app.component.ts')),

packages/ngtools/webpack/src/angular_compiler_plugin.ts

@@ -110,7 +110,9 @@ export class AngularCompilerPlugin {
   // even whe only a single file gets updated.
   private _hadFullJitEmit: boolean | undefined;
   private _unusedFiles = new Set<string>();
+  private _typeDeps = new Set<string>();
   private _changedFileExtensions = new Set(['ts', 'tsx', 'html', 'css', 'js', 'json']);
+  private _nodeModulesRegExp = /[\\\/]node_modules[\\\/]/;
 
   // Webpack plugin.
   private _firstRun = true;
@@ -592,7 +594,7 @@ export class AngularCompilerPlugin {
     }
   }
 
-  private _warnOnUnusedFiles(compilation: compilation.Compilation) {
+  private _checkUnusedFiles(compilation: compilation.Compilation) {
     // Only do the unused TS files checks when under Ivy
     // since previously we did include unused files in the compilation
     // See: https://github.com/angular/angular-cli/pull/15030
@@ -601,6 +603,7 @@ export class AngularCompilerPlugin {
       return;
     }
 
+    // Bail if there's no TS program. Nothing to do in that case.
     const program = this._getTsProgram();
     if (!program) {
       return;
@@ -609,26 +612,36 @@ export class AngularCompilerPlugin {
     // Exclude the following files from unused checks
     // - ngfactories & ngstyle might not have a correspondent
     //   JS file example `@angular/core/core.ngfactory.ts`.
-    // - .d.ts files might not have a correspondent JS file due to bundling.
     // - __ng_typecheck__.ts will never be requested.
-    const fileExcludeRegExp = /(\.(d|ngfactory|ngstyle|ngsummary)\.ts|ng_typecheck__\.ts)$/;
+    const fileExcludeRegExp = /(\.(ngfactory|ngstyle|ngsummary)\.ts|ng_typecheck__\.ts)$/;
+
+    // Start all the source file names we care about.
+    // Ignore matches to the regexp above, files we've already reported once before, and
+    // node_modules.
+    const sourceFiles = program.getSourceFiles()
+      .map(x => this._compilerHost.denormalizePath(x.fileName))
+      .filter(f => !(fileExcludeRegExp.test(f) || this._unusedFiles.has(f)
+        || this._nodeModulesRegExp.test(f)));
+
+    // Make a set with the sources, but exclude .d.ts files since those are type-only.
+    const unusedSourceFileNames = new Set(sourceFiles.filter(f => !f.endsWith('.d.ts')));
+    // Separately keep track of type-only deps.
+    const typeDepFileNames = new Set(sourceFiles);
 
-    // Start with a set of all the source file names we care about.
-    const unusedSourceFileNames = new Set(
-      program.getSourceFiles()
-        .map(x => this._compilerHost.denormalizePath(x.fileName))
-        .filter(f => !(fileExcludeRegExp.test(f) || this._unusedFiles.has(f))),
-    );
     // This function removes a source file name and all its dependencies from the set.
-    const removeSourceFile = (fileName: string) => {
-      if (unusedSourceFileNames.has(fileName)) {
+    const removeSourceFile = (fileName: string, originalModule = false) => {
+      if (unusedSourceFileNames.has(fileName)
+        || (originalModule && typeDepFileNames.has(fileName))) {
         unusedSourceFileNames.delete(fileName);
+        if (originalModule) {
+          typeDepFileNames.delete(fileName);
+        }
         this.getDependencies(fileName, false).forEach(f => removeSourceFile(f));
       }
     };
 
-    // Go over all the modules in the webpack compilation and remove them from the set.
-    compilation.modules.forEach(m => m.resource ? removeSourceFile(m.resource) : null);
+    // Go over all the modules in the webpack compilation and remove them from the sets.
+    compilation.modules.forEach(m => m.resource ? removeSourceFile(m.resource, true) : null);
 
     // Anything that remains is unused, because it wasn't referenced directly or transitively
     // on the files in the compilation.
@@ -638,7 +651,15 @@ export class AngularCompilerPlugin {
         `Add only entry points to the 'files' or 'include' properties in your tsconfig.`,
         );
       this._unusedFiles.add(fileName);
+      // Remove the truly unused from the type dep list.
+      typeDepFileNames.delete(fileName);
     }
+
+    // At this point we know what the type deps are.
+    // These are the TS files that weren't part of the compilation modules, aren't unused, but were
+    // part of the TS original source list.
+    // Next build we add them to the TS entry points so that they trigger rebuilds.
+    this._typeDeps = typeDepFileNames;
   }
 
   // Registration hook for webpack plugin.
@@ -657,7 +678,7 @@ export class AngularCompilerPlugin {
     // cleanup if not watching
     compiler.hooks.thisCompilation.tap('angular-compiler', compilation => {
       compilation.hooks.finishModules.tap('angular-compiler', () => {
-        this._warnOnUnusedFiles(compilation);
+        this._checkUnusedFiles(compilation);
 
         let rootCompiler = compiler;
         while (rootCompiler.parentCompilation) {
@@ -1188,7 +1209,7 @@ export class AngularCompilerPlugin {
         let msg = `${fileName} is missing from the TypeScript compilation. `
           + `Please make sure it is in your tsconfig via the 'files' or 'include' property.`;
 
-        if (/(\\|\/)node_modules(\\|\/)/.test(fileName)) {
+        if (this._nodeModulesRegExp.test(fileName)) {
           msg += '\nThe missing file seems to be part of a third party library. '
             + 'TS files in published libraries are often a sign of a badly packaged library. '
             + 'Please open an issue in the library repository to alert its author and ask them '
@@ -1267,6 +1288,18 @@ export class AngularCompilerPlugin {
     return this._resourceLoader.getResourceDependencies(resolvedFileName);
   }
 
+  getTypeDependencies(fileName: string): string[] {
+    // We currently add all type deps directly to the main path.
+    // If there's no main path or the lookup isn't the main path, bail.
+    if (!this._mainPath || this._compilerHost.resolve(fileName) != this._mainPath) {
+      return [];
+    }
+
+    // Note: this set is always for the previous build, not the current build.
+    // It should be better than not having rebuilds on type deps but isn't 100% correct.
+    return Array.from(this._typeDeps);
+  }
+
   // This code mostly comes from `performCompilation` in `@angular/compiler-cli`.
   // It skips the program creation because we need to use `loadNgStructureAsync()`,
   // and uses CustomTransformers.

packages/ngtools/webpack/src/loader.ts

@@ -100,6 +100,10 @@ export function ngcLoader(this: loader.LoaderContext) {
         styleDependencies.forEach(dep => this.addDependency(dep));
       }
 
+      // Add type-only dependencies that should trigger a rebuild when they change.
+      const typeDependencies = plugin.getTypeDependencies(sourceFileName);
+      typeDependencies.forEach(dep => this.addDependency(dep));
+
       timeEnd(timeLabel);
       cb(null, result.outputText, result.sourceMap as any);
     })