From ad6f250a971cc66a8df685b489104c9514ed4600 Mon Sep 17 00:00:00 2001 From: Alan Agius Date: Tue, 14 Jul 2020 09:07:59 +0200 Subject: [PATCH] refactor(@ngtools/webpack): remove unused files warnings We will be moving away from strict file inclusion to address issues where unused files are not part of the program like when having solutions style tsconfig such as #18040. --- .../specs/unused-files-warning_spec.ts | 281 ------------------ .../webpack/src/angular_compiler_plugin.ts | 33 +- 2 files changed, 5 insertions(+), 309 deletions(-) delete mode 100644 packages/angular_devkit/build_angular/src/browser/specs/unused-files-warning_spec.ts diff --git a/packages/angular_devkit/build_angular/src/browser/specs/unused-files-warning_spec.ts b/packages/angular_devkit/build_angular/src/browser/specs/unused-files-warning_spec.ts deleted file mode 100644 index 23c9b0ab4a3e..000000000000 --- a/packages/angular_devkit/build_angular/src/browser/specs/unused-files-warning_spec.ts +++ /dev/null @@ -1,281 +0,0 @@ -/** - * @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 { BrowserBuilderOutput } from '@angular-devkit/build-angular'; -import { debounceTime, take, tap } from 'rxjs/operators'; -import { createArchitect, host, veEnabled } from '../../test-utils'; - -// tslint:disable-next-line:no-big-function -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 (veEnabled) { - // 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 (veEnabled) { - // 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 excluded files are unused', async () => { - if (veEnabled) { - // TODO: https://github.com/angular/angular-cli/issues/15056 - pending('Only supported in Ivy.'); - - return; - } - - const ignoredFiles = { - 'src/file.d.ts': 'export type MyType = number;', - 'src/file.ngsummary.ts': 'export const hello = 42;', - 'src/file.ngfactory.ts': 'export const hello = 42;', - 'src/file.ngstyle.ts': 'export const hello = 42;', - 'src/file.ng_typecheck__.ts': 'export const hello = 42;', - }; - - host.writeMultipleFiles(ignoredFiles); - - host.replaceInFile( - 'src/tsconfig.app.json', - '"main.ts"', - `"main.ts", ${Object.keys(ignoredFiles).map(f => `"${f.replace('src/', '')}"`).join(',')}`, - ); - - 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 not show warning when type files are used', async () => { - if (veEnabled) { - // 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('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 - 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(); - }); - - it('should only show warning once per file', async () => { - if (veEnabled) { - // 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"', - ); - - // Write a used file - host.writeMultipleFiles({ - 'src/testing/type.ts': 'export type MyType = number;', - }); - - 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 have type.ts as unused. - expect(logger.includes(`type.ts ${warningMessageSuffix}`)).toBe(true, `Case ${buildNumber} failed.`); - - // touch a file to trigger a rebuild - host.appendToFile('src/main.ts', ''); - break; - case 2: - // The second should should have type.ts as unused but shouldn't warn. - expect(logger.includes(warningMessageSuffix)).toBe(false, `Case ${buildNumber} failed.`); - break; - } - - logger.clear(); - }), - take(2), - ) - .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 dd6ac7971ad2..1551e33b2a3c 100644 --- a/packages/ngtools/webpack/src/angular_compiler_plugin.ts +++ b/packages/ngtools/webpack/src/angular_compiler_plugin.ts @@ -108,7 +108,6 @@ export class AngularCompilerPlugin { // This is needed because if the first build fails we need to do a full emit // even whe only a single file gets updated. private _hadFullJitEmit: boolean | undefined; - private _unusedFiles = new Set(); private _typeDeps = new Set(); private _changedFileExtensions = new Set(['ts', 'tsx', 'html', 'css', 'js', 'json']); private _nodeModulesRegExp = /[\\\/]node_modules[\\\/]/; @@ -620,11 +619,7 @@ export class AngularCompilerPlugin { } } - 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 - // Don't do checks for compilations with errors, since that can result in a partial compilation. + private _findTypeDependencies(compilation: compilation.Compilation) { if (!this._compilerOptions.enableIvy || compilation.errors.length > 0) { return; } @@ -646,21 +641,15 @@ export class AngularCompilerPlugin { // 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))); + .filter(f => !(fileExcludeRegExp.test(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); // This function removes a source file name and all its dependencies from the set. const removeSourceFile = (fileName: string, originalModule = false) => { - if (unusedSourceFileNames.has(fileName) || (originalModule && typeDepFileNames.has(fileName))) { - unusedSourceFileNames.delete(fileName); - if (originalModule) { - typeDepFileNames.delete(fileName); - } + if (originalModule && typeDepFileNames.has(fileName)) { + typeDepFileNames.delete(fileName); this.getDependencies(fileName, false).forEach(f => removeSourceFile(f)); } }; @@ -668,18 +657,6 @@ export class AngularCompilerPlugin { // 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. - 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.`, - ); - 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. @@ -705,7 +682,7 @@ export class AngularCompilerPlugin { // cleanup if not watching compiler.hooks.thisCompilation.tap('angular-compiler', compilation => { compilation.hooks.finishModules.tap('angular-compiler', () => { - this._checkUnusedFiles(compilation); + this._findTypeDependencies(compilation); let rootCompiler = compiler; while (rootCompiler.parentCompilation) {