feat(@angular-devkit/build-angular): show warnings when depending on CommonJS.

Depending on CommonJS modules is know to cause optimization bailouts. With this change when running a browser build and scripts optimization is enabled we display a warning.

To suppress the warning for a particular package, users can use the `allowedCommonJsDepedencies` builder options.

Example:
```
"build": {
  "builder": "@angular-devkit/build-angular:browser",
  "options": {
    ...
    "allowedCommonJsDepedencies": ["bootstrap"]
  },
}
```

Reference: TOOL-1328

Author: alan-agius4

packages/angular/cli/lib/config/schema.json

@@ -974,6 +974,13 @@
                 "anonymous",
                 "use-credentials"
               ]
+            },
+            "allowedCommonJsDependencies": {
+              "description": "A list of CommonJS packages that are allowed to be used without a built time warning.",
+              "type": "array",
+              "items": {
+                "type": "string"
+              }
             }
           },
           "additionalProperties": false,

packages/angular_devkit/build_angular/src/angular-cli-files/models/build-options.ts

@@ -87,8 +87,8 @@ export interface BuildOptions {
 
   /* Append script target version to filename. */
   esVersionInFileName?: boolean;
-
   experimentalRollupPass?: boolean;
+  allowedCommonJsDependencies?: string[];
 }
 
 export interface WebpackTestOptions extends BuildOptions {

packages/angular_devkit/build_angular/src/angular-cli-files/models/webpack-configs/browser.ts

@@ -7,6 +7,7 @@
  */
 import { LicenseWebpackPlugin } from 'license-webpack-plugin';
 import * as webpack from 'webpack';
+import { CommonJsUsageWarnPlugin } from '../../plugins/webpack';
 import { WebpackConfigOptions } from '../build-options';
 import { getSourceMapDevTool, isPolyfillsEntry, normalizeExtraEntryPoints } from './utils';
 
@@ -23,12 +24,14 @@ export function getBrowserConfig(wco: WebpackConfigOptions): webpack.Configurati
     vendorChunk,
     commonChunk,
     styles,
+    allowedCommonJsDependencies,
+    optimization,
   } = buildOptions;
 
   const extraPlugins = [];
 
   let isEval = false;
-  const { styles: stylesOptimization, scripts: scriptsOptimization } = buildOptions.optimization;
+  const { styles: stylesOptimization, scripts: scriptsOptimization } = optimization;
   const {
     styles: stylesSourceMap,
     scripts: scriptsSourceMap,
@@ -123,7 +126,12 @@ export function getBrowserConfig(wco: WebpackConfigOptions): webpack.Configurati
         },
       },
     },
-    plugins: extraPlugins,
+    plugins: [
+      new CommonJsUsageWarnPlugin({
+        allowedDepedencies: allowedCommonJsDependencies,
+      }),
+      ...extraPlugins,
+    ],
     node: false,
   };
 }

packages/angular_devkit/build_angular/src/angular-cli-files/models/webpack-configs/common.ts

@@ -36,11 +36,13 @@ import {
   cachingDisabled,
   shouldBeautify,
 } from '../../../utils/environment-options';
-import { BundleBudgetPlugin } from '../../plugins/bundle-budget';
-import { NamedLazyChunksPlugin } from '../../plugins/named-chunks-plugin';
-import { OptimizeCssWebpackPlugin } from '../../plugins/optimize-css-webpack-plugin';
-import { ScriptsWebpackPlugin } from '../../plugins/scripts-webpack-plugin';
-import { WebpackRollupLoader } from '../../plugins/webpack';
+import {
+  BundleBudgetPlugin,
+  NamedLazyChunksPlugin,
+  OptimizeCssWebpackPlugin,
+  ScriptsWebpackPlugin,
+  WebpackRollupLoader,
+} from '../../plugins/webpack';
 import { findAllNodeModules } from '../../utilities/find-up';
 import { WebpackConfigOptions } from '../build-options';
 import { getEsVersionForFileName, getOutputHashFormat, normalizeExtraEntryPoints } from './utils';
