refactor(@angular-devkit/build-angular): cleanup Webpack rule generation

This change reduces the number of variables needed as well as reduces type casting.
angular · Aug 4, 2020 · 5d2865a · 5d2865a
1 parent 520459e
commit 5d2865a
Show file tree

Hide file tree

Showing 5 changed files with 46 additions and 66 deletions.
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
@@ -327,15 +327,13 @@ export function getCommonConfig(wco: WebpackConfigOptions): Configuration {
     extraPlugins.push(new BundleBudgetPlugin({ budgets: buildOptions.budgets }));
   }
 
-  let sourceMapUseRule;
   if ((scriptsSourceMap || stylesSourceMap) && vendorSourceMap) {
-    sourceMapUseRule = {
-      use: [
-        {
-          loader: require.resolve('source-map-loader'),
-        },
-      ],
-    };
+    extraRules.push({
+      test: /\.m?js$/,
+      exclude: /(ngfactory|ngstyle)\.js$/,
+      enforce: 'pre',
+      loader: require.resolve('source-map-loader'),
+    });
   }
 
   let buildOptimizerUseRule: RuleSetLoader[] = [];
@@ -583,12 +581,6 @@ export function getCommonConfig(wco: WebpackConfigOptions): Configuration {
             ...buildOptimizerUseRule,
           ],
         },
-        {
-          test: /\.m?js$/,
-          exclude: /(ngfactory|ngstyle)\.js$/,
-          enforce: 'pre',
-          ...sourceMapUseRule,
-        },
         ...extraRules,
       ],
     },

diff --git a/packages/angular_devkit/build_angular/src/angular-cli-files/models/webpack-configs/styles.ts b/packages/angular_devkit/build_angular/src/angular-cli-files/models/webpack-configs/styles.ts
@@ -121,7 +121,7 @@ export function getStylesConfig(wco: WebpackConfigOptions) {
   }
 
   // set base rules to derive final rules from
-  const baseRules: webpack.RuleSetRule[] = [
+  const baseRules: { test: RegExp, use: webpack.RuleSetLoader[] }[] = [
     { test: /\.css$/, use: [] },
     {
       test: /\.scss$|\.sass$/,
@@ -206,7 +206,7 @@ export function getStylesConfig(wco: WebpackConfigOptions) {
             && !buildOptions.sourceMap.hidden ? 'inline' : false,
         },
       },
-      ...(use as webpack.Loader[]),
+      ...use,
     ],
   }));
 
@@ -237,7 +237,7 @@ export function getStylesConfig(wco: WebpackConfigOptions) {
                     : cssSourceMap,
               },
             },
-            ...(use as webpack.Loader[]),
+            ...use,
           ],
         };
       }),

