Skip to content

Commit fd1af33

Browse files
committed
fix(aot): Use the proper path when statically analyzing lazy routes.
Before, we were using paths relative to base at all time, but these might not be the paths we get in System.import(), therefore we have to keep the relative path. BREAKING CHANGES: Using relative paths might lead to path clashing. We now properly output an error in this case. Fixes angular#2452 Fixes angular#2900
1 parent cabc9dc commit fd1af33

File tree

5 files changed

+98
-32
lines changed

5 files changed

+98
-32
lines changed

package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "angular-cli",
3-
"version": "1.0.0-beta.19",
3+
"version": "1.0.0-beta.19-3",
44
"description": "CLI tool for Angular",
55
"main": "packages/angular-cli/lib/cli/index.js",
66
"trackingCode": "UA-8594346-19",

packages/angular-cli/package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "angular-cli",
3-
"version": "1.0.0-beta.19",
3+
"version": "1.0.0-beta.19-3",
44
"description": "CLI tool for Angular",
55
"main": "lib/cli/index.js",
66
"trackingCode": "UA-8594346-19",

packages/webpack/src/compiler_host.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -109,9 +109,9 @@ export class WebpackCompilerHost implements ts.CompilerHost {
109109
this._changed = true;
110110
}
111111

112-
populateWebpackResolver(resolver: any) {
112+
populateWebpackResolver(resolver: any, force: boolean = false) {
113113
const fs = resolver.fileSystem;
114-
if (!this._changed) {
114+
if (!this._changed && !force) {
115115
return;
116116
}
117117

packages/webpack/src/loader.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ export function ngcLoader(source: string) {
147147
if (plugin && plugin instanceof AotPlugin) {
148148
const cb: any = this.async();
149149

150-
plugin.done
150+
Promise.resolve()
151151
.then(() => _removeDecorators(this.resource, source))
152152
.then(sourceText => _replaceBootstrap(this.resource, sourceText, plugin))
153153
.then(sourceText => {

packages/webpack/src/plugin.ts

+93-27
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {WebpackResourceLoader} from './resource_loader';
1111
import {createResolveDependenciesFromContextMap} from './utils';
1212
import {WebpackCompilerHost} from './compiler_host';
1313
import {resolveEntryModuleFromMain} from './entry_resolver';
14+
import {StaticSymbol} from '@angular/compiler-cli';
1415

1516

1617
/**
@@ -25,8 +26,24 @@ export interface AotPluginOptions {
2526
}
2627

2728

29+
export interface LazyRoute {
30+
moduleRoute: ModuleRoute;
31+
moduleRelativePath: string;
32+
moduleAbsolutePath: string;
33+
}
34+
35+
36+
export interface LazyRouteMap {
37+
[path: string]: LazyRoute;
38+
}
39+
40+
2841
export class ModuleRoute {
29-
constructor(public readonly path: string, public readonly className: string = null) {}
42+
constructor(public readonly path: string, public readonly className: string = null) {
43+
if (arguments.length == 1) {
44+
[this.path, this.className] = path.split('#');
45+
}
46+
}
3047

3148
toString() {
3249
return `${this.path}#${this.className}`;
@@ -168,9 +185,9 @@ export class AotPlugin {
168185

169186
// Virtual file system.
170187
compiler.resolvers.normal.plugin('resolve', (request: any, cb?: () => void) => {
171-
// Populate the file system cache with the virtual module.
172-
this._compilerHost.populateWebpackResolver(compiler.resolvers.normal);
173-
if (cb) {
188+
if (request.request.match(/\.ts$/)) {
189+
this.done.then(() => cb());
190+
} else {
174191
cb();
175192
}
176193
});
@@ -227,49 +244,85 @@ export class AotPlugin {
227244
throw new Error(message);
228245
}
229246
})
247+
.then(() => {
248+
// Populate the file system cache with the virtual module.
249+
this._compilerHost.populateWebpackResolver(this._compiler.resolvers.normal);
250+
})
230251
.then(() => {
231252
// Process the lazy routes
232-
this._lazyRoutes =
233-
this._processNgModule(this._entryModule, null)
234-
.map(module => ModuleRoute.fromString(module))
235-
.reduce((lazyRoutes: any, module: ModuleRoute) => {
236-
lazyRoutes[`${module.path}.ngfactory`] = path.join(
237-
this.genDir, module.path + '.ngfactory.ts');
238-
return lazyRoutes;
239-
}, {});
253+
this._lazyRoutes = {};
254+
const allLazyRoutes = this._processNgModule(this._entryModule, null);
255+
Object.keys(allLazyRoutes)
256+
.forEach(k => {
257+
const lazyRoute = allLazyRoutes[k];
258+
this._lazyRoutes[k + '.ngfactory'] = lazyRoute.moduleRelativePath + '.ngfactory.ts';
259+
});
240260
})
241-
.then(() => cb(), (err: any) => cb(err));
261+
.then(() => cb(), (err: any) => { cb(err); });
242262
}
243263

244-
private _resolveModule(module: ModuleRoute, containingFile: string) {
264+
private _resolveModulePath(module: ModuleRoute, containingFile: string) {
245265
if (module.path.startsWith('.')) {
246266
return path.join(path.dirname(containingFile), module.path);
247267
}
248268
return module.path;
249269
}
250270

251-
private _processNgModule(module: ModuleRoute, containingFile: string | null): string[] {
271+
private _processNgModule(module: ModuleRoute, containingFile: string | null): LazyRouteMap {
252272
const modulePath = containingFile ? module.path : ('./' + path.basename(module.path));
253273
if (containingFile === null) {
254274
containingFile = module.path + '.ts';
255275
}
276+
const relativeModulePath = this._resolveModulePath(module, containingFile);
256277

257-
const resolvedModulePath = this._resolveModule(module, containingFile);
258278
const staticSymbol = this._reflectorHost
259279
.findDeclaration(modulePath, module.className, containingFile);
260280
const entryNgModuleMetadata = this.getNgModuleMetadata(staticSymbol);
261-
const loadChildren = this.extractLoadChildren(entryNgModuleMetadata);
262-
const result = loadChildren.map(route => {
263-
return this._resolveModule(new ModuleRoute(route), resolvedModulePath);
264-
});
281+
const loadChildrenRoute: LazyRoute[] = this.extractLoadChildren(entryNgModuleMetadata)
282+
.map(route => {
283+
const mr = ModuleRoute.fromString(route);
284+
const relativePath = this._resolveModulePath(mr, relativeModulePath);
285+
const absolutePath = path.join(this.genDir, relativePath);
286+
return {
287+
moduleRoute: mr,
288+
moduleRelativePath: relativePath,
289+
moduleAbsolutePath: absolutePath
290+
};
291+
});
292+
const resultMap: LazyRouteMap = loadChildrenRoute
293+
.reduce((acc: LazyRouteMap, curr: LazyRoute) => {
294+
const key = curr.moduleRoute.path;
295+
if (acc[key]) {
296+
if (acc[key].moduleAbsolutePath != curr.moduleAbsolutePath) {
297+
throw new Error(`Duplicated path in loadChildren detected: "${key}" is used in 2 ` +
298+
'loadChildren, but they point to different modules. Webpack cannot distinguish ' +
299+
'between the two based on context and would fail to load the proper one.');
300+
}
301+
} else {
302+
acc[key] = curr;
303+
}
304+
return acc;
305+
}, {});
265306

266307
// Also concatenate every child of child modules.
267-
for (const route of loadChildren) {
268-
const childModule = ModuleRoute.fromString(route);
269-
const children = this._processNgModule(childModule, resolvedModulePath + '.ts');
270-
result.push(...children);
308+
for (const lazyRoute of loadChildrenRoute) {
309+
const mr = lazyRoute.moduleRoute;
310+
const children = this._processNgModule(mr, relativeModulePath);
311+
Object.keys(children).forEach(p => {
312+
const child = children[p];
313+
const key = child.moduleRoute.path;
314+
if (resultMap[key]) {
315+
if (resultMap[key].moduleAbsolutePath != child.moduleAbsolutePath) {
316+
throw new Error(`Duplicated path in loadChildren detected: "${key}" is used in 2 ` +
317+
'loadChildren, but they point to different modules. Webpack cannot distinguish ' +
318+
'between the two based on context and would fail to load the proper one.');
319+
}
320+
} else {
321+
resultMap[key] = child;
322+
}
323+
});
271324
}
272-
return result;
325+
return resultMap;
273326
}
274327

275328
private getNgModuleMetadata(staticSymbol: ngCompiler.StaticSymbol) {
@@ -281,10 +334,23 @@ export class AotPlugin {
281334
}
282335

283336
private extractLoadChildren(ngModuleDecorator: any): any[] {
284-
const routes = ngModuleDecorator.imports.reduce((mem: any[], m: any) => {
337+
const routes = (ngModuleDecorator.imports || []).reduce((mem: any[], m: any) => {
285338
return mem.concat(this.collectRoutes(m.providers));
286339
}, this.collectRoutes(ngModuleDecorator.providers));
287-
return this.collectLoadChildren(routes);
340+
return this.collectLoadChildren(routes)
341+
.concat((ngModuleDecorator.imports || [])
342+
// Also recursively extractLoadChildren of modules we import.
343+
.map((staticSymbol: any) => {
344+
if (staticSymbol instanceof StaticSymbol) {
345+
const entryNgModuleMetadata = this.getNgModuleMetadata(staticSymbol);
346+
return this.extractLoadChildren(entryNgModuleMetadata);
347+
} else {
348+
return [];
349+
}
350+
})
351+
// Poor man's flat map.
352+
.reduce((acc: any[], i: any) => acc.concat(i), []))
353+
.filter(x => !!x);
288354
}
289355

290356
private collectRoutes(providers: any[]): any[] {

0 commit comments

Comments
 (0)