From 9fc3d4080cfe9bfe607915e16968a658d2673af5 Mon Sep 17 00:00:00 2001 From: Filipe Silva Date: Sat, 21 Jan 2017 19:14:52 +0000 Subject: [PATCH 1/2] fix(AoTPlugin): don't override context module deps Fix #2496 --- packages/@ngtools/webpack/src/plugin.ts | 31 +++++++++++++++------ packages/@ngtools/webpack/src/utils.ts | 18 ++++++++---- tests/e2e/tests/misc/lazy-module.ts | 37 +++++++++++++++++++++++-- 3 files changed, 68 insertions(+), 18 deletions(-) diff --git a/packages/@ngtools/webpack/src/plugin.ts b/packages/@ngtools/webpack/src/plugin.ts index 3c43a823b574..3cc0695df06a 100644 --- a/packages/@ngtools/webpack/src/plugin.ts +++ b/packages/@ngtools/webpack/src/plugin.ts @@ -200,6 +200,11 @@ export class AotPlugin implements Tapable { return callback(); } + // alter only request from @angular/core/src/linker + if (!request.context.endsWith(path.join('@angular/core/src/linker'))) { + return callback(null, request); + } + request.request = this.skipCodeGeneration ? this.basePath : this.genDir; request.recursive = true; request.dependencies.forEach((d: any) => d.critical = false); @@ -210,16 +215,24 @@ export class AotPlugin implements Tapable { return callback(); } - this.done.then(() => { - result.resource = this.skipCodeGeneration ? this.basePath : this.genDir; - result.recursive = true; - result.dependencies.forEach((d: any) => d.critical = false); - result.resolveDependencies = createResolveDependenciesFromContextMap( - (_: any, cb: any) => cb(null, this._lazyRoutes)); - + // there is no way to find the original request, so try to + // match request from @angular/core/src/linker as best as possible + if ( + result.resource == (this.skipCodeGeneration ? this.basePath : this.genDir) + && result.recursive == true + && result.dependencies.every((d: any) => d.critical == false) + ) { + this.done.then(() => { + result.resolveDependencies = createResolveDependenciesFromContextMap( + this._lazyRoutes, + result.resolveDependencies + ); + return callback(null, result); + }, () => callback(null)) + .catch(err => callback(err)); + } else { return callback(null, result); - }, () => callback(null)) - .catch(err => callback(err)); + } }); }); diff --git a/packages/@ngtools/webpack/src/utils.ts b/packages/@ngtools/webpack/src/utils.ts index 954cc206ad9b..847edfcbbb0b 100644 --- a/packages/@ngtools/webpack/src/utils.ts +++ b/packages/@ngtools/webpack/src/utils.ts @@ -1,16 +1,22 @@ const ContextElementDependency = require('webpack/lib/dependencies/ContextElementDependency'); -export function createResolveDependenciesFromContextMap(createContextMap: Function) { +export function createResolveDependenciesFromContextMap( + contextMap: { [route: string]: string }, + originalResolveDependencies: any +) { return (fs: any, resource: any, recursive: any, regExp: RegExp, callback: any) => { - createContextMap(fs, function(err: Error, map: any) { + const newCallback = (err: any, deps: any) => { if (err) { return callback(err); } - const dependencies = Object.keys(map) - .map((key) => new ContextElementDependency(map[key], key)); + const dependencies = Object.keys(contextMap) + .map((key) => new ContextElementDependency(contextMap[key], key)); - callback(null, dependencies); - }); + callback(null, deps.concat(dependencies)); + }; + + // Use original resolver and add in context map to result + originalResolveDependencies(fs, resource, recursive, regExp, newCallback); }; } diff --git a/tests/e2e/tests/misc/lazy-module.ts b/tests/e2e/tests/misc/lazy-module.ts index d3d33482d877..5e95a8dd8319 100644 --- a/tests/e2e/tests/misc/lazy-module.ts +++ b/tests/e2e/tests/misc/lazy-module.ts @@ -1,8 +1,9 @@ import {readdirSync} from 'fs'; import {oneLine} from 'common-tags'; -import {ng} from '../../utils/process'; +import {ng, npm} from '../../utils/process'; import {addImportToModule} from '../../utils/ast'; +import {appendToFile, writeFile} from '../../utils/fs'; export default function() { @@ -21,13 +22,43 @@ export default function() { if (oldNumberOfFiles >= currentNumberOfDistFiles) { throw new Error('A bundle for the lazy module was not created.'); } + oldNumberOfFiles = currentNumberOfDistFiles; + }) + // verify System.import still works + .then(() => writeFile('src/app/lazy-file.ts', '')) + .then(() => appendToFile('src/app/app.component.ts', ` + // verify other System.import still work + declare var System: any; + const lazyFile = 'file'; + System.import('./lazy-' + lazyFile); + `)) + .then(() => ng('build')) + .then(() => readdirSync('dist').length) + .then(currentNumberOfDistFiles => { + if (oldNumberOfFiles >= currentNumberOfDistFiles) { + throw new Error('A bundle for the lazy file was not created.'); + } + oldNumberOfFiles = currentNumberOfDistFiles; + }) + // verify 'import *' syntax doesn't break lazy modules + .then(() => npm('install', 'moment')) + .then(() => appendToFile('src/app/app.component.ts', ` + import * as moment from 'moment'; + console.log(moment); + `)) + .then(() => ng('build')) + .then(() => readdirSync('dist').length) + .then(currentNumberOfDistFiles => { + if (oldNumberOfFiles != currentNumberOfDistFiles) { + throw new Error('Bundles were not created after adding \'import *\'.'); + } }) // Check for AoT and lazy routes. .then(() => ng('build', '--aot')) .then(() => readdirSync('dist').length) .then(currentNumberOfDistFiles => { - if (oldNumberOfFiles >= currentNumberOfDistFiles) { - throw new Error('A bundle for the lazy module was not created.'); + if (oldNumberOfFiles != currentNumberOfDistFiles) { + throw new Error('AoT build contains a different number of files.'); } }); } From 2b6af243f3de112c15083662ef0e3da9eaa3aa65 Mon Sep 17 00:00:00 2001 From: Filipe Silva Date: Sun, 22 Jan 2017 00:19:12 +0000 Subject: [PATCH 2/2] add clydins solution --- packages/@ngtools/webpack/src/plugin.ts | 49 +++++++++---------------- packages/@ngtools/webpack/src/utils.ts | 22 ----------- 2 files changed, 17 insertions(+), 54 deletions(-) delete mode 100644 packages/@ngtools/webpack/src/utils.ts diff --git a/packages/@ngtools/webpack/src/plugin.ts b/packages/@ngtools/webpack/src/plugin.ts index 3cc0695df06a..bb309206cad7 100644 --- a/packages/@ngtools/webpack/src/plugin.ts +++ b/packages/@ngtools/webpack/src/plugin.ts @@ -4,9 +4,9 @@ import * as ts from 'typescript'; import {__NGTOOLS_PRIVATE_API_2} from '@angular/compiler-cli'; import {AngularCompilerOptions} from '@angular/tsc-wrapped'; +const ContextElementDependency = require('webpack/lib/dependencies/ContextElementDependency'); import {WebpackResourceLoader} from './resource_loader'; -import {createResolveDependenciesFromContextMap} from './utils'; import {WebpackCompilerHost} from './compiler_host'; import {resolveEntryModuleFromMain} from './entry_resolver'; import {Tapable} from './webpack'; @@ -194,45 +194,30 @@ export class AotPlugin implements Tapable { apply(compiler: any) { this._compiler = compiler; + // Add lazy modules to the context module for @angular/core/src/linker compiler.plugin('context-module-factory', (cmf: any) => { - cmf.plugin('before-resolve', (request: any, callback: (err?: any, request?: any) => void) => { - if (!request) { - return callback(); - } - - // alter only request from @angular/core/src/linker - if (!request.context.endsWith(path.join('@angular/core/src/linker'))) { - return callback(null, request); - } - - request.request = this.skipCodeGeneration ? this.basePath : this.genDir; - request.recursive = true; - request.dependencies.forEach((d: any) => d.critical = false); - return callback(null, request); - }); cmf.plugin('after-resolve', (result: any, callback: (err?: any, request?: any) => void) => { if (!result) { return callback(); } - // there is no way to find the original request, so try to - // match request from @angular/core/src/linker as best as possible - if ( - result.resource == (this.skipCodeGeneration ? this.basePath : this.genDir) - && result.recursive == true - && result.dependencies.every((d: any) => d.critical == false) - ) { - this.done.then(() => { - result.resolveDependencies = createResolveDependenciesFromContextMap( - this._lazyRoutes, - result.resolveDependencies - ); - return callback(null, result); - }, () => callback(null)) - .catch(err => callback(err)); - } else { + // alter only request from @angular/core/src/linker + if (!result.resource.endsWith(path.join('@angular/core/src/linker'))) { return callback(null, result); } + + this.done.then(() => { + result.resource = this.skipCodeGeneration ? this.basePath : this.genDir; + result.recursive = true; + 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)); + cb(null, dependencies); + }; + return callback(null, result); + }, () => callback(null)) + .catch(err => callback(err)); }); }); diff --git a/packages/@ngtools/webpack/src/utils.ts b/packages/@ngtools/webpack/src/utils.ts deleted file mode 100644 index 847edfcbbb0b..000000000000 --- a/packages/@ngtools/webpack/src/utils.ts +++ /dev/null @@ -1,22 +0,0 @@ -const ContextElementDependency = require('webpack/lib/dependencies/ContextElementDependency'); - -export function createResolveDependenciesFromContextMap( - contextMap: { [route: string]: string }, - originalResolveDependencies: any -) { - return (fs: any, resource: any, recursive: any, regExp: RegExp, callback: any) => { - const newCallback = (err: any, deps: any) => { - if (err) { - return callback(err); - } - - const dependencies = Object.keys(contextMap) - .map((key) => new ContextElementDependency(contextMap[key], key)); - - callback(null, deps.concat(dependencies)); - }; - - // Use original resolver and add in context map to result - originalResolveDependencies(fs, resource, recursive, regExp, newCallback); - }; -}