From 865aece496523de7d77ea30247ea8a7586a8b0ec Mon Sep 17 00:00:00 2001 From: Alan Agius Date: Thu, 18 Jul 2019 14:51:25 +0200 Subject: [PATCH] feat(@ngtools/webpack): show warning when a TS compilations contains unused files When a tsconfig without `includes` or `files` all files under the `rootDir` will be included in the compilation. This results in redundant files to inserted as part of the ts compilation which in some cases might reduce the compilation drastically. Related to: TOOL-949 --- .../unused-files-warning_spec_large.ts | 165 ++++++++++++++++++ .../webpack/src/angular_compiler_plugin.ts | 51 +++++- 2 files changed, 213 insertions(+), 3 deletions(-) create mode 100644 packages/angular_devkit/build_angular/test/browser/unused-files-warning_spec_large.ts 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 new file mode 100644 index 000000000000..75b16fbb998d --- /dev/null +++ b/packages/angular_devkit/build_angular/test/browser/unused-files-warning_spec_large.ts @@ -0,0 +1,165 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ +import { Architect } from '@angular-devkit/architect'; +import { TestLogger } from '@angular-devkit/architect/testing'; +import { debounceTime, take, tap } from 'rxjs/operators'; +import { BrowserBuilderOutput } from '../../src/browser/index'; +import { createArchitect, host, ivyEnabled } from '../utils'; + + +describe('Browser Builder unused files warnings', () => { + const warningMessageSuffix = `is part of the TypeScript compilation but it's unused`; + const targetSpec = { project: 'app', target: 'build' }; + let architect: Architect; + + beforeEach(async () => { + await host.initialize().toPromise(); + + architect = (await createArchitect(host.root())).architect; + }); + afterEach(async () => host.restore().toPromise()); + + it('should not show warning when all files are used', async () => { + if (!ivyEnabled) { + // TODO: https://github.com/angular/angular-cli/issues/15056 + pending('Only supported in Ivy.'); + + return; + } + + 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('should show warning when some files are unused', async () => { + if (!ivyEnabled) { + // TODO: https://github.com/angular/angular-cli/issues/15056 + pending('Only supported in Ivy.'); + + return; + } + + host.replaceInFile( + 'src/tsconfig.app.json', + '"main.ts"', + '"main.ts", "environments/environment.prod.ts"', + ); + + 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(`environment.prod.ts ${warningMessageSuffix}`)).toBe(true); + logger.clear(); + + await run.stop(); + }); + + it('should not show warning when type files are used', async () => { + if (!ivyEnabled) { + // TODO: https://github.com/angular/angular-cli/issues/15056 + pending('Only supported in Ivy.'); + + return; + } + + host.writeMultipleFiles({ + 'src/app/type.ts': 'export type MyType = 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 (!ivyEnabled) { + // TODO: https://github.com/angular/angular-cli/issues/15056 + pending('Only supported in Ivy.'); + + return; + } + + host.replaceInFile( + 'src/tsconfig.app.json', + '"**/*.d.ts"', + '"**/*.d.ts", "testing/**/*.ts"', + ); + + const logger = new TestLogger('unused-files-warnings'); + let buildNumber = 0; + const run = await architect.scheduleTarget(targetSpec, { watch: true }, { logger }); + + await run.output + .pipe( + debounceTime(1000), + tap(buildEvent => { + expect(buildEvent.success).toBe(true); + + buildNumber ++; + switch (buildNumber) { + case 1: + // The first should not have unused files + expect(logger.includes(warningMessageSuffix)).toBe(false, `Case ${buildNumber} failed.`); + + // Write a used file + host.writeMultipleFiles({ + 'src/testing/type.ts': 'export type MyType = number;' + }); + + // touch file to trigger build + host.replaceInFile( + 'src/app/app.component.ts', + `'@angular/core';`, + `'@angular/core';\n` + ); + break; + + case 2: + // The second should have type.ts as unused + expect(logger.includes(`type.ts ${warningMessageSuffix}`)).toBe(true, `Case ${buildNumber} failed.`); + + host.replaceInFile( + 'src/app/app.component.ts', + `'@angular/core';`, + `'@angular/core';\nimport { MyType } from '../testing/type';` + ); + break; + + case 3: + // The third should not have any unused files + expect(logger.includes(warningMessageSuffix)).toBe(false, `Case ${buildNumber} failed.`); + break; + } + + logger.clear(); + }), + take(3), + ) + .toPromise(); + await run.stop(); + }); + +}); diff --git a/packages/ngtools/webpack/src/angular_compiler_plugin.ts b/packages/ngtools/webpack/src/angular_compiler_plugin.ts index 872b47f37ca7..455c6bd434c7 100644 --- a/packages/ngtools/webpack/src/angular_compiler_plugin.ts +++ b/packages/ngtools/webpack/src/angular_compiler_plugin.ts @@ -593,6 +593,50 @@ export class AngularCompilerPlugin { } } + private _warnOnUnusedFiles(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 + if (!this._compilerOptions.enableIvy) { + return; + } + + const program = this._getTsProgram(); + if (!program) { + return; + } + + // 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)\.ts|ng_typecheck__\.ts)$/; + + const usedFiles = new Set(); + for (const compilationModule of compilation.modules) { + if (!compilationModule.resource) { + continue; + } + + 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 unusedFilesWarning = program.getSourceFiles() + .filter(({ fileName }) => !fileExcludeRegExp.test(fileName) && !usedFiles.has(fileName)) + .map(({ fileName }) => `${fileName} is part of the TypeScript compilation but it's unused.`); + + if (unusedFilesWarning.length) { + compilation.warnings.push(...unusedFilesWarning); + } + } + // Registration hook for webpack plugin. // tslint:disable-next-line:no-big-function apply(compiler: Compiler & { watchMode?: boolean, parentCompilation?: compilation.Compilation }) { @@ -609,6 +653,8 @@ export class AngularCompilerPlugin { // cleanup if not watching compiler.hooks.thisCompilation.tap('angular-compiler', compilation => { compilation.hooks.finishModules.tap('angular-compiler', () => { + this._warnOnUnusedFiles(compilation); + let rootCompiler = compiler; while (rootCompiler.parentCompilation) { // tslint:disable-next-line:no-any @@ -1174,7 +1220,7 @@ export class AngularCompilerPlugin { return null; } }) - .filter(x => x); + .filter(x => x) as string[]; const resourceImports = findResources(sourceFile) .map(resourcePath => resolve(dirname(resolvedFileName), normalize(resourcePath))); @@ -1186,8 +1232,7 @@ export class AngularCompilerPlugin { ...this.getResourceDependencies(this._compilerHost.denormalizePath(resolvedFileName)), ].map((p) => p && this._compilerHost.denormalizePath(p))); - return [...uniqueDependencies] - .filter(x => !!x) as string[]; + return [...uniqueDependencies]; } getResourceDependencies(fileName: string): string[] {