From edbace11750d2f490825abb3bd0012c9a64e6401 Mon Sep 17 00:00:00 2001 From: Filipe Silva Date: Thu, 28 Feb 2019 13:58:23 +0000 Subject: [PATCH 1/6] feat(@ngtools/webpack): add option to control lazy route discovery --- .../webpack/src/angular_compiler_plugin.ts | 180 ++++++++++-------- 1 file changed, 105 insertions(+), 75 deletions(-) diff --git a/packages/ngtools/webpack/src/angular_compiler_plugin.ts b/packages/ngtools/webpack/src/angular_compiler_plugin.ts index e42eccdd6c83..6f4c4fb470e3 100644 --- a/packages/ngtools/webpack/src/angular_compiler_plugin.ts +++ b/packages/ngtools/webpack/src/angular_compiler_plugin.ts @@ -103,6 +103,11 @@ export interface AngularCompilerPluginOptions { nameLazyFiles?: boolean; logger?: logging.Logger; directTemplateLoading?: boolean; + // When using the loadChildren string syntax, @ngtools/webpack must query @angular/compiler-cli + // via a private API to know which lazy routes exist. This increases build and rebuild time. + // When using Ivy, the string syntax is not supported at all. Thus we shouldn't attempt that. + // This option is also used for when the compilation doesn't need this sort of processing at all. + discoverLazyRoutes?: boolean; // added to the list of lazy routes additionalLazyModules?: { [module: string]: string }; @@ -134,6 +139,7 @@ export class AngularCompilerPlugin { private _compilerHost: WebpackCompilerHost & CompilerHost; private _moduleResolutionCache: ts.ModuleResolutionCache; private _resourceLoader?: WebpackResourceLoader; + private _discoverLazyRoutes = true; // Contains `moduleImportPath#exportName` => `fullModulePath`. private _lazyRoutes: LazyRouteMap = {}; private _tsConfigPath: string; @@ -292,6 +298,26 @@ export class AngularCompilerPlugin { this._platformTransformers = options.platformTransformers; } + if (options.discoverLazyRoutes !== undefined) { + this._discoverLazyRoutes = options.discoverLazyRoutes; + } + + if (this._discoverLazyRoutes === false && this.options.additionalLazyModuleResources + && this.options.additionalLazyModuleResources.length > 0) { + this._warnings.push( + new Error(`Lazy route discovery is disabled but additional Lazy Module Resources were` + + ` provided. These will be ignored.`), + ); + } + + if (this._discoverLazyRoutes === false && this.options.additionalLazyModules + && Object.keys(this.options.additionalLazyModules).length > 0) { + this._warnings.push( + new Error(`Lazy route discovery is disabled but additional lazy modules were provided.` + + `These will be ignored.`), + ); + } + // Default ContextElementDependency to the one we can import from here. // Failing to use the right ContextElementDependency will throw the error below: // "No module factory available for dependency type: ContextElementDependency" @@ -411,7 +437,7 @@ export class AngularCompilerPlugin { this._entryModule = resolveEntryModuleFromMain( this._mainPath, this._compilerHost, this._getTsProgram() as ts.Program); - if (!this.entryModule && !this._compilerOptions.enableIvy) { + if (this._discoverLazyRoutes && !this.entryModule && !this._compilerOptions.enableIvy) { this._warnings.push('Lazy routes discovery is not enabled. ' + 'Because there is neither an entryModule nor a ' + 'statically analyzable bootstrap code in the main file.', @@ -697,64 +723,66 @@ export class AngularCompilerPlugin { ); }); - // Add lazy modules to the context module for @angular/core - compiler.hooks.contextModuleFactory.tap('angular-compiler', cmf => { - const angularCorePackagePath = require.resolve('@angular/core/package.json'); - - // APFv6 does not have single FESM anymore. Instead of verifying if we're pointing to - // FESMs, we resolve the `@angular/core` path and verify that the path for the - // module starts with it. - // This may be slower but it will be compatible with both APF5, 6 and potential future - // versions (until the dynamic import appears outside of core I suppose). - // We resolve any symbolic links in order to get the real path that would be used in webpack. - const angularCoreResourceRoot = fs.realpathSync(path.dirname(angularCorePackagePath)); - - cmf.hooks.afterResolve.tapPromise('angular-compiler', async result => { - // Alter only existing request from Angular or one of the additional lazy module resources. - const isLazyModuleResource = (resource: string) => - resource.startsWith(angularCoreResourceRoot) || - ( this.options.additionalLazyModuleResources && - this.options.additionalLazyModuleResources.includes(resource)); - - if (!result || !this.done || !isLazyModuleResource(result.resource)) { - return result; - } - - return this.done.then( - () => { - // This folder does not exist, but we need to give webpack a resource. - // TODO: check if we can't just leave it as is (angularCoreModuleDir). - result.resource = path.join(this._basePath, '$$_lazy_route_resource'); - // tslint:disable-next-line:no-any - result.dependencies.forEach((d: any) => d.critical = false); - // tslint:disable-next-line:no-any - result.resolveDependencies = (_fs: any, options: any, callback: Callback) => { - const dependencies = Object.keys(this._lazyRoutes) - .map((key) => { - const modulePath = this._lazyRoutes[key]; - if (modulePath !== null) { - const name = key.split('#')[0]; - - return new this._contextElementDependencyConstructor(modulePath, name); - } else { - return null; - } - }) - .filter(x => !!x); - - if (this._options.nameLazyFiles) { - options.chunkName = '[request]'; - } - - callback(null, dependencies); - }; - + if (this._discoverLazyRoutes) { + // Add lazy modules to the context module for @angular/core + compiler.hooks.contextModuleFactory.tap('angular-compiler', cmf => { + const angularCorePackagePath = require.resolve('@angular/core/package.json'); + + // APFv6 does not have single FESM anymore. Instead of verifying if we're pointing to + // FESMs, we resolve the `@angular/core` path and verify that the path for the + // module starts with it. + // This may be slower but it will be compatible with both APF5, 6 and potential future + // versions (until the dynamic import appears outside of core I suppose). + // We resolve symbolic links in order to get the real path that would be used in webpack. + const angularCoreResourceRoot = fs.realpathSync(path.dirname(angularCorePackagePath)); + + cmf.hooks.afterResolve.tapPromise('angular-compiler', async result => { + // Alter only existing request from Angular or the additional lazy module resources. + const isLazyModuleResource = (resource: string) => + resource.startsWith(angularCoreResourceRoot) || + (this.options.additionalLazyModuleResources && + this.options.additionalLazyModuleResources.includes(resource)); + + if (!result || !this.done || !isLazyModuleResource(result.resource)) { return result; - }, - () => undefined, - ); + } + + return this.done.then( + () => { + // This folder does not exist, but we need to give webpack a resource. + // TODO: check if we can't just leave it as is (angularCoreModuleDir). + result.resource = path.join(this._basePath, '$$_lazy_route_resource'); + // tslint:disable-next-line:no-any + result.dependencies.forEach((d: any) => d.critical = false); + // tslint:disable-next-line:no-any + result.resolveDependencies = (_fs: any, options: any, callback: Callback) => { + const dependencies = Object.keys(this._lazyRoutes) + .map((key) => { + const modulePath = this._lazyRoutes[key]; + if (modulePath !== null) { + const name = key.split('#')[0]; + + return new this._contextElementDependencyConstructor(modulePath, name); + } else { + return null; + } + }) + .filter(x => !!x); + + if (this._options.nameLazyFiles) { + options.chunkName = '[request]'; + } + + callback(null, dependencies); + }; + + return result; + }, + () => undefined, + ); + }); }); - }); + } // Create and destroy forked type checker on watch mode. compiler.hooks.watchRun.tap('angular-compiler', () => { @@ -922,27 +950,29 @@ export class AngularCompilerPlugin { // Make a new program and load the Angular structure. await this._createOrUpdateProgram(); - // Try to find lazy routes if we have an entry module. - // We need to run the `listLazyRoutes` the first time because it also navigates libraries - // and other things that we might miss using the (faster) findLazyRoutesInAst. - // Lazy routes modules will be read with compilerHost and added to the changed files. - let lazyRouteMap: LazyRouteMap = {}; - if (!this._JitMode || this._firstRun) { - lazyRouteMap = this._listLazyRoutesFromProgram(); - } else { - const changedTsFiles = this._getChangedTsFiles(); - if (changedTsFiles.length > 0) { - lazyRouteMap = this._findLazyRoutesInAst(changedTsFiles); + if (this._discoverLazyRoutes) { + // Try to find lazy routes if we have an entry module. + // We need to run the `listLazyRoutes` the first time because it also navigates libraries + // and other things that we might miss using the (faster) findLazyRoutesInAst. + // Lazy routes modules will be read with compilerHost and added to the changed files. + let lazyRouteMap: LazyRouteMap = {}; + if (!this._JitMode || this._firstRun) { + lazyRouteMap = this._listLazyRoutesFromProgram(); + } else { + const changedTsFiles = this._getChangedTsFiles(); + if (changedTsFiles.length > 0) { + lazyRouteMap = this._findLazyRoutesInAst(changedTsFiles); + } } - } - // Find lazy routes - lazyRouteMap = { - ...lazyRouteMap, - ...this._options.additionalLazyModules, - }; + // Find lazy routes + lazyRouteMap = { + ...lazyRouteMap, + ...this._options.additionalLazyModules, + }; - this._processLazyRoutes(lazyRouteMap); + this._processLazyRoutes(lazyRouteMap); + } // Emit files. time('AngularCompilerPlugin._update._emit'); From 8eacb50d98d63c60f8beb0e27a1ed1675ab63335 Mon Sep 17 00:00:00 2001 From: Filipe Silva Date: Tue, 19 Mar 2019 15:08:34 +0000 Subject: [PATCH 2/6] feat(@ngtools/webpack): support import syntax for loadChildren with ViewEngine This feature ONLY matches the format below: ``` loadChildren: () => import('IMPORT_STRING').then(m => m.EXPORT_NAME) ``` It will not match nor alter variations, for instance: - not using arrow functions - not using `m` as the module argument - using `await` instead of `then` - using a default export (https://github.com/angular/angular/issues/11402) The only parts that can change are the ones in caps: IMPORT_STRING and EXPORT_NAME. --- .../models/webpack-configs/common.ts | 16 +- .../webpack/src/angular_compiler_plugin.ts | 19 +- .../src/transformers/import_factory.ts | 198 ++++++++++++++++++ .../src/transformers/import_factory_spec.ts | 55 +++++ .../ngtools/webpack/src/transformers/index.ts | 1 + 5 files changed, 286 insertions(+), 3 deletions(-) create mode 100644 packages/ngtools/webpack/src/transformers/import_factory.ts create mode 100644 packages/ngtools/webpack/src/transformers/import_factory_spec.ts diff --git a/packages/angular_devkit/build_angular/src/angular-cli-files/models/webpack-configs/common.ts b/packages/angular_devkit/build_angular/src/angular-cli-files/models/webpack-configs/common.ts index 1cf394e0cb25..a9c03738a641 100644 --- a/packages/angular_devkit/build_angular/src/angular-cli-files/models/webpack-configs/common.ts +++ b/packages/angular_devkit/build_angular/src/angular-cli-files/models/webpack-configs/common.ts @@ -8,7 +8,13 @@ import { tags } from '@angular-devkit/core'; import * as CopyWebpackPlugin from 'copy-webpack-plugin'; import * as path from 'path'; -import { Configuration, HashedModuleIdsPlugin, Output, debug } from 'webpack'; +import { + Configuration, + ContextReplacementPlugin, + HashedModuleIdsPlugin, + Output, + debug, +} from 'webpack'; import { AssetPatternClass } from '../../../browser/schema'; import { BundleBudgetPlugin } from '../../plugins/bundle-budget'; import { CleanCssWebpackPlugin } from '../../plugins/cleancss-webpack-plugin'; @@ -345,6 +351,12 @@ export function getCommonConfig(wco: WebpackConfigOptions): Configuration { ...extraMinimizers, ], }, - plugins: extraPlugins, + plugins: [ + // Always replace the context for the System.import in angular/core to prevent warnings. + // https://github.com/angular/angular/issues/11580 + // With VE the correct context is added in @ngtools/webpack, but Ivy doesn't need it at all. + new ContextReplacementPlugin(/\@angular(\\|\/)core(\\|\/)/), + ...extraPlugins, + ], }; } diff --git a/packages/ngtools/webpack/src/angular_compiler_plugin.ts b/packages/ngtools/webpack/src/angular_compiler_plugin.ts index 6f4c4fb470e3..5f079af2abad 100644 --- a/packages/ngtools/webpack/src/angular_compiler_plugin.ts +++ b/packages/ngtools/webpack/src/angular_compiler_plugin.ts @@ -46,6 +46,7 @@ import { exportLazyModuleMap, exportNgFactory, findResources, + importFactory, registerLocaleData, removeDecorators, replaceBootstrap, @@ -108,6 +109,7 @@ export interface AngularCompilerPluginOptions { // When using Ivy, the string syntax is not supported at all. Thus we shouldn't attempt that. // This option is also used for when the compilation doesn't need this sort of processing at all. discoverLazyRoutes?: boolean; + importFactories?: boolean; // added to the list of lazy routes additionalLazyModules?: { [module: string]: string }; @@ -140,6 +142,7 @@ export class AngularCompilerPlugin { private _moduleResolutionCache: ts.ModuleResolutionCache; private _resourceLoader?: WebpackResourceLoader; private _discoverLazyRoutes = true; + private _importFactories = false; // Contains `moduleImportPath#exportName` => `fullModulePath`. private _lazyRoutes: LazyRouteMap = {}; private _tsConfigPath: string; @@ -298,7 +301,12 @@ export class AngularCompilerPlugin { this._platformTransformers = options.platformTransformers; } - if (options.discoverLazyRoutes !== undefined) { + // Determine if lazy route discovery via Compiler CLI private API should be attempted. + if (this._compilerOptions.enableIvy) { + // Never try to discover lazy routes with Ivy. + this._discoverLazyRoutes = false; + } else if (options.discoverLazyRoutes !== undefined) { + // The default is to discover routes, but it can be overriden. this._discoverLazyRoutes = options.discoverLazyRoutes; } @@ -318,6 +326,11 @@ export class AngularCompilerPlugin { ); } + if (!this._compilerOptions.enableIvy && options.importFactories === true) { + // Only transform imports to use factories with View Engine. + this._importFactories = true; + } + // Default ContextElementDependency to the one we can import from here. // Failing to use the right ContextElementDependency will throw the error below: // "No module factory available for dependency type: ContextElementDependency" @@ -896,6 +909,10 @@ export class AngularCompilerPlugin { } else { // Remove unneeded angular decorators. this._transformers.push(removeDecorators(isAppPath, getTypeChecker)); + // Import ngfactory in loadChildren import syntax + if (this._importFactories) { + this._transformers.push(importFactory(msg => this._warnings.push(msg))); + } } if (this._platformTransformers !== null) { diff --git a/packages/ngtools/webpack/src/transformers/import_factory.ts b/packages/ngtools/webpack/src/transformers/import_factory.ts new file mode 100644 index 000000000000..0ff35a7393e8 --- /dev/null +++ b/packages/ngtools/webpack/src/transformers/import_factory.ts @@ -0,0 +1,198 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ +import * as ts from 'typescript'; + + +/** + * Given this original source code: + * + * import { NgModule } from '@angular/core'; + * import { Routes, RouterModule } from '@angular/router'; + * + * const routes: Routes = [{ + * path: 'lazy', + * loadChildren: () => import('./lazy/lazy.module').then(m => m.LazyModule). + * }]; + * + * @NgModule({ + * imports: [RouterModule.forRoot(routes)], + * exports: [RouterModule] + * }) + * export class AppRoutingModule { } + * + * NGC (View Engine) will process it into: + * + * import { Routes } from '@angular/router'; + * const ɵ0 = () => import('./lazy/lazy.module').then(m => m.LazyModule); + * const routes: Routes = [{ + * path: 'lazy', + * loadChildren: ɵ0 + * }]; + * export class AppRoutingModule { + * } + * export { ɵ0 }; + * + * The importFactory transformation will only see the AST after it is process by NGC. + * You can confirm this with the code below: + * + * const res = ts.createPrinter().printNode(ts.EmitHint.Unspecified, sourceFile, sourceFile); + * console.log(`### Original source: \n${sourceFile.text}\n###`); + * console.log(`### Current source: \n${currentText}\n###`); + * + * At this point it doesn't yet matter what the target (ES5/ES2015/etc) is, so the original + * constructs, like `class` and arrow functions, still remain. + * + */ + +export function importFactory( + warningCb: (warning: string) => void, +): ts.TransformerFactory { + return (context: ts.TransformationContext) => { + return (sourceFile: ts.SourceFile) => { + const warning = ` +Found 'loadChildren' with a non-string syntax in ${sourceFile.fileName} but could not transform it. +Make sure it matches the format below: + +loadChildren: () => import('IMPORT_STRING').then(m => m.EXPORT_NAME) + +Please note that only IMPORT_STRING and EXPORT_NAME can be replaced in this format.`; + + const emitWarning = () => warningCb(warning); + const visitVariableStatement: ts.Visitor = (node: ts.Node) => { + if (ts.isVariableDeclaration(node)) { + return replaceImport(node, context, emitWarning); + } + + return ts.visitEachChild(node, visitVariableStatement, context); + }; + + const visitToplevelNodes: ts.Visitor = (node: ts.Node) => { + // We only care about finding variable declarations, which are found in this structure: + // VariableStatement -> VariableDeclarationList -> VariableDeclaration + if (ts.isVariableStatement(node)) { + return ts.visitEachChild(node, visitVariableStatement, context); + } + + // There's no point in recursing into anything but variable statements, so return the node. + return node; + }; + + return ts.visitEachChild(sourceFile, visitToplevelNodes, context); + }; + }; +} + +function replaceImport( + node: ts.VariableDeclaration, + context: ts.TransformationContext, + emitWarning: () => void, +): ts.Node { + // This ONLY matches the original source code format below: + // loadChildren: () => import('IMPORT_STRING').then(m => m.EXPORT_NAME) + // And expects that source code to be transformed by NGC (see comment for importFactory). + // It will not match nor alter variations, for instance: + // - not using arrow functions + // - not using `m` as the module argument + // - using `await` instead of `then` + // - using a default export (https://github.com/angular/angular/issues/11402) + // The only parts that can change are the ones in caps: IMPORT_STRING and EXPORT_NAME. + + // Exit early if the structure is not what we expect. + + // ɵ0 = something + const name = node.name; + if (!( + ts.isIdentifier(name) + && /ɵ\d+/.test(name.text) + )) { + return node; + } + + const initializer = node.initializer; + if (initializer === undefined) { + return node; + } + + // ɵ0 = () => something + if (!( + ts.isArrowFunction(initializer) + && initializer.parameters.length === 0 + )) { + return node; + } + + // ɵ0 = () => something.then(something) + const topArrowFnBody = initializer.body; + if (!ts.isCallExpression(topArrowFnBody)) { + return node; + } + + const topArrowFnBodyExpr = topArrowFnBody.expression; + if (!( + ts.isPropertyAccessExpression(topArrowFnBodyExpr) + && ts.isIdentifier(topArrowFnBodyExpr.name) + )) { + return node; + } + if (topArrowFnBodyExpr.name.text != 'then') { + return node; + } + + // ɵ0 = () => import('IMPORT_STRING').then(something) + const importCall = topArrowFnBodyExpr.expression; + if (!( + ts.isCallExpression(importCall) + && importCall.expression.kind === ts.SyntaxKind.ImportKeyword + && importCall.arguments.length === 1 + && ts.isStringLiteral(importCall.arguments[0]) + )) { + return node; + } + + // ɵ0 = () => import('IMPORT_STRING').then(m => m.EXPORT_NAME) + if (!( + topArrowFnBody.arguments.length === 1 + && ts.isArrowFunction(topArrowFnBody.arguments[0]) + )) { + // Now that we know it's both `ɵ0` (generated by NGC) and a `import()`, start emitting a warning + // if the structure isn't as expected to help users identify unusable syntax. + emitWarning(); + + return node; + } + + const thenArrowFn = topArrowFnBody.arguments[0] as ts.ArrowFunction; + if (!( + thenArrowFn.parameters.length === 1 + && ts.isPropertyAccessExpression(thenArrowFn.body) + && ts.isIdentifier(thenArrowFn.body.name) + )) { + emitWarning(); + + return node; + } + + // At this point we know what are the nodes we need to replace. + const importStringLit = importCall.arguments[0] as ts.StringLiteral; + const exportNameId = thenArrowFn.body.name; + + // The easiest way to alter them is with a simple visitor. + const replacementVisitor: ts.Visitor = (node: ts.Node) => { + if (node === importStringLit) { + // Transform the import string. + return ts.createStringLiteral(importStringLit.text + '.ngfactory'); + } else if (node === exportNameId) { + // Transform the export name. + return ts.createIdentifier(exportNameId.text + 'NgFactory'); + } + + return ts.visitEachChild(node, replacementVisitor, context); + }; + + return ts.visitEachChild(node, replacementVisitor, context); +} diff --git a/packages/ngtools/webpack/src/transformers/import_factory_spec.ts b/packages/ngtools/webpack/src/transformers/import_factory_spec.ts new file mode 100644 index 000000000000..8e199c92b45b --- /dev/null +++ b/packages/ngtools/webpack/src/transformers/import_factory_spec.ts @@ -0,0 +1,55 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ +import { tags } from '@angular-devkit/core'; +import { transformTypescript } from './ast_helpers'; +import { importFactory } from './import_factory'; + +describe('@ngtools/webpack transformers', () => { + describe('import_factory', () => { + it('should support arrow functions', () => { + const input = tags.stripIndent` + const ɵ0 = () => import('./lazy/lazy.module').then(m => m.LazyModule); + const routes = [{ + path: 'lazy', + loadChildren: ɵ0 + }]; + `; + const output = tags.stripIndent` + const ɵ0 = () => import("./lazy/lazy.module.ngfactory").then(m => m.LazyModuleNgFactory); + const routes = [{ + path: 'lazy', + loadChildren: ɵ0 + }]; + `; + + let warningCalled = false; + const transformer = importFactory(() => warningCalled = true); + const result = transformTypescript(input, [transformer]); + + expect(tags.oneLine`${result}`).toEqual(tags.oneLine`${output}`); + expect(warningCalled).toBeFalsy(); + }); + + it('should not transform if the format is different than expected', () => { + const input = tags.stripIndent` + const ɵ0 = () => import('./lazy/lazy.module').then(function (m) { return m.LazyModule; }); + const routes = [{ + path: 'lazy', + loadChildren: ɵ0 + }]; + `; + + let warningCalled = false; + const transformer = importFactory(() => warningCalled = true); + const result = transformTypescript(input, [transformer]); + + expect(tags.oneLine`${result}`).toEqual(tags.oneLine`${input}`); + expect(warningCalled).toBeTruthy(); + }); + }); +}); diff --git a/packages/ngtools/webpack/src/transformers/index.ts b/packages/ngtools/webpack/src/transformers/index.ts index de51a382ba1d..95ed09de0ce9 100644 --- a/packages/ngtools/webpack/src/transformers/index.ts +++ b/packages/ngtools/webpack/src/transformers/index.ts @@ -18,3 +18,4 @@ export * from './register_locale_data'; export * from './replace_resources'; export * from './remove_decorators'; export * from './find_resources'; +export * from './import_factory'; From ca4cea9791c1136684318d830b2858dc026cb37e Mon Sep 17 00:00:00 2001 From: Filipe Silva Date: Mon, 25 Mar 2019 15:22:59 +0000 Subject: [PATCH 3/6] feat(@angular-devkit/build-angular): add experimentalImportFactories option --- .../src/angular-cli-files/models/build-options.ts | 1 + .../angular-cli-files/models/webpack-configs/typescript.ts | 1 + .../angular_devkit/build_angular/src/browser/schema.json | 5 +++++ packages/ngtools/webpack/src/transformers/import_factory.ts | 6 +++++- 4 files changed, 12 insertions(+), 1 deletion(-) diff --git a/packages/angular_devkit/build_angular/src/angular-cli-files/models/build-options.ts b/packages/angular_devkit/build_angular/src/angular-cli-files/models/build-options.ts index 2397878332ac..993ae9593239 100644 --- a/packages/angular_devkit/build_angular/src/angular-cli-files/models/build-options.ts +++ b/packages/angular_devkit/build_angular/src/angular-cli-files/models/build-options.ts @@ -59,6 +59,7 @@ export interface BuildOptions { forkTypeChecker: boolean; profile?: boolean; es5BrowserSupport?: boolean; + experimentalImportFactories?: boolean; main: string; index: string; diff --git a/packages/angular_devkit/build_angular/src/angular-cli-files/models/webpack-configs/typescript.ts b/packages/angular_devkit/build_angular/src/angular-cli-files/models/webpack-configs/typescript.ts index 56eea03ae40b..d9a0b8a06748 100644 --- a/packages/angular_devkit/build_angular/src/angular-cli-files/models/webpack-configs/typescript.ts +++ b/packages/angular_devkit/build_angular/src/angular-cli-files/models/webpack-configs/typescript.ts @@ -75,6 +75,7 @@ function _createAotPlugin( contextElementDependencyConstructor: require('webpack/lib/dependencies/ContextElementDependency'), logger: wco.logger, directTemplateLoading: true, + importFactories: buildOptions.experimentalImportFactories, ...options, }; return new AngularCompilerPlugin(pluginOptions); diff --git a/packages/angular_devkit/build_angular/src/browser/schema.json b/packages/angular_devkit/build_angular/src/browser/schema.json index e8c21c2406cb..d8aeb05a119e 100644 --- a/packages/angular_devkit/build_angular/src/browser/schema.json +++ b/packages/angular_devkit/build_angular/src/browser/schema.json @@ -315,6 +315,11 @@ "type": "boolean", "default": false, "x-deprecated": true + }, + "experimentalImportFactories": { + "description": "**EXPERIMENTAL** Transform import statements for lazy routes to import factories when using View Engine. Should only be used when switching back and forth between View Engine and Ivy. See https://angular.io/guide/ivy for usage information.", + "type": "boolean", + "default": false } }, "additionalProperties": false, diff --git a/packages/ngtools/webpack/src/transformers/import_factory.ts b/packages/ngtools/webpack/src/transformers/import_factory.ts index 0ff35a7393e8..3750f7b9c0fe 100644 --- a/packages/ngtools/webpack/src/transformers/import_factory.ts +++ b/packages/ngtools/webpack/src/transformers/import_factory.ts @@ -53,6 +53,7 @@ export function importFactory( warningCb: (warning: string) => void, ): ts.TransformerFactory { return (context: ts.TransformationContext) => { + // TODO(filipesilva): change the link to https://angular.io/guide/ivy once it is out. return (sourceFile: ts.SourceFile) => { const warning = ` Found 'loadChildren' with a non-string syntax in ${sourceFile.fileName} but could not transform it. @@ -60,7 +61,10 @@ Make sure it matches the format below: loadChildren: () => import('IMPORT_STRING').then(m => m.EXPORT_NAME) -Please note that only IMPORT_STRING and EXPORT_NAME can be replaced in this format.`; +Please note that only IMPORT_STRING and EXPORT_NAME can be replaced in this format. + +Visit https://next.angular.io/guide/ivy for more information on using Ivy. +`; const emitWarning = () => warningCb(warning); const visitVariableStatement: ts.Visitor = (node: ts.Node) => { From 429fd626273e7449c9dc71832278593cb373a01e Mon Sep 17 00:00:00 2001 From: Filipe Silva Date: Tue, 19 Mar 2019 10:44:07 +0000 Subject: [PATCH 4/6] test: test lazy-loading syntax for VE and Ivy --- .../lazy-load-syntax.ts} | 75 ++++++++++++++----- 1 file changed, 58 insertions(+), 17 deletions(-) rename tests/legacy-cli/e2e/tests/{ivy/ivy-lazy-load.ts => build/lazy-load-syntax.ts} (58%) diff --git a/tests/legacy-cli/e2e/tests/ivy/ivy-lazy-load.ts b/tests/legacy-cli/e2e/tests/build/lazy-load-syntax.ts similarity index 58% rename from tests/legacy-cli/e2e/tests/ivy/ivy-lazy-load.ts rename to tests/legacy-cli/e2e/tests/build/lazy-load-syntax.ts index fdd2485fad72..f453dfad9cf9 100644 --- a/tests/legacy-cli/e2e/tests/ivy/ivy-lazy-load.ts +++ b/tests/legacy-cli/e2e/tests/build/lazy-load-syntax.ts @@ -5,17 +5,36 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ -import { readFile, replaceInFile, writeFile } from '../../utils/fs'; +import { getGlobalVariable } from '../../utils/env'; +import { appendToFile, prependToFile, readFile, replaceInFile, writeFile } from '../../utils/fs'; import { ng } from '../../utils/process'; -import { createProject, updateJsonFile } from '../../utils/project'; +import { updateJsonFile } from '../../utils/project'; import { expectToFail } from '../../utils/utils'; export default async function () { - const projectName = 'ivy-lazy-loading'; + const argv = getGlobalVariable('argv'); + const ivyProject = argv['ivy']; + const projectName = 'test-project'; const appRoutingModulePath = 'src/app/app-routing.module.ts'; - // Make Ivy project. - await createProject(projectName, '--enable-ivy', '--routing'); + // Add app routing. + // This is done automatically on a new app with --routing. + await writeFile(appRoutingModulePath, ` + import { NgModule } from '@angular/core'; + import { Routes, RouterModule } from '@angular/router'; + + const routes: Routes = []; + + @NgModule({ + imports: [RouterModule.forRoot(routes)], + exports: [RouterModule] + }) + export class AppRoutingModule { } + `); + await prependToFile('src/app/app.module.ts', + `import { AppRoutingModule } from './app-routing.module';`); + await replaceInFile('src/app/app.module.ts', `imports: [`, `imports: [ AppRoutingModule,`); + await appendToFile('src/app/app.component.html', ''); const originalAppRoutingModule = await readFile(appRoutingModulePath); // helper to replace loadChildren @@ -57,8 +76,10 @@ export default async function () { // Set factory shims to false. await updateJsonFile('tsconfig.app.json', json => { - const angularCompilerOptions = json['angularCompilerOptions']; - angularCompilerOptions['allowEmptyCodegenFiles'] = false; + if (json['angularCompilerOptions'] === undefined) { + json['angularCompilerOptions'] = {}; + } + json['angularCompilerOptions']['allowEmptyCodegenFiles'] = false; }); // Convert the default config to use JIT and prod to just do AOT. @@ -69,22 +90,42 @@ export default async function () { buildTarget['configurations']['production'] = { aot: true }; }); - // Test `import()` style lazy load. - await replaceLoadChildren(`() => import('./lazy/lazy.module').then(m => m.LazyModule)`); - await ng('e2e'); - await ng('e2e', '--prod'); - // Test string import with factory shims. await replaceLoadChildren(`'./lazy/lazy.module#LazyModule'`); await replaceInFile('tsconfig.app.json', `"allowEmptyCodegenFiles": false`, `"allowEmptyCodegenFiles": true`); - await expectToFail(() => ng('e2e')); // Currently broken. - await ng('e2e', '--prod'); + if (ivyProject) { + // Ivy should not support the string syntax. + await expectToFail(() => ng('e2e')); + await expectToFail(() => ng('e2e', '--prod')); + } else { + // View engine should support the string syntax. + await ng('e2e'); + await ng('e2e', '--prod'); + } // Test string import without factory shims. await replaceLoadChildren(`'./lazy/lazy.module#LazyModule'`); await replaceInFile('tsconfig.app.json', `"allowEmptyCodegenFiles": true`, - `"allowEmptyCodegenFiles": false`); - await expectToFail(() => ng('e2e')); // Not supported. - await expectToFail(() => ng('e2e', '--prod')); // Not supported. + `"allowEmptyCodegenFiles": false`); + if (ivyProject) { + // Ivy should not support the string syntax. + await expectToFail(() => ng('e2e')); + await expectToFail(() => ng('e2e', '--prod')); + } else { + // View engine should support the string syntax. + await ng('e2e'); + await ng('e2e', '--prod'); + } + + // Test `import()` style lazy load. + await updateJsonFile('angular.json', json => { + // Add the experimental flag to import factories in View Engine. + const buildTarget = json['projects'][projectName]['architect']['build']; + buildTarget['options']['experimentalImportFactories'] = true; + }); + // Both Ivy and View Engine should support it. + await replaceLoadChildren(`() => import('./lazy/lazy.module').then(m => m.LazyModule)`); + await ng('e2e'); + await ng('e2e', '--prod'); } From 6166c8d9b9c1e064b5446d108b6fa1372d3eb187 Mon Sep 17 00:00:00 2001 From: Filipe Silva Date: Tue, 26 Mar 2019 15:55:41 +0000 Subject: [PATCH 5/6] refactor(@ngtools/webpack): simplify a call using await --- .../webpack/src/angular_compiler_plugin.ts | 57 +++++++++---------- 1 file changed, 27 insertions(+), 30 deletions(-) diff --git a/packages/ngtools/webpack/src/angular_compiler_plugin.ts b/packages/ngtools/webpack/src/angular_compiler_plugin.ts index 5f079af2abad..b9393a9a7514 100644 --- a/packages/ngtools/webpack/src/angular_compiler_plugin.ts +++ b/packages/ngtools/webpack/src/angular_compiler_plugin.ts @@ -760,39 +760,36 @@ export class AngularCompilerPlugin { return result; } - return this.done.then( - () => { - // This folder does not exist, but we need to give webpack a resource. - // TODO: check if we can't just leave it as is (angularCoreModuleDir). - result.resource = path.join(this._basePath, '$$_lazy_route_resource'); - // tslint:disable-next-line:no-any - result.dependencies.forEach((d: any) => d.critical = false); - // tslint:disable-next-line:no-any - result.resolveDependencies = (_fs: any, options: any, callback: Callback) => { - const dependencies = Object.keys(this._lazyRoutes) - .map((key) => { - const modulePath = this._lazyRoutes[key]; - if (modulePath !== null) { - const name = key.split('#')[0]; - - return new this._contextElementDependencyConstructor(modulePath, name); - } else { - return null; - } - }) - .filter(x => !!x); - - if (this._options.nameLazyFiles) { - options.chunkName = '[request]'; + await this.done; + + // This folder does not exist, but we need to give webpack a resource. + // TODO: check if we can't just leave it as is (angularCoreModuleDir). + result.resource = path.join(this._basePath, '$$_lazy_route_resource'); + // tslint:disable-next-line:no-any + result.dependencies.forEach((d: any) => d.critical = false); + // tslint:disable-next-line:no-any + result.resolveDependencies = (_fs: any, options: any, callback: Callback) => { + const dependencies = Object.keys(this._lazyRoutes) + .map((key) => { + const modulePath = this._lazyRoutes[key]; + if (modulePath !== null) { + const name = key.split('#')[0]; + + return new this._contextElementDependencyConstructor(modulePath, name); + } else { + return null; } + }) + .filter(x => !!x); - callback(null, dependencies); - }; + if (this._options.nameLazyFiles) { + options.chunkName = '[request]'; + } - return result; - }, - () => undefined, - ); + callback(null, dependencies); + }; + + return result; }); }); } From c9b3071ec9e0592c96eabba9676877873cf07aa0 Mon Sep 17 00:00:00 2001 From: Filipe Silva Date: Thu, 28 Mar 2019 15:00:01 +0000 Subject: [PATCH 6/6] ci: add ivy track for all prs --- .circleci/config.yml | 21 +++++--------- tests/legacy-cli/e2e/tests/basic/rebuild.ts | 10 +++++-- .../legacy-cli/e2e/tests/misc/lazy-module.ts | 29 ++++++++++++++----- tests/legacy-cli/e2e_runner.ts | 2 ++ 4 files changed, 37 insertions(+), 25 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 2b9a7e00fb32..54362128dbc1 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -106,24 +106,17 @@ jobs: path: /tmp/dist destination: cli/new-production - e2e-cli-ng-snapshots: + e2e-cli-ivy: <<: *defaults environment: BASH_ENV: ~/.profile resource_class: xlarge parallelism: 4 steps: - - run: - name: Don't run expensive e2e tests for forks other than renovate-bot and angular - command: > - if [[ "$CIRCLE_PR_USERNAME" != "renovate-bot" ]] && - [[ "$CIRCLE_PROJECT_USERNAME" != "angular" || $CIRCLE_BRANCH != "master" ]]; then - circleci step halt - fi - attach_workspace: *attach_options - - run: xvfb-run -a node ./tests/legacy-cli/run_e2e --nb-shards=${CIRCLE_NODE_TOTAL} --shard=${CIRCLE_NODE_INDEX} --ng-snapshots + - run: xvfb-run -a node ./tests/legacy-cli/run_e2e --nb-shards=${CIRCLE_NODE_TOTAL} --shard=${CIRCLE_NODE_INDEX} --ivy - e2e-cli-ivy-snapshots: + e2e-cli-ng-snapshots: <<: *defaults environment: BASH_ENV: ~/.profile @@ -138,7 +131,7 @@ jobs: circleci step halt fi - attach_workspace: *attach_options - - run: xvfb-run -a node ./tests/legacy-cli/run_e2e --nb-shards=${CIRCLE_NODE_TOTAL} --shard=${CIRCLE_NODE_INDEX} --ng-snapshots --ivy + - run: xvfb-run -a node ./tests/legacy-cli/run_e2e --nb-shards=${CIRCLE_NODE_TOTAL} --shard=${CIRCLE_NODE_INDEX} --ng-snapshots build: <<: *defaults @@ -210,6 +203,9 @@ workflows: - e2e-cli: requires: - build + - e2e-cli-ivy: + requires: + - build - snapshot_publish_docs: requires: - install @@ -220,9 +216,6 @@ workflows: - e2e-cli-ng-snapshots: requires: - build - - e2e-cli-ivy-snapshots: - requires: - - build - snapshot_publish: requires: - test diff --git a/tests/legacy-cli/e2e/tests/basic/rebuild.ts b/tests/legacy-cli/e2e/tests/basic/rebuild.ts index 3ca37f3bc175..4c1dccf36735 100644 --- a/tests/legacy-cli/e2e/tests/basic/rebuild.ts +++ b/tests/legacy-cli/e2e/tests/basic/rebuild.ts @@ -7,6 +7,7 @@ import { import {writeFile, writeMultipleFiles} from '../../utils/fs'; import {wait} from '../../utils/utils'; import {request} from '../../utils/http'; +import {getGlobalVariable} from '../../utils/env'; const validBundleRegEx = /: Compiled successfully./; @@ -15,7 +16,10 @@ export default function() { return Promise.resolve(); } - const lazyChunkRegExp = /lazy-module\.js/g; + const argv = getGlobalVariable('argv'); + const ivyProject = argv['ivy']; + const lazyImport = ivyProject ? `() => import('./lazy/lazy.module').then(m => m.LazyModule)` + : `'./lazy/lazy.module#LazyModule'`; return execAndWaitForOutputToMatch('ng', ['serve'], validBundleRegEx) // Add a lazy module. @@ -43,7 +47,7 @@ export default function() { FormsModule, HttpClientModule, RouterModule.forRoot([ - { path: 'lazy', loadChildren: './lazy/lazy.module#LazyModule' } + { path: 'lazy', loadChildren: ${lazyImport} } ]) ], providers: [], @@ -55,7 +59,7 @@ export default function() { // Count the bundles. .then((results) => { const stdout = results[0].stdout; - if (!lazyChunkRegExp.test(stdout)) { + if (!/(lazy-module|0)\.js/g.test(stdout)) { throw new Error('Expected webpack to create a new chunk, but did not.'); } }) diff --git a/tests/legacy-cli/e2e/tests/misc/lazy-module.ts b/tests/legacy-cli/e2e/tests/misc/lazy-module.ts index cf73ee7b06e2..65183f7d6746 100644 --- a/tests/legacy-cli/e2e/tests/misc/lazy-module.ts +++ b/tests/legacy-cli/e2e/tests/misc/lazy-module.ts @@ -2,9 +2,19 @@ import {readdirSync} from 'fs'; import {ng, silentNpm} from '../../utils/process'; import {appendToFile, writeFile, prependToFile, replaceInFile} from '../../utils/fs'; +import {getGlobalVariable} from '../../utils/env'; export default function() { + const argv = getGlobalVariable('argv'); + const ivyProject = argv['ivy']; + const lazyImport = ivyProject ? `() => import('src/app/lazy/lazy.module').then(m => m.LazyModule)` + : `'src/app/lazy/lazy.module#LazyModule'`; + const lazyImport1 = ivyProject ? `() => import('./lazy/lazy.module').then(m => m.LazyModule)` + : `'./lazy/lazy.module#LazyModule'`; + const lazyImport2 = ivyProject ? `() => import('./too/lazy/lazy.module').then(m => m.LazyModule)` + : `'./too/lazy/lazy.module#LazyModule'`; + let oldNumberOfFiles = 0; return Promise.resolve() .then(() => ng('build')) @@ -15,9 +25,9 @@ export default function() { import { RouterModule } from '@angular/router'; `)) .then(() => replaceInFile('src/app/app.module.ts', 'imports: [', `imports: [ - RouterModule.forRoot([{ path: "lazy", loadChildren: "src/app/lazy/lazy.module#LazyModule" }]), - RouterModule.forRoot([{ path: "lazy1", loadChildren: "./lazy/lazy.module#LazyModule" }]), - RouterModule.forRoot([{ path: "lazy2", loadChildren: "./too/lazy/lazy.module#LazyModule" }]), + RouterModule.forRoot([{ path: "lazy", loadChildren: ${lazyImport} }]), + RouterModule.forRoot([{ path: "lazy1", loadChildren: ${lazyImport1} }]), + RouterModule.forRoot([{ path: "lazy2", loadChildren: ${lazyImport2} }]), `)) .then(() => ng('build', '--named-chunks')) .then(() => readdirSync('dist/test-project')) @@ -28,11 +38,14 @@ export default function() { } oldNumberOfFiles = currentNumberOfDistFiles; - if (!distFiles.includes('lazy-lazy-module.js')) { - throw new Error('The lazy module chunk did not have a name.'); - } - if (!distFiles.includes('too-lazy-lazy-module.js')) { - throw new Error('The lazy module chunk did not use a unique name.'); + // Named lazy route chunks are not available with Ivy. + if (!ivyProject) { + if (!distFiles.includes('lazy-lazy-module.js')) { + throw new Error('The lazy module chunk did not have a name.'); + } + if (!distFiles.includes('too-lazy-lazy-module.js')) { + throw new Error('The lazy module chunk did not use a unique name.'); + } } }) // verify System.import still works diff --git a/tests/legacy-cli/e2e_runner.ts b/tests/legacy-cli/e2e_runner.ts index 171aeb44492a..cf2f3904455d 100644 --- a/tests/legacy-cli/e2e_runner.ts +++ b/tests/legacy-cli/e2e_runner.ts @@ -144,6 +144,8 @@ if (argv.ivy) { .filter(name => !name.endsWith('tests/build/aot/aot-i18n.ts')) // We don't have a library consumption story yet for Ivy. .filter(name => !name.endsWith('tests/generate/library/library-consumption.ts')) + // The additional lazy modules array does not work with Ivy because it's not needed. + .filter(name => !name.endsWith('tests/build/dynamic-import.ts')) // We don't have a platform-server usage story yet for Ivy. // It's contingent on lazy loading and factory shim considerations that are still being // discussed.