fix(@ngtools/webpack): recursive look up unused files

Fix #15626
angular · Nov 12, 2019 · 3e1d1ce · 3e1d1ce
1 parent 31fccb2
commit 3e1d1ce
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 26 deletions.
diff --git a/packages/angular_devkit/build_angular/test/browser/unused-files-warning_spec_large.ts b/packages/angular_devkit/build_angular/test/browser/unused-files-warning_spec_large.ts
@@ -94,6 +94,36 @@ describe('Browser Builder unused files warnings', () => {
     await run.stop();
   });
 
+  it('should not show warning when type files are used transitively', async () => {
+    if (veEnabled) {
+      // TODO: https://github.com/angular/angular-cli/issues/15056
+      pending('Only supported in Ivy.');
+
+      return;
+    }
+
+    host.writeMultipleFiles({
+      'src/app/type.ts':
+        `import {Myinterface} from './interface'; export type MyType = Myinterface;`,
+      'src/app/interface.ts': 'export interface Myinterface {nbr: number;}',
+    });
+
+    host.replaceInFile(
+      'src/app/app.component.ts',
+      `'@angular/core';`,
+      `'@angular/core';\nimport { MyType } from './type';\n`,
+    );
+
+    const logger = new TestLogger('unused-files-warnings');
+    const run = await architect.scheduleTarget(targetSpec, undefined, { logger });
+    const output = await run.result as BrowserBuilderOutput;
+    expect(output.success).toBe(true);
+    expect(logger.includes(warningMessageSuffix)).toBe(false);
+    logger.clear();
+
+    await run.stop();
+  });
+
   it('works for rebuilds', async () => {
     if (veEnabled) {
       // TODO: https://github.com/angular/angular-cli/issues/15056

diff --git a/packages/ngtools/webpack/src/angular_compiler_plugin.ts b/packages/ngtools/webpack/src/angular_compiler_plugin.ts
@@ -613,31 +613,26 @@ export class AngularCompilerPlugin {
     // - __ng_typecheck__.ts will never be requested.
     const fileExcludeRegExp = /(\.(d|ngfactory|ngstyle)\.ts|ng_typecheck__\.ts)$/;
 
-    const usedFiles = new Set<string>();
-    for (const compilationModule of compilation.modules) {
-      if (!compilationModule.resource) {
-        continue;
+    // 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)) {
+        unusedSourceFileNames.delete(fileName);
+        this.getDependencies(fileName, false).forEach(f => removeSourceFile(f));
       }
+    };
 
-      usedFiles.add(forwardSlashPath(compilationModule.resource));
-
-      // We need the below for dependencies which
-      // are not emitted such as type only TS files
-      for (const dependency of compilationModule.buildInfo.fileDependencies) {
-        usedFiles.add(forwardSlashPath(dependency));
-      }
-    }
-
-    const sourceFiles = program.getSourceFiles();
-    for (const { fileName } of sourceFiles) {
-      if (
-        fileExcludeRegExp.test(fileName)
-        || usedFiles.has(fileName)
-        || this._unusedFiles.has(fileName)
-      ) {
-        continue;
-      }
+    // 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);
 
+    // Anything that remains is unused, because it wasn't referenced directly or transitively
+    // on the files in the compilation.
+    for (const fileName of unusedSourceFileNames) {
       compilation.warnings.push(
         `${fileName} is part of the TypeScript compilation but it's unused.\n` +
         `Add only entry points to the 'files' or 'include' properties in your tsconfig.`,
@@ -1210,7 +1205,7 @@ export class AngularCompilerPlugin {
     return { outputText, sourceMap, errorDependencies };
   }
 
-  getDependencies(fileName: string): string[] {
+  getDependencies(fileName: string, includeResources = true): string[] {
     const resolvedFileName = this._compilerHost.resolve(fileName);
     const sourceFile = this._compilerHost.getSourceFile(resolvedFileName, ts.ScriptTarget.Latest);
     if (!sourceFile) {
@@ -1244,14 +1239,19 @@ export class AngularCompilerPlugin {
       })
       .filter(x => x) as string[];
 
-    const resourceImports = findResources(sourceFile)
-      .map(resourcePath => resolve(dirname(resolvedFileName), normalize(resourcePath)));
+    let resourceImports: string[] = [], resourceDependencies: string[] = [];
+    if (includeResources) {
+      resourceImports = findResources(sourceFile)
+        .map(resourcePath => resolve(dirname(resolvedFileName), normalize(resourcePath)));
+      resourceDependencies =
+        this.getResourceDependencies(this._compilerHost.denormalizePath(resolvedFileName));
+    }
 
     // These paths are meant to be used by the loader so we must denormalize them.
     const uniqueDependencies = new Set([
       ...esImports,
       ...resourceImports,
-      ...this.getResourceDependencies(this._compilerHost.denormalizePath(resolvedFileName)),
+      ...resourceDependencies,
     ].map((p) => p && this._compilerHost.denormalizePath(p)));
 
     return [...uniqueDependencies];