Skip to content

Commit 63f0049

Browse files
alan-agius4vikerman
authored andcommitted
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
1 parent 069a182 commit 63f0049

File tree

2 files changed

+213
-3
lines changed

2 files changed

+213
-3
lines changed
Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
/**
2+
* @license
3+
* Copyright Google Inc. All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
import { Architect } from '@angular-devkit/architect';
9+
import { TestLogger } from '@angular-devkit/architect/testing';
10+
import { debounceTime, take, tap } from 'rxjs/operators';
11+
import { BrowserBuilderOutput } from '../../src/browser/index';
12+
import { createArchitect, host, ivyEnabled } from '../utils';
13+
14+
15+
describe('Browser Builder unused files warnings', () => {
16+
const warningMessageSuffix = `is part of the TypeScript compilation but it's unused`;
17+
const targetSpec = { project: 'app', target: 'build' };
18+
let architect: Architect;
19+
20+
beforeEach(async () => {
21+
await host.initialize().toPromise();
22+
23+
architect = (await createArchitect(host.root())).architect;
24+
});
25+
afterEach(async () => host.restore().toPromise());
26+
27+
it('should not show warning when all files are used', async () => {
28+
if (!ivyEnabled) {
29+
// TODO: https://github.com/angular/angular-cli/issues/15056
30+
pending('Only supported in Ivy.');
31+
32+
return;
33+
}
34+
35+
const logger = new TestLogger('unused-files-warnings');
36+
const run = await architect.scheduleTarget(targetSpec, undefined, { logger });
37+
const output = await run.result as BrowserBuilderOutput;
38+
expect(output.success).toBe(true);
39+
expect(logger.includes(warningMessageSuffix)).toBe(false);
40+
logger.clear();
41+
42+
await run.stop();
43+
});
44+
45+
it('should show warning when some files are unused', async () => {
46+
if (!ivyEnabled) {
47+
// TODO: https://github.com/angular/angular-cli/issues/15056
48+
pending('Only supported in Ivy.');
49+
50+
return;
51+
}
52+
53+
host.replaceInFile(
54+
'src/tsconfig.app.json',
55+
'"main.ts"',
56+
'"main.ts", "environments/environment.prod.ts"',
57+
);
58+
59+
const logger = new TestLogger('unused-files-warnings');
60+
const run = await architect.scheduleTarget(targetSpec, undefined, { logger });
61+
const output = await run.result as BrowserBuilderOutput;
62+
expect(output.success).toBe(true);
63+
expect(logger.includes(`environment.prod.ts ${warningMessageSuffix}`)).toBe(true);
64+
logger.clear();
65+
66+
await run.stop();
67+
});
68+
69+
it('should not show warning when type files are used', async () => {
70+
if (!ivyEnabled) {
71+
// TODO: https://github.com/angular/angular-cli/issues/15056
72+
pending('Only supported in Ivy.');
73+
74+
return;
75+
}
76+
77+
host.writeMultipleFiles({
78+
'src/app/type.ts': 'export type MyType = number;'
79+
});
80+
81+
host.replaceInFile(
82+
'src/app/app.component.ts',
83+
`'@angular/core';`,
84+
`'@angular/core';\nimport { MyType } from './type';\n`
85+
);
86+
87+
const logger = new TestLogger('unused-files-warnings');
88+
const run = await architect.scheduleTarget(targetSpec, undefined, { logger });
89+
const output = await run.result as BrowserBuilderOutput;
90+
expect(output.success).toBe(true);
91+
expect(logger.includes(warningMessageSuffix)).toBe(false);
92+
logger.clear();
93+
94+
await run.stop();
95+
});
96+
97+
it('works for rebuilds', async () => {
98+
if (!ivyEnabled) {
99+
// TODO: https://github.com/angular/angular-cli/issues/15056
100+
pending('Only supported in Ivy.');
101+
102+
return;
103+
}
104+
105+
host.replaceInFile(
106+
'src/tsconfig.app.json',
107+
'"**/*.d.ts"',
108+
'"**/*.d.ts", "testing/**/*.ts"',
109+
);
110+
111+
const logger = new TestLogger('unused-files-warnings');
112+
let buildNumber = 0;
113+
const run = await architect.scheduleTarget(targetSpec, { watch: true }, { logger });
114+
115+
await run.output
116+
.pipe(
117+
debounceTime(1000),
118+
tap(buildEvent => {
119+
expect(buildEvent.success).toBe(true);
120+
121+
buildNumber ++;
122+
switch (buildNumber) {
123+
case 1:
124+
// The first should not have unused files
125+
expect(logger.includes(warningMessageSuffix)).toBe(false, `Case ${buildNumber} failed.`);
126+
127+
// Write a used file
128+
host.writeMultipleFiles({
129+
'src/testing/type.ts': 'export type MyType = number;'
130+
});
131+
132+
// touch file to trigger build
133+
host.replaceInFile(
134+
'src/app/app.component.ts',
135+
`'@angular/core';`,
136+
`'@angular/core';\n`
137+
);
138+
break;
139+
140+
case 2:
141+
// The second should have type.ts as unused
142+
expect(logger.includes(`type.ts ${warningMessageSuffix}`)).toBe(true, `Case ${buildNumber} failed.`);
143+
144+
host.replaceInFile(
145+
'src/app/app.component.ts',
146+
`'@angular/core';`,
147+
`'@angular/core';\nimport { MyType } from '../testing/type';`
148+
);
149+
break;
150+
151+
case 3:
152+
// The third should not have any unused files
153+
expect(logger.includes(warningMessageSuffix)).toBe(false, `Case ${buildNumber} failed.`);
154+
break;
155+
}
156+
157+
logger.clear();
158+
}),
159+
take(3),
160+
)
161+
.toPromise();
162+
await run.stop();
163+
});
164+
165+
});

