Skip to content

Commit 5cba79c

Browse files
filipesilvadgp1130
authored andcommitted
fix(@ngtools/webpack): fix rebuilds for transitive and global type deps
Fix #15856 (cherry picked from commit bf39e74)
1 parent b925207 commit 5cba79c

File tree

3 files changed

+98
-15
lines changed

3 files changed

+98
-15
lines changed

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

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,52 @@ describe('Browser Builder rebuilds', () => {
222222
.toPromise();
223223
});
224224

225+
it('rebuilds on transitive type-only file changes', async () => {
226+
if (veEnabled) {
227+
// TODO: https://github.com/angular/angular-cli/issues/15056
228+
pending('Only supported in Ivy.');
229+
230+
return;
231+
}
232+
host.writeMultipleFiles({
233+
'src/interface1.ts': `
234+
import { Interface2 } from './interface2';
235+
export interface Interface1 extends Interface2 { }
236+
`,
237+
'src/interface2.ts': `
238+
import { Interface3 } from './interface3';
239+
export interface Interface2 extends Interface3 { }
240+
`,
241+
'src/interface3.ts': `export interface Interface3 { nbr: number; }`,
242+
});
243+
host.appendToFile('src/main.ts', `
244+
import { Interface1 } from './interface1';
245+
const something: Interface1 = { nbr: 43 };
246+
`);
247+
248+
const overrides = { watch: true };
249+
const run = await architect.scheduleTarget(target, overrides);
250+
let buildNumber = 0;
251+
await run.output
252+
.pipe(
253+
debounceTime(rebuildDebounceTime),
254+
tap(buildEvent => expect(buildEvent.success).toBe(true)),
255+
tap(() => {
256+
// NOTE: this only works for transitive type deps after the first build, and only if the
257+
// typedep file was there on the previous build.
258+
// Make sure the first rebuild is triggered on a direct dep (typedep or not).
259+
buildNumber++;
260+
if (buildNumber < 4) {
261+
host.appendToFile(`src/interface${buildNumber}.ts`, `export type MyType = string;`);
262+
} else {
263+
host.appendToFile(`src/typings.d.ts`, `export type MyType = string;`);
264+
}
265+
}),
266+
take(5),
267+
)
268+
.toPromise();
269+
});
270+
225271
it('rebuilds after errors in JIT', async () => {
226272
const origContent = virtualFs.fileBufferToString(
227273
host.scopedSync().read(normalize('src/app/app.component.ts')),

packages/ngtools/webpack/src/angular_compiler_plugin.ts

Lines changed: 48 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,9 @@ export class AngularCompilerPlugin {
110110
// even whe only a single file gets updated.
111111
private _hadFullJitEmit: boolean | undefined;
112112
private _unusedFiles = new Set<string>();
113+
private _typeDeps = new Set<string>();
113114
private _changedFileExtensions = new Set(['ts', 'tsx', 'html', 'css', 'js', 'json']);
115+
private _nodeModulesRegExp = /[\\\/]node_modules[\\\/]/;
114116

115117
// Webpack plugin.
116118
private _firstRun = true;
@@ -592,7 +594,7 @@ export class AngularCompilerPlugin {
592594
}
593595
}
594596

595-
private _warnOnUnusedFiles(compilation: compilation.Compilation) {
597+
private _checkUnusedFiles(compilation: compilation.Compilation) {
596598
// Only do the unused TS files checks when under Ivy
597599
// since previously we did include unused files in the compilation
598600
// See: https://github.com/angular/angular-cli/pull/15030
@@ -601,6 +603,7 @@ export class AngularCompilerPlugin {
601603
return;
602604
}
603605

606+
// Bail if there's no TS program. Nothing to do in that case.
604607
const program = this._getTsProgram();
605608
if (!program) {
606609
return;
@@ -609,26 +612,36 @@ export class AngularCompilerPlugin {
609612
// Exclude the following files from unused checks
610613
// - ngfactories & ngstyle might not have a correspondent
611614
// JS file example `@angular/core/core.ngfactory.ts`.
612-
// - .d.ts files might not have a correspondent JS file due to bundling.
613615
// - __ng_typecheck__.ts will never be requested.
614-
const fileExcludeRegExp = /(\.(d|ngfactory|ngstyle|ngsummary)\.ts|ng_typecheck__\.ts)$/;
616+
const fileExcludeRegExp = /(\.(ngfactory|ngstyle|ngsummary)\.ts|ng_typecheck__\.ts)$/;
617+
618+
// Start all the source file names we care about.
619+
// Ignore matches to the regexp above, files we've already reported once before, and
620+
// node_modules.
621+
const sourceFiles = program.getSourceFiles()
622+
.map(x => this._compilerHost.denormalizePath(x.fileName))
623+
.filter(f => !(fileExcludeRegExp.test(f) || this._unusedFiles.has(f)
624+
|| this._nodeModulesRegExp.test(f)));
625+
626+
// Make a set with the sources, but exclude .d.ts files since those are type-only.
627+
const unusedSourceFileNames = new Set(sourceFiles.filter(f => !f.endsWith('.d.ts')));
628+
// Separately keep track of type-only deps.
629+
const typeDepFileNames = new Set(sourceFiles);
615630

616-
// Start with a set of all the source file names we care about.
617-
const unusedSourceFileNames = new Set(
618-
program.getSourceFiles()
619-
.map(x => this._compilerHost.denormalizePath(x.fileName))
620-
.filter(f => !(fileExcludeRegExp.test(f) || this._unusedFiles.has(f))),
621-
);
622631
// This function removes a source file name and all its dependencies from the set.
623-
const removeSourceFile = (fileName: string) => {
624-
if (unusedSourceFileNames.has(fileName)) {
632+
const removeSourceFile = (fileName: string, originalModule = false) => {
633+
if (unusedSourceFileNames.has(fileName)
634+
|| (originalModule && typeDepFileNames.has(fileName))) {
625635
unusedSourceFileNames.delete(fileName);
636+
if (originalModule) {
637+
typeDepFileNames.delete(fileName);
638+
}
626639
this.getDependencies(fileName, false).forEach(f => removeSourceFile(f));
627640
}
628641
};
629642

630-
// Go over all the modules in the webpack compilation and remove them from the set.
631-
compilation.modules.forEach(m => m.resource ? removeSourceFile(m.resource) : null);
643+
// Go over all the modules in the webpack compilation and remove them from the sets.
644+
compilation.modules.forEach(m => m.resource ? removeSourceFile(m.resource, true) : null);
632645

633646
// Anything that remains is unused, because it wasn't referenced directly or transitively
634647
// on the files in the compilation.
@@ -638,7 +651,15 @@ export class AngularCompilerPlugin {
638651
`Add only entry points to the 'files' or 'include' properties in your tsconfig.`,
639652
);
640653
this._unusedFiles.add(fileName);
654+
// Remove the truly unused from the type dep list.
655+
typeDepFileNames.delete(fileName);
641656
}
657+
658+
// At this point we know what the type deps are.
659+
// These are the TS files that weren't part of the compilation modules, aren't unused, but were
660+
// part of the TS original source list.
661+
// Next build we add them to the TS entry points so that they trigger rebuilds.
662+
this._typeDeps = typeDepFileNames;
642663
}
643664

644665
// Registration hook for webpack plugin.
@@ -657,7 +678,7 @@ export class AngularCompilerPlugin {
657678
// cleanup if not watching
658679
compiler.hooks.thisCompilation.tap('angular-compiler', compilation => {
659680
compilation.hooks.finishModules.tap('angular-compiler', () => {
660-
this._warnOnUnusedFiles(compilation);
681+
this._checkUnusedFiles(compilation);
661682

662683
let rootCompiler = compiler;
663684
while (rootCompiler.parentCompilation) {
@@ -1188,7 +1209,7 @@ export class AngularCompilerPlugin {
11881209
let msg = `${fileName} is missing from the TypeScript compilation. `
11891210
+ `Please make sure it is in your tsconfig via the 'files' or 'include' property.`;
11901211

1191-
if (/(\\|\/)node_modules(\\|\/)/.test(fileName)) {
1212+
if (this._nodeModulesRegExp.test(fileName)) {
11921213
msg += '\nThe missing file seems to be part of a third party library. '
11931214
+ 'TS files in published libraries are often a sign of a badly packaged library. '
11941215
+ 'Please open an issue in the library repository to alert its author and ask them '
@@ -1267,6 +1288,18 @@ export class AngularCompilerPlugin {
12671288
return this._resourceLoader.getResourceDependencies(resolvedFileName);
12681289
}
12691290

1291+
getTypeDependencies(fileName: string): string[] {
1292+
// We currently add all type deps directly to the main path.
1293+
// If there's no main path or the lookup isn't the main path, bail.
1294+
if (!this._mainPath || this._compilerHost.resolve(fileName) != this._mainPath) {
1295+
return [];
1296+
}
1297+
1298+
// Note: this set is always for the previous build, not the current build.
1299+
// It should be better than not having rebuilds on type deps but isn't 100% correct.
1300+
return Array.from(this._typeDeps);
1301+
}
1302+
12701303
// This code mostly comes from `performCompilation` in `@angular/compiler-cli`.
12711304
// It skips the program creation because we need to use `loadNgStructureAsync()`,
12721305
// and uses CustomTransformers.

packages/ngtools/webpack/src/loader.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,10 @@ export function ngcLoader(this: loader.LoaderContext) {
100100
styleDependencies.forEach(dep => this.addDependency(dep));
101101
}
102102

103+
// Add type-only dependencies that should trigger a rebuild when they change.
104+
const typeDependencies = plugin.getTypeDependencies(sourceFileName);
105+
typeDependencies.forEach(dep => this.addDependency(dep));
106+
103107
timeEnd(timeLabel);
104108
cb(null, result.outputText, result.sourceMap as any);
105109
})

0 commit comments

Comments
 (0)