Skip to content

Commit d820065

Browse files
committed
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
1 parent 3cd6a33 commit d820065

File tree

4 files changed

+58
-131
lines changed

4 files changed

+58
-131
lines changed

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

+47-2
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/ngtools/webpack/src/angular_compiler_plugin.ts

+7-28
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

-100
This file was deleted.

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

+4-1
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)