fix(@angular-devkit/build-angular): used named chunks for dynamic imports

filipesilva · filipesilva · commit 9de1f3405e95 · 2019-05-02T14:19:17.000+01:00
Fix #14251
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
@@ -20,6 +20,7 @@ import { AssetPatternClass } from '../../../browser/schema';
 import { isEs5SupportNeeded } from '../../../utils/differential-loading';
 import { BundleBudgetPlugin } from '../../plugins/bundle-budget';
 import { CleanCssWebpackPlugin } from '../../plugins/cleancss-webpack-plugin';
+import { NamedLazyChunksPlugin } from '../../plugins/named-chunks-plugin';
 import { ScriptsWebpackPlugin } from '../../plugins/scripts-webpack-plugin';
 import { findAllNodeModules, findUp } from '../../utilities/find-up';
 import { requireProjectModule } from '../../utilities/require-project-module';
@@ -202,6 +203,10 @@ export function getCommonConfig(wco: WebpackConfigOptions): Configuration {
     extraPlugins.push(new StatsPlugin(`stats${targetInFileName}.json`, 'verbose'));
   }
 
+  if (buildOptions.namedChunks) {
+    extraPlugins.push(new NamedLazyChunksPlugin());
+  }
+
   let sourceMapUseRule;
   if ((scriptsSourceMap || stylesSourceMap) && vendorSourceMap) {
     sourceMapUseRule = {
diff --git a/packages/angular_devkit/build_angular/src/angular-cli-files/plugins/named-chunks-plugin.ts b/packages/angular_devkit/build_angular/src/angular-cli-files/plugins/named-chunks-plugin.ts
@@ -0,0 +1,39 @@
+/**
+ * @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 { Compiler } from 'webpack';
+// Webpack doesn't export these so the deep imports can potentially break.
+// There doesn't seem to exist any ergonomic way to alter chunk names for non-context lazy chunks
+// (https://github.com/webpack/webpack/issues/9075) so this is the best alternative for now.
+const ImportDependency = require('webpack/lib/dependencies/ImportDependency');
+const ImportDependenciesBlock = require('webpack/lib/dependencies/ImportDependenciesBlock');
+const Template = require('webpack/lib/Template');
+
+export class NamedLazyChunksPlugin {
+  constructor() { }
+  apply(compiler: Compiler): void {
+    compiler.hooks.compilation.tap('named-lazy-chunks-plugin', compilation => {
+      // The dependencyReference hook isn't in the webpack typings so we have to type it as any.
+      // tslint:disable-next-line: no-any
+      (compilation.hooks as any).dependencyReference.tap('named-lazy-chunks-plugin',
+        // tslint:disable-next-line: no-any
+        (_: any, dependency: any) => {
+          if (
+            // Check this dependency is from an `import()` statement.
+            dependency instanceof ImportDependency
+            && dependency.block instanceof ImportDependenciesBlock
+            // Don't rename chunks that already have a name.
+            && dependency.block.chunkName === null
+          ) {
+            // Convert the request to a valid chunk name using the same logic used
+            // in webpack/lib/ContextModule.js
+            dependency.block.chunkName = Template.toPath(dependency.request);
+          }
+        });
+    });
+  }
+}
diff --git a/packages/angular_devkit/build_angular/src/angular-cli-files/plugins/webpack.ts b/packages/angular_devkit/build_angular/src/angular-cli-files/plugins/webpack.ts
@@ -12,6 +12,7 @@ export { BundleBudgetPlugin, BundleBudgetPluginOptions } from './bundle-budget';
 export { ScriptsWebpackPlugin, ScriptsWebpackPluginOptions } from './scripts-webpack-plugin';
 export { SuppressExtractedTextChunksWebpackPlugin } from './suppress-entry-chunks-webpack-plugin';
 export { RemoveHashPlugin, RemoveHashPluginOptions } from './remove-hash-plugin';
+export { NamedLazyChunksPlugin as NamedChunksPlugin } from './named-chunks-plugin';
 export {
   default as PostcssCliResources,
   PostcssCliResourcesOptions,
diff --git a/packages/angular_devkit/build_angular/test/browser/lazy-module_spec_large.ts b/packages/angular_devkit/build_angular/test/browser/lazy-module_spec_large.ts
@@ -9,7 +9,10 @@
 import { Architect } from '@angular-devkit/architect';
 import { TestLogger } from '@angular-devkit/architect/testing';
 import { take, tap, timeout } from 'rxjs/operators';
-import { browserBuild, createArchitect, host, lazyModuleFiles, lazyModuleImport } from '../utils';
+import {
+  browserBuild, createArchitect, host, lazyModuleFiles,
+  lazyModuleFnImport, lazyModuleStringImport,
+} from '../utils';
 
 // tslint:disable-next-line:no-big-function
 describe('Browser Builder lazy modules', () => {
@@ -22,46 +25,60 @@ describe('Browser Builder lazy modules', () => {
   });
   afterEach(async () => host.restore().toPromise());
 
-  it('supports lazy bundle for lazy routes with JIT', async () => {
-    host.writeMultipleFiles(lazyModuleFiles);
-    host.writeMultipleFiles(lazyModuleImport);
-
-    const { files } = await browserBuild(architect, host, target);
-    expect('lazy-lazy-module.js' in files).toBe(true);
-  });
-
-  it('should show error when lazy route is invalid on watch mode AOT', async () => {
-    host.writeMultipleFiles(lazyModuleFiles);
-    host.writeMultipleFiles(lazyModuleImport);
-    host.replaceInFile(
-      'src/app/app.module.ts',
-      'lazy.module#LazyModule',
-      'invalid.module#LazyModule',
-    );
-
-    const logger = new TestLogger('rebuild-lazy-errors');
-    const overrides = { watch: true, aot: true };
-    const run = await architect.scheduleTarget(target, overrides, { logger });
-    await run.output.pipe(
-      timeout(15000),
-      tap((buildEvent) => expect(buildEvent.success).toBe(false)),
-      tap(() => {
-        expect(logger.includes('Could not resolve module')).toBe(true);
-        logger.clear();
-        host.appendToFile('src/main.ts', ' ');
-      }),
-      take(2),
-    ).toPromise();
-    await run.stop();
-  });
-
-  it('supports lazy bundle for lazy routes with AOT', async () => {
-    host.writeMultipleFiles(lazyModuleFiles);
-    host.writeMultipleFiles(lazyModuleImport);
-
-    const { files } = await browserBuild(architect, host, target, { aot: true });
-    expect(files['lazy-lazy-module-ngfactory.js']).not.toBeUndefined();
-  });
+  for (const [name, imports] of Object.entries({
+    'string': lazyModuleStringImport,
+    'function': lazyModuleFnImport,
+  })) {
+    describe(`Load children ${name} syntax`, () => {
+      it('supports lazy bundle for lazy routes with JIT', async () => {
+        host.writeMultipleFiles(lazyModuleFiles);
+        host.writeMultipleFiles(imports);
+
+        const { files } = await browserBuild(architect, host, target);
+        expect('lazy-lazy-module.js' in files).toBe(true);
+      });
+
+      it('should show error when lazy route is invalid on watch mode AOT', async () => {
+        host.writeMultipleFiles(lazyModuleFiles);
+        host.writeMultipleFiles(imports);
+        host.replaceInFile(
+          'src/app/app.module.ts',
+          'lazy.module',
+          'invalid.module',
+        );
+
+        const logger = new TestLogger('rebuild-lazy-errors');
+        const overrides = { watch: true, aot: true };
+        const run = await architect.scheduleTarget(target, overrides, { logger });
+        await run.output.pipe(
+          timeout(15000),
+          tap((buildEvent) => expect(buildEvent.success).toBe(false)),
+          tap(() => {
+            // Webpack error when using loadchildren string syntax.
+            const hasMissingModuleError = logger.includes('Could not resolve module')
+              // TS type error when using import().
+              || logger.includes('Cannot find module')
+              // Webpack error when using import() on a rebuild.
+              // There is no TS error because the type checker is forked on rebuilds.
+              || logger.includes('Module not found');
+            expect(hasMissingModuleError).toBe(true, 'Should show missing module error');
+            logger.clear();
+            host.appendToFile('src/main.ts', ' ');
+          }),
+          take(2),
+        ).toPromise();
+        await run.stop();
+      });
+
+      it('supports lazy bundle for lazy routes with AOT', async () => {
+        host.writeMultipleFiles(lazyModuleFiles);
+        host.writeMultipleFiles(imports);
+
+        const { files } = await browserBuild(architect, host, target, { aot: true });
+        expect(files['lazy-lazy-module-ngfactory.js']).not.toBeUndefined();
+      });
+    });
+  }
 
   it(`supports lazy bundle for import() calls`, async () => {
     host.writeMultipleFiles({
@@ -72,7 +89,7 @@ describe('Browser Builder lazy modules', () => {
     host.replaceInFile('src/tsconfig.app.json', `"module": "es2015"`, `"module": "esnext"`);
 
     const { files } = await browserBuild(architect, host, target);
-    expect(files['0.js']).not.toBeUndefined();
+    expect(files['lazy-module.js']).not.toBeUndefined();
   });
 
   it(`supports lazy bundle for dynamic import() calls`, async () => {
@@ -96,7 +113,7 @@ describe('Browser Builder lazy modules', () => {
     });
 
     const { files } = await browserBuild(architect, host, target);
-    expect(files['0.js']).not.toBeUndefined();
+    expect(files['lazy-module.js']).not.toBeUndefined();
   });
 
   it(`supports hiding lazy bundle module name`, async () => {
@@ -116,13 +133,12 @@ describe('Browser Builder lazy modules', () => {
       'src/two.ts': `import * as http from '@angular/common/http'; console.log(http);`,
       'src/main.ts': `import('./one'); import('./two');`,
     });
-    host.replaceInFile('src/tsconfig.app.json', `"module": "es2015"`, `"module": "esnext"`);
 
-    const { files } = await browserBuild(architect, host, target, { namedChunks: false });
-    expect(files['0.js']).not.toBeUndefined();
-    expect(files['1.js']).not.toBeUndefined();
+    const { files } = await browserBuild(architect, host, target);
+    expect(files['one.js']).not.toBeUndefined();
+    expect(files['two.js']).not.toBeUndefined();
     // TODO: the chunk with common modules used to be called `common`, see why that changed.
-    expect(files['2.js']).not.toBeUndefined();
+    expect(files['default~one~two.js']).not.toBeUndefined();
   });
 
   it(`supports disabling the common bundle`, async () => {
@@ -131,12 +147,11 @@ describe('Browser Builder lazy modules', () => {
       'src/two.ts': `import * as http from '@angular/common/http'; console.log(http);`,
       'src/main.ts': `import('./one'); import('./two');`,
     });
-    host.replaceInFile('src/tsconfig.app.json', `"module": "es2015"`, `"module": "esnext"`);
 
     const { files } = await browserBuild(architect, host, target, { commonChunk: false });
-    expect(files['0.js']).not.toBeUndefined();
-    expect(files['1.js']).not.toBeUndefined();
-    expect(files['2.js']).toBeUndefined();
+    expect(files['one.js']).not.toBeUndefined();
+    expect(files['two.js']).not.toBeUndefined();
+    expect(files['common.js']).toBeUndefined();
   });
 
   it(`supports extra lazy modules array in JIT`, async () => {
diff --git a/packages/angular_devkit/build_angular/test/browser/output-hashing_spec_large.ts b/packages/angular_devkit/build_angular/test/browser/output-hashing_spec_large.ts
@@ -8,7 +8,7 @@
 
 import { Architect } from '@angular-devkit/architect';
 import { normalize } from '@angular-devkit/core';
-import { browserBuild, createArchitect, host, lazyModuleFiles, lazyModuleImport } from '../utils';
+import { browserBuild, createArchitect, host, lazyModuleFiles, lazyModuleStringImport } from '../utils';
 
 describe('Browser Builder output hashing', () => {
   const target = { project: 'app', target: 'build' };
@@ -58,7 +58,7 @@ describe('Browser Builder output hashing', () => {
     let newHashes: Map<string, string>;
 
     host.writeMultipleFiles(lazyModuleFiles);
-    host.writeMultipleFiles(lazyModuleImport);
+    host.writeMultipleFiles(lazyModuleStringImport);
 
     const overrides = { outputHashing: 'all', extractCss: true };
 
@@ -69,7 +69,7 @@ describe('Browser Builder output hashing', () => {
     // Save the current hashes.
     oldHashes = generateFileHashMap();
     host.writeMultipleFiles(lazyModuleFiles);
-    host.writeMultipleFiles(lazyModuleImport);
+    host.writeMultipleFiles(lazyModuleStringImport);
 
     await browserBuild(architect, host, target, overrides);
     newHashes = generateFileHashMap();
@@ -107,7 +107,7 @@ describe('Browser Builder output hashing', () => {
   it('supports options', async () => {
     host.writeMultipleFiles({ 'src/styles.css': `h1 { background: url('./spectrum.png')}` });
     host.writeMultipleFiles(lazyModuleFiles);
-    host.writeMultipleFiles(lazyModuleImport);
+    host.writeMultipleFiles(lazyModuleStringImport);
 
     // We must do several builds instead of a single one in watch mode, so that the output
     // path is deleted on each run and only contains the most recent files.
diff --git a/packages/angular_devkit/build_angular/test/browser/rebuild_spec_large.ts b/packages/angular_devkit/build_angular/test/browser/rebuild_spec_large.ts
@@ -11,7 +11,7 @@ import { Architect } from '@angular-devkit/architect';
 import { TestLogger } from '@angular-devkit/architect/testing';
 import { normalize, virtualFs } from '@angular-devkit/core';
 import { debounceTime, take, takeWhile, tap } from 'rxjs/operators';
-import { createArchitect, host, lazyModuleFiles, lazyModuleImport } from '../utils';
+import { createArchitect, host, lazyModuleFiles, lazyModuleStringImport } from '../utils';
 
 
 describe('Browser Builder rebuilds', () => {
@@ -82,7 +82,7 @@ describe('Browser Builder rebuilds', () => {
             // No lazy chunk should exist.
             if (!hasLazyChunk) {
               phase = 2;
-              host.writeMultipleFiles({ ...lazyModuleFiles, ...lazyModuleImport });
+              host.writeMultipleFiles({ ...lazyModuleFiles, ...lazyModuleStringImport });
             }
             break;
 
diff --git a/packages/angular_devkit/build_angular/test/utils.ts b/packages/angular_devkit/build_angular/test/utils.ts
@@ -140,7 +140,7 @@ export const lazyModuleFiles: { [path: string]: string } = {
   `,
 };
 
-export const lazyModuleImport: { [path: string]: string } = {
+export const lazyModuleStringImport: { [path: string]: string } = {
   'src/app/app.module.ts': `
     import { BrowserModule } from '@angular/platform-browser';
     import { NgModule } from '@angular/core';
@@ -164,3 +164,10 @@ export const lazyModuleImport: { [path: string]: string } = {
     export class AppModule { }
   `,
 };
+
+export const lazyModuleFnImport: { [path: string]: string } = {
+  'src/app/app.module.ts': lazyModuleStringImport['src/app/app.module.ts'].replace(
+    `loadChildren: './lazy/lazy.module#LazyModule'`,
+    `loadChildren: () => import('./lazy/lazy.module').then(m => m.LazyModule)`,
+  ),
+};