@@ -149,7 +151,7 @@ export function getCommonConfig(wco: WebpackConfigOptions): Configuration {
                 // tslint:disable-next-line: no-any
                 (compilation.mainTemplate.hooks as any).assetPath.tap(
                   'build-angular',
-                (filename: string | ((data: ChunkData) => string), data: ChunkData) => {
+                  (filename: string | ((data: ChunkData) => string), data: ChunkData) => {
                     const assetName = typeof filename === 'function' ? filename(data) : filename;
                     const isMap = assetName && assetName.endsWith('.map');
 
@@ -313,6 +315,12 @@ export function getCommonConfig(wco: WebpackConfigOptions): Configuration {
     extraPlugins.push(new NamedLazyChunksPlugin());
   }
 
+  if (!differentialLoadingMode) {
+    // Budgets are computed after differential builds, not via a plugin.
+    // https://github.com/angular/angular-cli/blob/master/packages/angular_devkit/build_angular/src/browser/index.ts
+    extraPlugins.push(new BundleBudgetPlugin({ budgets: buildOptions.budgets }));
+  }
+
   let sourceMapUseRule;
   if ((scriptsSourceMap || stylesSourceMap) && vendorSourceMap) {
     sourceMapUseRule = {
@@ -411,18 +419,18 @@ export function getCommonConfig(wco: WebpackConfigOptions): Configuration {
         allowMinify &&
         (buildOptions.platform == 'server'
           ? {
-              ecma: terserEcma,
-              global_defs: angularGlobalDefinitions,
-              keep_fnames: true,
-            }
+            ecma: terserEcma,
+            global_defs: angularGlobalDefinitions,
+            keep_fnames: true,
+          }
           : {
-              ecma: terserEcma,
-              pure_getters: buildOptions.buildOptimizer,
-              // PURE comments work best with 3 passes.
-              // See https://github.com/webpack/webpack/issues/2899#issuecomment-317425926.
-              passes: buildOptions.buildOptimizer ? 3 : 1,
-              global_defs: angularGlobalDefinitions,
-            }),
+            ecma: terserEcma,
+            pure_getters: buildOptions.buildOptimizer,
+            // PURE comments work best with 3 passes.
+            // See https://github.com/webpack/webpack/issues/2899#issuecomment-317425926.
+            passes: buildOptions.buildOptimizer ? 3 : 1,
+            global_defs: angularGlobalDefinitions,
+          }),
       // We also want to avoid mangling on server.
       // Name mangling is handled within the browser builder
       mangle: allowMangle && buildOptions.platform !== 'server' && !differentialLoadingMode,
@@ -543,13 +551,7 @@ export function getCommonConfig(wco: WebpackConfigOptions): Configuration {
       minimizer: [
         new HashedModuleIdsPlugin(),
         ...extraMinimizers,
-      ].concat(differentialLoadingMode ? [
-        // Budgets are computed after differential builds, not via a plugin.
-        // https://github.com/angular/angular-cli/blob/master/packages/angular_devkit/build_angular/src/browser/index.ts
-      ] : [
-        // Non differential builds should be computed here, as a plugin.
-        new BundleBudgetPlugin({ budgets: buildOptions.budgets }),
-      ]),
+      ],
     },
     plugins: [
       // Always replace the context for the System.import in angular/core to prevent warnings.

packages/angular_devkit/build_angular/src/angular-cli-files/plugins/common-js-usage-warn-plugin.ts

@@ -0,0 +1,91 @@
+
+/**
+ * @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 { isAbsolute } from 'path';
+import { Compiler, compilation } from 'webpack';
+
+// Webpack doesn't export these so the deep imports can potentially break.
+const CommonJsRequireDependency = require('webpack/lib/dependencies/CommonJsRequireDependency');
+const AMDDefineDependency = require('webpack/lib/dependencies/AMDDefineDependency');
+
+// The below is extended because there are not in the typings
+interface WebpackModule extends compilation.Module {
+  name?: string;
+  rawRequest?: string;
+  dependencies: unknown[];
+  issuer: WebpackModule | null;
+  userRequest?: string;
+}
+
+export interface CommonJsUsageWarnPluginOptions {
+  /** A list of CommonJS packages that are allowed to be used without a warning. */
+  allowedDepedencies?: string[];
+}
+
+export class CommonJsUsageWarnPlugin {
+  private shownWarnings = new Set<string>();
+
+  constructor(private options: CommonJsUsageWarnPluginOptions = {}) {
+
+  }
+
+  apply(compiler: Compiler) {
+    compiler.hooks.compilation.tap('CommonJsUsageWarnPlugin', compilation => {
+      compilation.hooks.finishModules.tap('CommonJsUsageWarnPlugin', modules => {
+        for (const { dependencies, rawRequest, issuer } of modules as unknown as WebpackModule[]) {
+          if (
+            !rawRequest ||
+            rawRequest.startsWith('.') ||
+            isAbsolute(rawRequest)
+          ) {
+            // Skip if module is absolute or relative.
+            continue;
+          }
+
+          if (this.options.allowedDepedencies?.includes(rawRequest)) {
+            // Skip as this module is allowed even if it's a CommonJS.
+            continue;
+          }
+
+          if (this.hasCommonJsDependencies(dependencies)) {
+            // Dependency is CommonsJS or AMD.
+
+            // Check if it's parent issuer is also a CommonJS dependency.
+            // In case it is skip as an warning will be show for the parent CommonJS dependency.
+            if (this.hasCommonJsDependencies(issuer?.issuer?.dependencies ?? [])) {
+              continue;
+            }
+
+            // Find the main issuer (entry-point).
+            let mainIssuer = issuer;
+            while (mainIssuer?.issuer) {
+              mainIssuer = mainIssuer.issuer;
+            }
+
+            // Only show warnings for modules from main entrypoint.
+            if (mainIssuer?.name === 'main') {
+              const warning = `${issuer?.userRequest} depends on ${rawRequest}. CommonJS or AMD dependencies can cause optimization bailouts.`;
+
+              // Avoid showing the same warning multiple times when in 'watch' mode.
+              if (!this.shownWarnings.has(warning)) {
+                compilation.warnings.push(warning);
+                this.shownWarnings.add(warning);
+              }
+            }
+          }
+        }
+      });
+    });
+  }
+
+  private hasCommonJsDependencies(dependencies: unknown[]): boolean {
+    return dependencies.some(d => d instanceof CommonJsRequireDependency || d instanceof AMDDefineDependency);
+  }
+
+}

packages/angular_devkit/build_angular/src/angular-cli-files/plugins/webpack.ts

@@ -13,7 +13,8 @@ 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 { NamedLazyChunksPlugin } from './named-chunks-plugin';
+export { CommonJsUsageWarnPlugin } from './common-js-usage-warn-plugin';
 export {
   default as PostcssCliResources,
   PostcssCliResourcesOptions,

packages/angular_devkit/build_angular/src/browser/schema.json

@@ -379,6 +379,14 @@
       "type": "boolean",
       "description": "Concatenate modules with Rollup before bundling them with Webpack.",
       "default": false
+    },
+    "allowedCommonJsDependencies": {
+      "description": "A list of CommonJS packages that are allowed to be used without a built time warning.",
+      "type": "array",
+      "items": {
+        "type": "string"
+      },
+      "default": []
     }
   },
   "additionalProperties": false,

packages/angular_devkit/build_angular/test/browser/common-js-warning_spec_large.ts

@@ -0,0 +1,57 @@
+/**
+ * @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 { Architect } from '@angular-devkit/architect';
+import { logging } from '@angular-devkit/core';
+import { createArchitect, host } from '../utils';
+
+describe('Browser Builder commonjs warning', () => {
+  const targetSpec = { project: 'app', target: 'build' };
+
+  let architect: Architect;
+  let logger: logging.Logger;
+  let logs: string[];
+
+  beforeEach(async () => {
+    await host.initialize().toPromise();
+    architect = (await createArchitect(host.root())).architect;
+
+    // Add a Common JS dependency
+    host.appendToFile('src/app/app.component.ts', `import 'bootstrap';`);
+
+    // Create logger
+    logger = new logging.Logger('');
+    logs = [];
+    logger.subscribe(e => logs.push(e.message));
+  });
+
+  afterEach(async () => host.restore().toPromise());
+
+  it('should show warning when depending on a Common JS bundle', async () => {
+    const run = await architect.scheduleTarget(targetSpec, undefined, { logger });
+    const output = await run.result;
+    expect(output.success).toBe(true);
+    const logMsg = logs.join();
+    expect(logMsg).toMatch(/WARNING in.+app\.component\.ts depends on bootstrap\. CommonJS or AMD dependencies/);
+    expect(logMsg).not.toContain('jquery', 'Should not warn on transitive CommonJS packages which parent is also CommonJS.');
+    await run.stop();
+  });
+
+  it('should not show warning when depending on a Common JS bundle which is allowed', async () => {
+    const overrides = {
+      allowedCommonJsDependencies: [
+        'bootstrap',
+      ],
+    };
+
+    const run = await architect.scheduleTarget(targetSpec, overrides, { logger });
+    const output = await run.result;
+    expect(output.success).toBe(true);
+    expect(logs.join()).not.toContain('WARNING');
+    await run.stop();
+  });
+});