diff --git a/packages/@ngtools/webpack/src/compiler_host.ts b/packages/@ngtools/webpack/src/compiler_host.ts index 608e79dfa2a7..d7af11ada7c6 100644 --- a/packages/@ngtools/webpack/src/compiler_host.ts +++ b/packages/@ngtools/webpack/src/compiler_host.ts @@ -95,7 +95,9 @@ export class WebpackCompilerHost implements ts.CompilerHost { private _delegate: ts.CompilerHost; private _files: {[path: string]: VirtualFileStats} = Object.create(null); private _directories: {[path: string]: VirtualDirStats} = Object.create(null); - private _changed = false; + + private _changedFiles: {[path: string]: boolean} = Object.create(null); + private _changedDirs: {[path: string]: boolean} = Object.create(null); private _basePath: string; private _setParentNodes: boolean; @@ -129,10 +131,15 @@ export class WebpackCompilerHost implements ts.CompilerHost { let p = dirname(fileName); while (p && !this._directories[p]) { this._directories[p] = new VirtualDirStats(p); + this._changedDirs[p] = true; p = dirname(p); } - this._changed = true; + this._changedFiles[fileName] = true; + } + + get dirty() { + return Object.keys(this._changedFiles).length > 0; } enableCaching() { @@ -141,21 +148,26 @@ export class WebpackCompilerHost implements ts.CompilerHost { populateWebpackResolver(resolver: any) { const fs = resolver.fileSystem; - if (!this._changed) { + if (!this.dirty) { return; } const isWindows = process.platform.startsWith('win'); - for (const fileName of Object.keys(this._files)) { + for (const fileName of this.getChangedFilePaths()) { const stats = this._files[fileName]; if (stats) { // If we're on windows, we need to populate with the proper path separator. const path = isWindows ? fileName.replace(/\//g, '\\') : fileName; fs._statStorage.data[path] = [null, stats]; fs._readFileStorage.data[path] = [null, stats.content]; + } else { + // Support removing files as well. + const path = isWindows ? fileName.replace(/\//g, '\\') : fileName; + fs._statStorage.data[path] = [new Error(), null]; + fs._readFileStorage.data[path] = [new Error(), null]; } } - for (const dirName of Object.keys(this._directories)) { + for (const dirName of Object.keys(this._changedDirs)) { const stats = this._directories[dirName]; const dirs = this.getDirectories(dirName); const files = this.getFiles(dirName); @@ -164,12 +176,19 @@ export class WebpackCompilerHost implements ts.CompilerHost { fs._statStorage.data[path] = [null, stats]; fs._readdirStorage.data[path] = [null, files.concat(dirs)]; } + } - this._changed = false; + resetChangedFileTracker() { + this._changedFiles = Object.create(null); + this._changedDirs = Object.create(null); + } + getChangedFilePaths(): string[] { + return Object.keys(this._changedFiles); } invalidate(fileName: string): void { this._files[fileName] = null; + this._changedFiles[fileName] = true; } fileExists(fileName: string): boolean { diff --git a/packages/@ngtools/webpack/src/lazy_routes.ts b/packages/@ngtools/webpack/src/lazy_routes.ts new file mode 100644 index 000000000000..8fee4bfce2f5 --- /dev/null +++ b/packages/@ngtools/webpack/src/lazy_routes.ts @@ -0,0 +1,66 @@ +import {dirname, join} from 'path'; +import * as ts from 'typescript'; + +import {TypeScriptFileRefactor} from './refactor'; + + +function _getContentOfKeyLiteral(source: ts.SourceFile, node: ts.Node): string { + if (node.kind == ts.SyntaxKind.Identifier) { + return (node as ts.Identifier).text; + } else if (node.kind == ts.SyntaxKind.StringLiteral) { + return (node as ts.StringLiteral).text; + } else { + return null; + } +} + + +export interface LazyRouteMap { + [path: string]: string; +} + + +export function findLazyRoutes(filePath: string, + program: ts.Program, + host: ts.CompilerHost): LazyRouteMap { + const refactor = new TypeScriptFileRefactor(filePath, host, program); + + return refactor + // Find all object literals in the file. + .findAstNodes(null, ts.SyntaxKind.ObjectLiteralExpression, true) + // Get all their property assignments. + .map((node: ts.ObjectLiteralExpression) => { + return refactor.findAstNodes(node, ts.SyntaxKind.PropertyAssignment, false); + }) + // Take all `loadChildren` elements. + .reduce((acc: ts.PropertyAssignment[], props: ts.PropertyAssignment[]) => { + return acc.concat(props.filter(literal => { + return _getContentOfKeyLiteral(refactor.sourceFile, literal.name) == 'loadChildren'; + })); + }, []) + // Get only string values. + .filter((node: ts.PropertyAssignment) => node.initializer.kind == ts.SyntaxKind.StringLiteral) + // Get the string value. + .map((node: ts.PropertyAssignment) => (node.initializer as ts.StringLiteral).text) + // Map those to either [path, absoluteModulePath], or [path, null] if the module pointing to + // does not exist. + .map((routePath: string) => { + const moduleName = routePath.split('#')[0]; + const resolvedModuleName: ts.ResolvedModuleWithFailedLookupLocations = moduleName[0] == '.' + ? { resolvedModule: { resolvedFileName: join(dirname(filePath), moduleName) + '.ts' }, + failedLookupLocations: [] } + : ts.resolveModuleName(moduleName, filePath, program.getCompilerOptions(), host); + if (resolvedModuleName.resolvedModule + && resolvedModuleName.resolvedModule.resolvedFileName + && host.fileExists(resolvedModuleName.resolvedModule.resolvedFileName)) { + return [routePath, resolvedModuleName.resolvedModule.resolvedFileName]; + } else { + return [routePath, null]; + } + }) + // Reduce to the LazyRouteMap map. + .reduce((acc: LazyRouteMap, [routePath, resolvedModuleName]: [string, string | null]) => { + acc[routePath] = resolvedModuleName; + return acc; + }, {}); +} diff --git a/packages/@ngtools/webpack/src/plugin.ts b/packages/@ngtools/webpack/src/plugin.ts index fe7f14684f3e..b310845f92d9 100644 --- a/packages/@ngtools/webpack/src/plugin.ts +++ b/packages/@ngtools/webpack/src/plugin.ts @@ -11,6 +11,7 @@ import {WebpackCompilerHost} from './compiler_host'; import {resolveEntryModuleFromMain} from './entry_resolver'; import {Tapable} from './webpack'; import {PathsPlugin} from './paths-plugin'; +import {findLazyRoutes, LazyRouteMap} from './lazy_routes'; /** @@ -39,7 +40,7 @@ export class AotPlugin implements Tapable { private _rootFilePath: string[]; private _compilerHost: WebpackCompilerHost; private _resourceLoader: WebpackResourceLoader; - private _lazyRoutes: { [route: string]: string }; + private _lazyRoutes: LazyRouteMap = Object.create(null); private _tsConfigPath: string; private _entryModule: string; @@ -56,6 +57,8 @@ export class AotPlugin implements Tapable { private _i18nFormat: string; private _locale: string; + private _firstRun = true; + constructor(options: AotPluginOptions) { this._setupOptions(options); } @@ -78,6 +81,7 @@ export class AotPlugin implements Tapable { get i18nFile() { return this._i18nFile; } get i18nFormat() { return this._i18nFormat; } get locale() { return this._locale; } + get firstRun() { return this._firstRun; } private _setupOptions(options: AotPluginOptions) { // Fill in the missing options. @@ -194,6 +198,31 @@ export class AotPlugin implements Tapable { } } + private _findLazyRoutesInAst(): LazyRouteMap { + const result: LazyRouteMap = Object.create(null); + const changedFilePaths = this._compilerHost.getChangedFilePaths(); + for (const filePath of changedFilePaths) { + const fileLazyRoutes = findLazyRoutes(filePath, this._program, this._compilerHost); + for (const routeKey of Object.keys(fileLazyRoutes)) { + const route = fileLazyRoutes[routeKey]; + if (routeKey in this._lazyRoutes) { + if (route === null) { + this._lazyRoutes[routeKey] = null; + } else if (this._lazyRoutes[routeKey] !== route) { + this._compilation.warnings.push( + new Error(`Duplicated path in loadChildren detected during a rebuild. ` + + `We will take the latest version detected and override it to save rebuild time. ` + + `You should perform a full build to validate that your routes don't overlap.`) + ); + } + } else { + result[routeKey] = route; + } + } + } + return result; + } + // registration hook for webpack plugin apply(compiler: any) { this._compiler = compiler; @@ -220,7 +249,15 @@ export class AotPlugin implements Tapable { result.dependencies.forEach((d: any) => d.critical = false); result.resolveDependencies = (p1: any, p2: any, p3: any, p4: RegExp, cb: any ) => { const dependencies = Object.keys(this._lazyRoutes) - .map((key) => new ContextElementDependency(this._lazyRoutes[key], key)); + .map((key) => { + const value = this._lazyRoutes[key]; + if (value !== null) { + return new ContextElementDependency(value, key); + } else { + return null; + } + }) + .filter(x => !!x); cb(null, dependencies); }; return callback(null, result); @@ -314,18 +351,26 @@ export class AotPlugin implements Tapable { this._compilerHost.populateWebpackResolver(this._compiler.resolvers.normal); }) .then(() => { - // Process the lazy routes - this._lazyRoutes = {}; - const allLazyRoutes = __NGTOOLS_PRIVATE_API_2.listLazyRoutes({ - program: this._program, - host: this._compilerHost, - angularCompilerOptions: this._angularCompilerOptions, - entryModule: this._entryModule - }); - Object.keys(allLazyRoutes) + // We need to run the `listLazyRoutes` the first time because it also navigates libraries + // and other things that we might miss using the findLazyRoutesInAst. + let discoveredLazyRoutes: LazyRouteMap = this.firstRun ? + __NGTOOLS_PRIVATE_API_2.listLazyRoutes({ + program: this._program, + host: this._compilerHost, + angularCompilerOptions: this._angularCompilerOptions, + entryModule: this._entryModule + }) + : this._findLazyRoutesInAst(); + + // Process the lazy routes discovered. + Object.keys(discoveredLazyRoutes) .forEach(k => { - const lazyRoute = allLazyRoutes[k]; + const lazyRoute = discoveredLazyRoutes[k]; k = k.split('#')[0]; + if (lazyRoute === null) { + return; + } + if (this.skipCodeGeneration) { this._lazyRoutes[k] = lazyRoute; } else { @@ -334,7 +379,13 @@ export class AotPlugin implements Tapable { } }); }) - .then(() => cb(), (err: any) => { + .then(() => { + this._compilerHost.resetChangedFileTracker(); + + // Only turn this off for the first successful run. + this._firstRun = false; + cb(); + }, (err: any) => { compilation.errors.push(err); cb(); }); diff --git a/packages/angular-cli/models/webpack-configs/common.ts b/packages/angular-cli/models/webpack-configs/common.ts index 1835704df9b9..bee0221f76cf 100644 --- a/packages/angular-cli/models/webpack-configs/common.ts +++ b/packages/angular-cli/models/webpack-configs/common.ts @@ -67,7 +67,7 @@ export function getCommonConfig(wco: WebpackConfigOptions) { extraPlugins.push(new webpack.optimize.CommonsChunkPlugin({ name: 'vendor', chunks: ['main'], - minChunks: (module: any) => module.userRequest && module.userRequest.startsWith(nodeModules) + minChunks: (module: any) => module.resource && module.resource.startsWith(nodeModules) })); } diff --git a/tests/e2e/tests/build/rebuild.ts b/tests/e2e/tests/build/rebuild.ts new file mode 100644 index 000000000000..de914664edb4 --- /dev/null +++ b/tests/e2e/tests/build/rebuild.ts @@ -0,0 +1,73 @@ +import { + killAllProcesses, + exec, + waitForAnyProcessOutputToMatch, + silentExecAndWaitForOutputToMatch, + ng, execAndWaitForOutputToMatch, +} from '../../utils/process'; +import {writeFile} from '../../utils/fs'; +import {wait} from '../../utils/utils'; + + +export default function() { + if (process.platform.startsWith('win')) { + return Promise.resolve(); + } + + let oldNumberOfChunks = 0; + const chunkRegExp = /chunk\s+\{/g; + + return execAndWaitForOutputToMatch('ng', ['serve'], /webpack: bundle is now VALID/) + // Should trigger a rebuild. + .then(() => exec('touch', 'src/main.ts')) + .then(() => waitForAnyProcessOutputToMatch(/webpack: bundle is now INVALID/, 1000)) + .then(() => waitForAnyProcessOutputToMatch(/webpack: bundle is now VALID/, 5000)) + // Count the bundles. + .then((stdout: string) => { + oldNumberOfChunks = stdout.split(chunkRegExp).length; + }) + // Add a lazy module. + .then(() => ng('generate', 'module', 'lazy', '--routing')) + // Just wait for the rebuild, otherwise we might be validating this build. + .then(() => wait(1000)) + .then(() => writeFile('src/app/app.module.ts', ` + import { BrowserModule } from '@angular/platform-browser'; + import { NgModule } from '@angular/core'; + import { FormsModule } from '@angular/forms'; + import { HttpModule } from '@angular/http'; + + import { AppComponent } from './app.component'; + import { RouterModule } from '@angular/router'; + + @NgModule({ + declarations: [ + AppComponent + ], + imports: [ + BrowserModule, + FormsModule, + HttpModule, + RouterModule.forRoot([ + { path: 'lazy', loadChildren: './lazy/lazy.module#LazyModule' } + ]) + ], + providers: [], + bootstrap: [AppComponent] + }) + export class AppModule { } + `)) + // Should trigger a rebuild with a new bundle. + .then(() => waitForAnyProcessOutputToMatch(/webpack: bundle is now INVALID/, 1000)) + .then(() => waitForAnyProcessOutputToMatch(/webpack: bundle is now VALID/, 5000)) + // Count the bundles. + .then((stdout: string) => { + let newNumberOfChunks = stdout.split(chunkRegExp).length; + if (oldNumberOfChunks >= newNumberOfChunks) { + throw new Error('Expected webpack to create a new chunk, but did not.'); + } + }) + .then(() => killAllProcesses(), (err: any) => { + killAllProcesses(); + throw err; + }); +} diff --git a/tests/e2e/tests/packages/webpack/test.ts b/tests/e2e/tests/packages/webpack/test.ts index a540c9760b1b..ff844d957913 100644 --- a/tests/e2e/tests/packages/webpack/test.ts +++ b/tests/e2e/tests/packages/webpack/test.ts @@ -16,5 +16,6 @@ export default function(skipCleaning: () => void) { .then(() => replaceInFile('app/app.component.ts', './app.component.scss', 'app.component.scss')) .then(() => exec(normalize('node_modules/.bin/webpack'), '-p')) + // test .then(() => skipCleaning()); } diff --git a/tests/e2e/utils/process.ts b/tests/e2e/utils/process.ts index ff26447ff838..02b79addfd42 100644 --- a/tests/e2e/utils/process.ts +++ b/tests/e2e/utils/process.ts @@ -83,6 +83,26 @@ function _exec(options: ExecOptions, cmd: string, args: string[]): Promise { + // Race between _all_ processes, and the timeout. First one to resolve/reject wins. + return Promise.race(_processes.map(childProcess => new Promise(resolve => { + let stdout = ''; + childProcess.stdout.on('data', (data: Buffer) => { + stdout += data.toString(); + if (data.toString().match(match)) { + resolve(stdout); + } + }); + })).concat([ + new Promise((resolve, reject) => { + // Wait for 30 seconds and timeout. + setTimeout(() => { + reject(new Error(`Waiting for ${match} timed out (timeout: ${timeout}msec)...`)); + }, timeout); + }) + ])); +} + export function killAllProcesses(signal = 'SIGTERM') { _processes.forEach(process => treeKill(process.pid, signal)); _processes = [];