diff --git a/packages/angular_devkit/build_angular/src/angular-cli-files/models/webpack-configs/test.ts b/packages/angular_devkit/build_angular/src/angular-cli-files/models/webpack-configs/test.ts
@@ -15,25 +15,27 @@ import { getSourceMapDevTool, isPolyfillsEntry } from './utils';
 export function getTestConfig(
   wco: WebpackConfigOptions<WebpackTestOptions>,
 ): webpack.Configuration {
-  const { root, buildOptions, sourceRoot: include } = wco;
+  const {
+    buildOptions: { codeCoverage, codeCoverageExclude, main, sourceMap },
+    root,
+    sourceRoot,
+  } = wco;
 
   const extraRules: webpack.RuleSetRule[] = [];
   const extraPlugins: { apply(compiler: webpack.Compiler): void }[] = [];
 
-  if (buildOptions.codeCoverage) {
-    const codeCoverageExclude = buildOptions.codeCoverageExclude;
+  if (codeCoverage) {
     const exclude: (string | RegExp)[] = [
       /\.(e2e|spec)\.tsx?$/,
       /node_modules/,
     ];
 
     if (codeCoverageExclude) {
-      codeCoverageExclude.forEach((excludeGlob: string) => {
-        const excludeFiles = glob
+      for (const excludeGlob of codeCoverageExclude) {
+        glob
           .sync(path.join(root, excludeGlob), { nodir: true })
-          .map(file => path.normalize(file));
-        exclude.push(...excludeFiles);
-      });
+          .forEach((file) => exclude.push(path.normalize(file)));
+      }
     }
 
     extraRules.push({
@@ -42,31 +44,27 @@ export function getTestConfig(
       options: { esModules: true },
       enforce: 'post',
       exclude,
-      include,
+      include: sourceRoot,
     });
   }
 
-  if (wco.buildOptions.sourceMap) {
-    const { styles, scripts } = wco.buildOptions.sourceMap;
-
-    if (styles || scripts) {
-      extraPlugins.push(getSourceMapDevTool(
-        scripts,
-        styles,
-        false,
-        true,
-      ));
-    }
+  if (sourceMap.scripts || sourceMap.styles) {
+    extraPlugins.push(getSourceMapDevTool(
+      sourceMap.scripts,
+      sourceMap.styles,
+      false,
+      true,
+    ));
   }
 
   return {
     mode: 'development',
     resolve: {
       mainFields: ['es2015', 'browser', 'module', 'main'],
     },
-    devtool: buildOptions.sourceMap ? false : 'eval',
+    devtool: false,
     entry: {
-      main: path.resolve(root, buildOptions.main),
+      main: path.resolve(root, main),
     },
     module: {
       rules: extraRules,

diff --git a/packages/angular_devkit/build_angular/src/dev-server/index.ts b/packages/angular_devkit/build_angular/src/dev-server/index.ts
@@ -306,17 +306,7 @@ async function setupLocalize(
     }
   }
 
-  // Get the insertion point for the i18n babel loader rule
-  // This is currently dependent on the rule order/construction in common.ts
-  // A future refactor of the webpack configuration definition will improve this situation
-  // tslint:disable-next-line: no-non-null-assertion
-  const rules = webpackConfig.module!.rules;
-  const index = rules.findIndex(r => r.enforce === 'pre');
-  if (index === -1) {
-    throw new Error('Invalid internal webpack configuration');
-  }
-
-  const i18nRule: webpack.Rule = {
+  const i18nRule: webpack.RuleSetRule = {
     test: /\.(?:m?js|ts)$/,
     enforce: 'post',
     use: [
@@ -334,15 +324,17 @@ async function setupLocalize(
             translationIntegrity: localeDescription && localeDescription.integrity,
           }),
           plugins,
-          parserOpts: {
-            plugins: ['dynamicImport'],
-          },
         },
       },
     ],
   };
 
-  rules.splice(index, 0, i18nRule);
+  if (!webpackConfig.module) {
+    webpackConfig.module = { rules: [] };
+  } else if (!webpackConfig.module.rules) {
+    webpackConfig.module.rules = [];
+  }
+  webpackConfig.module.rules.push(i18nRule);
 
   // Add a plugin to inject the i18n diagnostics
   // tslint:disable-next-line: no-non-null-assertion

diff --git a/packages/angular_devkit/build_angular/src/karma/index.ts b/packages/angular_devkit/build_angular/src/karma/index.ts
@@ -18,10 +18,7 @@ import {
   getTestConfig,
   getWorkerConfig,
 } from '../angular-cli-files/models/webpack-configs';
-import {
-  SingleTestTransformLoader,
-  SingleTestTransformLoaderOptions,
-} from '../angular-cli-files/plugins/single-test-transform';
+import { SingleTestTransformLoader } from '../angular-cli-files/plugins/single-test-transform';
 import { findTests } from '../angular-cli-files/utilities/find-tests';
 import { Schema as BrowserBuilderOptions } from '../browser/schema';
 import { ExecutionTransformer } from '../transforms';
@@ -104,12 +101,7 @@ export function execute(
           }
 
           // prepend special webpack loader that will transform test.ts
-          if (
-            webpackConfig &&
-            webpackConfig.module &&
-            options.include &&
-            options.include.length > 0
-          ) {
+          if (options.include && options.include.length > 0) {
             const mainFilePath = getSystemPath(
               join(normalize(context.workspaceRoot), options.main),
             );
@@ -123,15 +115,21 @@ export function execute(
               return;
             }
 
+            if (!webpackConfig.module) {
+              webpackConfig.module = { rules: [] };
+            } else if (!webpackConfig.module.rules) {
+              webpackConfig.module.rules = [];
+            }
+
             webpackConfig.module.rules.unshift({
-              test: path => path === mainFilePath,
+              test: mainFilePath,
               use: {
                 // cannot be a simple path as it differs between environments
                 loader: SingleTestTransformLoader,
                 options: {
                   files,
                   logger: context.logger,
-                } as SingleTestTransformLoaderOptions,
+                },
               },
             });
           }