Skip to content

Commit edb84b3

Browse files
alan-agius4Keen Yee Liau
alan-agius4
authored and
Keen Yee Liau
committed
fix(@ngtools/webpack): emit lazy routes errors on rebuilds (#12418)
* fix(@ngtools/webpack): emit lazy routes errors on rebuilds At the moment lazy route errors are only emitted in the initial builds because in following builds we are only processed lazy routes that are declared in the changed files. At the moment, we cannot cache the previously resolved routes as there is no way to track in which file the lazy route was declared so that we can bust the lazy route if it was removed. Closes #12236 * test: add test for compilation errors in watch mode Closes #12311
1 parent 8bbba9d commit edb84b3

5 files changed

Lines changed: 85 additions & 131 deletions

File tree

‎packages/angular_devkit/build_angular/test/browser/lazy-module_spec_large.ts‎

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import { DefaultTimeout, runTargetSpec } from '@angular-devkit/architect/testing';
9+
import { DefaultTimeout, TestLogger, runTargetSpec } from '@angular-devkit/architect/testing';
1010
import { join, normalize } from '@angular-devkit/core';
11-
import { tap } from 'rxjs/operators';
11+
import { take, tap } from 'rxjs/operators';
1212
import { BrowserBuilderSchema } from '../../src';
1313
import { browserTargetSpec, host } from '../utils';
1414

@@ -70,6 +70,7 @@ export const lazyModuleImport: { [path: string]: string } = {
7070
`,
7171
};
7272

73+
// tslint:disable-next-line:no-big-function
7374
describe('Browser Builder lazy modules', () => {
7475

7576
const outputPath = normalize('dist');
@@ -89,6 +90,50 @@ describe('Browser Builder lazy modules', () => {
8990
).toPromise().then(done, done.fail);
9091
});
9192

93+
it('should show error when lazy route is invalid on watch mode AOT', (done) => {
94+
host.writeMultipleFiles(lazyModuleFiles);
95+
host.writeMultipleFiles(lazyModuleImport);
96+
host.replaceInFile(
97+
'src/app/app.module.ts',
98+
'lazy.module#LazyModule',
99+
'invalid.module#LazyModule',
100+
);
101+
102+
const logger = new TestLogger('rebuild-lazy-errors');
103+
const overrides = { watch: true, aot: true };
104+
runTargetSpec(host, browserTargetSpec, overrides, DefaultTimeout, logger).pipe(
105+
tap((buildEvent) => expect(buildEvent.success).toBe(false)),
106+
tap(() => {
107+
expect(logger.includes('Could not resolve module')).toBe(true);
108+
logger.clear();
109+
host.appendToFile('src/main.ts', ' ');
110+
}),
111+
take(2),
112+
).toPromise().then(done, done.fail);
113+
});
114+
115+
it('should show error when lazy route is invalid on watch mode JIT', (done) => {
116+
host.writeMultipleFiles(lazyModuleFiles);
117+
host.writeMultipleFiles(lazyModuleImport);
118+
host.replaceInFile(
119+
'src/app/app.module.ts',
120+
'lazy.module#LazyModule',
121+
'invalid.module#LazyModule',
122+
);
123+
124+
const logger = new TestLogger('rebuild-lazy-errors');
125+
const overrides = { watch: true, aot: false };
126+
runTargetSpec(host, browserTargetSpec, overrides, DefaultTimeout, logger).pipe(
127+
tap((buildEvent) => expect(buildEvent.success).toBe(false)),
128+
tap(() => {
129+
expect(logger.includes('Could not resolve module')).toBe(true);
130+
logger.clear();
131+
host.appendToFile('src/main.ts', ' ');
132+
}),
133+
take(2),
134+
).toPromise().then(done, done.fail);
135+
});
136+
92137
it('supports lazy bundle for lazy routes with AOT', (done) => {
93138
host.writeMultipleFiles(lazyModuleFiles);
94139
host.writeMultipleFiles(lazyModuleImport);

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,33 @@ describe('Browser Builder rebuilds', () => {
198198
).toPromise().then(done, done.fail);
199199
});
200200

201+
it('rebuilds shows error', (done) => {
202+
host.replaceInFile('./src/app/app.component.ts', 'AppComponent', 'AppComponentZ');
203+
204+
const overrides = { watch: true, aot: false };
205+
let buildCount = 1;
206+
const logger = new TestLogger('rebuild-errors');
207+
208+
runTargetSpec(host, browserTargetSpec, overrides, DefaultTimeout * 3, logger).pipe(
209+
tap((buildEvent) => {
210+
switch (buildCount) {
211+
case 1:
212+
expect(buildEvent.success).toBe(false);
213+
expect(logger.includes('AppComponent cannot be used as an entry component')).toBe(true);
214+
logger.clear();
215+
216+
host.replaceInFile('./src/app/app.component.ts', 'AppComponentZ', 'AppComponent');
217+
break;
218+
219+
default:
220+
expect(buildEvent.success).toBe(true);
221+
break;
222+
}
223+
buildCount ++;
224+
}),
225+
take(2),
226+
).toPromise().then(done, done.fail);
227+
});
201228

202229
it('rebuilds after errors in AOT', (done) => {
203230
// Save the original contents of `./src/app/app.component.ts`.

‎packages/ngtools/webpack/src/angular_compiler_plugin.ts‎

Lines changed: 7 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,10 @@ import { time, timeEnd } from './benchmark';
4040
import { WebpackCompilerHost, workaroundResolve } from './compiler_host';
4141
import { resolveEntryModuleFromMain } from './entry_resolver';
4242
import { gatherDiagnostics, hasErrors } from './gather_diagnostics';
43-
import { LazyRouteMap, findLazyRoutes } from './lazy_routes';
4443
import { TypeScriptPathsPlugin } from './paths-plugin';
4544
import { WebpackResourceLoader } from './resource_loader';
4645
import {
46+
LazyRouteMap,
4747
exportLazyModuleMap,
4848
exportNgFactory,
4949
findResources,
@@ -418,22 +418,6 @@ export class AngularCompilerPlugin {
418418
}
419419
}
420420

421-
private _findLazyRoutesInAst(changedFilePaths: string[]): LazyRouteMap {
422-
time('AngularCompilerPlugin._findLazyRoutesInAst');
423-
const result: LazyRouteMap = Object.create(null);
424-
for (const filePath of changedFilePaths) {
425-
const fileLazyRoutes = findLazyRoutes(filePath, this._compilerHost, undefined,
426-
this._compilerOptions);
427-
for (const routeKey of Object.keys(fileLazyRoutes)) {
428-
const route = fileLazyRoutes[routeKey];
429-
result[routeKey] = route;
430-
}
431-
}
432-
timeEnd('AngularCompilerPlugin._findLazyRoutesInAst');
433-
434-
return result;
435-
}
436-
437421
private _listLazyRoutesFromProgram(): LazyRouteMap {
438422
let lazyRoutes: LazyRoute[];
439423
if (this._JitMode) {
@@ -876,17 +860,12 @@ export class AngularCompilerPlugin {
876860
// We need to run the `listLazyRoutes` the first time because it also navigates libraries
877861
// and other things that we might miss using the (faster) findLazyRoutesInAst.
878862
// Lazy routes modules will be read with compilerHost and added to the changed files.
879-
if (this._firstRun || !this._JitMode) {
880-
this._processLazyRoutes(this._listLazyRoutesFromProgram());
881-
} else {
882-
const changedTsFiles = this._getChangedTsFiles();
883-
if (changedTsFiles.length > 0) {
884-
this._processLazyRoutes(this._findLazyRoutesInAst(changedTsFiles));
885-
}
886-
}
887-
if (this._options.additionalLazyModules) {
888-
this._processLazyRoutes(this._options.additionalLazyModules);
889-
}
863+
const lazyRouteMap: LazyRouteMap = {
864+
... (this._entryModule || !this._JitMode ? this._listLazyRoutesFromProgram() : {}),
865+
...this._options.additionalLazyModules,
866+
};
867+
868+
this._processLazyRoutes(lazyRouteMap);
890869

891870
// Emit and report errors.
892871

‎packages/ngtools/webpack/src/lazy_routes.ts‎

Lines changed: 0 additions & 100 deletions
This file was deleted.

‎packages/ngtools/webpack/src/transformers/export_lazy_module_map.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,14 @@
77
*/
88
import * as path from 'path';
99
import * as ts from 'typescript';
10-
import { LazyRouteMap } from '../lazy_routes';
1110
import { getFirstNode, getLastNode } from './ast_helpers';
1211
import { AddNodeOperation, StandardTransform, TransformOperation } from './interfaces';
1312
import { makeTransform } from './make_transform';
1413

14+
export interface LazyRouteMap {
15+
[path: string]: string;
16+
}
17+
1518
export function exportLazyModuleMap(
1619
shouldTransform: (fileName: string) => boolean,
1720
lazyRoutesCb: () => LazyRouteMap,

0 commit comments

Comments
 (0)