packages/ngtools/webpack/src/angular_compiler_plugin.ts

Lines changed: 48 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -593,6 +593,50 @@ export class AngularCompilerPlugin {
593593
}
594594
}
595595

596+
private _warnOnUnusedFiles(compilation: compilation.Compilation) {
597+
// Only do the unused TS files checks when under Ivy
598+
// since previously we did include unused files in the compilation
599+
// See: https://github.com/angular/angular-cli/pull/15030
600+
if (!this._compilerOptions.enableIvy) {
601+
return;
602+
}
603+
604+
const program = this._getTsProgram();
605+
if (!program) {
606+
return;
607+
}
608+
609+
// Exclude the following files from unused checks
610+
// - ngfactories & ngstyle might not have a correspondent
611+
// JS file example `@angular/core/core.ngfactory.ts`.
612+
// - .d.ts files might not have a correspondent JS file due to bundling.
613+
// - __ng_typecheck__.ts will never be requested.
614+
const fileExcludeRegExp = /(\.(d|ngfactory|ngstyle)\.ts|ng_typecheck__\.ts)$/;
615+
616+
const usedFiles = new Set<string>();
617+
for (const compilationModule of compilation.modules) {
618+
if (!compilationModule.resource) {
619+
continue;
620+
}
621+
622+
usedFiles.add(forwardSlashPath(compilationModule.resource));
623+
624+
// We need the below for dependencies which
625+
// are not emitted such as type only TS files
626+
for (const dependency of compilationModule.buildInfo.fileDependencies) {
627+
usedFiles.add(forwardSlashPath(dependency));
628+
}
629+
}
630+
631+
const unusedFilesWarning = program.getSourceFiles()
632+
.filter(({ fileName }) => !fileExcludeRegExp.test(fileName) && !usedFiles.has(fileName))
633+
.map(({ fileName }) => `${fileName} is part of the TypeScript compilation but it's unused.`);
634+
635+
if (unusedFilesWarning.length) {
636+
compilation.warnings.push(...unusedFilesWarning);
637+
}
638+
}
639+
596640
// Registration hook for webpack plugin.
597641
// tslint:disable-next-line:no-big-function
598642
apply(compiler: Compiler & { watchMode?: boolean, parentCompilation?: compilation.Compilation }) {
@@ -609,6 +653,8 @@ export class AngularCompilerPlugin {
609653
// cleanup if not watching
610654
compiler.hooks.thisCompilation.tap('angular-compiler', compilation => {
611655
compilation.hooks.finishModules.tap('angular-compiler', () => {
656+
this._warnOnUnusedFiles(compilation);
657+
612658
let rootCompiler = compiler;
613659
while (rootCompiler.parentCompilation) {
614660
// tslint:disable-next-line:no-any
@@ -1174,7 +1220,7 @@ export class AngularCompilerPlugin {
11741220
return null;
11751221
}
11761222
})
1177-
.filter(x => x);
1223+
.filter(x => x) as string[];
11781224

11791225
const resourceImports = findResources(sourceFile)
11801226
.map(resourcePath => resolve(dirname(resolvedFileName), normalize(resourcePath)));
@@ -1186,8 +1232,7 @@ export class AngularCompilerPlugin {
11861232
...this.getResourceDependencies(this._compilerHost.denormalizePath(resolvedFileName)),
11871233
].map((p) => p && this._compilerHost.denormalizePath(p)));
11881234

1189-
return [...uniqueDependencies]
1190-
.filter(x => !!x) as string[];
1235+
return [...uniqueDependencies];
11911236
}
11921237

11931238
getResourceDependencies(fileName: string): string[] {

0 commit comments

Comments
 (0)