refactor(@angular-devkit/build-angular): ability to perform DCE but retain symbol names

Previously, we enabled the `keepNames` esbuild when mangling was disabled, this caused dead code to be retained because of the transformations that esbuild did to the input.

Input
```js
class foo {}
```

Output
```js
var l = Object.defineProperty,
  a = (s, c) => l(s, "name", { value: c, configurable: !0 });
class foo {}
a(foo, "foo");
```

Previously we enabled the `keepNames` esbuild option when mangling was disabled, which is actually not needed to retain the name of symbols but is needed for SSR because Domino relies on the `name` property on functions and classes.

Closes #22354

(cherry picked from commit 2c9a33d)

Author: alan-agius4

packages/angular_devkit/build_angular/src/webpack/configs/common.ts

@@ -277,7 +277,8 @@ export async function getCommonConfig(wco: WebpackConfigOptions): Promise<Config
         define: buildOptions.aot ? GLOBAL_DEFS_FOR_TERSER_WITH_AOT : GLOBAL_DEFS_FOR_TERSER,
         sourcemap: scriptsSourceMap,
         target: scriptTarget,
-        keepNames: !allowMangle || isPlatformServer,
+        keepIdentifierNames: !allowMangle || isPlatformServer,
+        keepNames: isPlatformServer,
         removeLicenses: buildOptions.extractLicenses,
         advanced: buildOptions.buildOptimizer,
       }),

packages/angular_devkit/build_angular/src/webpack/plugins/javascript-optimizer-plugin.ts

@@ -11,6 +11,7 @@ import { ScriptTarget } from 'typescript';
 import type { Compiler, sources } from 'webpack';
 import { maxWorkers } from '../../utils/environment-options';
 import { EsbuildExecutor } from './esbuild-executor';
+import type { OptimizeRequestOptions } from './javascript-optimizer-worker';
 
 /**
  * The maximum number of Workers that will be created to execute optimize tasks.
@@ -28,10 +29,10 @@ const PLUGIN_NAME = 'angular-javascript-optimizer';
 export interface JavaScriptOptimizerOptions {
   /**
    * Enables advanced optimizations in the underlying JavaScript optimizers.
-   * This currently increases the `terser` passes to 3 and enables the `pure_getters`
+   * This currently increases the `terser` passes to 2 and enables the `pure_getters`
    * option for `terser`.
    */
-  advanced: boolean;
+  advanced?: boolean;
 
   /**
    * An object record of string keys that will be replaced with their respective values when found
@@ -44,7 +45,7 @@ export interface JavaScriptOptimizerOptions {
    * The output sourcemap will be a full sourcemap containing the merge of the input sourcemap and
    * all intermediate sourcemaps.
    */
-  sourcemap: boolean;
+  sourcemap?: boolean;
 
   /**
    * The ECMAScript version that should be used when generating output code.
@@ -56,13 +57,22 @@ export interface JavaScriptOptimizerOptions {
   /**
    * Enables the retention of identifier names and ensures that function and class names are
    * present in the output code.
+   *
+   * **Note**: in some cases symbols are still renamed to avoid collisions.
+   */
+  keepIdentifierNames: boolean;
+
+  /**
+   * Enables the retention of original name of classes and functions.
+   *
+   * **Note**: this causes increase of bundle size as it causes dead-code elimination to not work fully.
    */
   keepNames: boolean;
 
   /**
    * Enables the removal of all license comments from the output code.
    */
-  removeLicenses: boolean;
+  removeLicenses?: boolean;
 }
 
 /**
@@ -74,7 +84,7 @@ export interface JavaScriptOptimizerOptions {
  * optimizations not yet implemented by `esbuild`.
  */
 export class JavaScriptOptimizerPlugin {
-  constructor(public options: Partial<JavaScriptOptimizerOptions> = {}) {}
+  constructor(public options: JavaScriptOptimizerOptions) {}
 
   apply(compiler: Compiler) {
     const { OriginalSource, SourceMapSource } = compiler.webpack.sources;
@@ -142,22 +152,25 @@ export class JavaScriptOptimizerPlugin {
             }
           }
 
-          let target = 2017;
+          let target: OptimizeRequestOptions['target'] = 2017;
           if (this.options.target) {
             if (this.options.target <= ScriptTarget.ES5) {
               target = 5;
             } else if (this.options.target < ScriptTarget.ESNext) {
-              target = Number(ScriptTarget[this.options.target].slice(2));
+              target = Number(
+                ScriptTarget[this.options.target].slice(2),
+              ) as OptimizeRequestOptions['target'];
             } else {
               target = 2020;
             }
           }
 
           // Setup the options used by all worker tasks
-          const optimizeOptions = {
+          const optimizeOptions: OptimizeRequestOptions = {
             sourcemap: this.options.sourcemap,
             define,
             keepNames: this.options.keepNames,
+            keepIdentifierNames: this.options.keepIdentifierNames,
             target,
             removeLicenses: this.options.removeLicenses,
             advanced: this.options.advanced,

packages/angular_devkit/build_angular/src/webpack/plugins/javascript-optimizer-worker.ts

@@ -11,50 +11,60 @@ import type { TransformFailure, TransformResult } from 'esbuild';
 import { minify } from 'terser';
 import { EsbuildExecutor } from './esbuild-executor';
 
+/**
+ * The options to use when optimizing.
+ */
+export interface OptimizeRequestOptions {
+  /**
+   * Controls advanced optimizations.
+   * Currently these are only terser related:
+   * * terser compress passes are set to 2
+   * * terser pure_getters option is enabled
+   */
+  advanced?: boolean;
+  /**
+   * Specifies the string tokens that should be replaced with a defined value.
+   */
+  define?: Record<string, string>;
+  /**
+   * Controls whether class, function, and variable names should be left intact
+   * throughout the output code.
+   */
+  keepIdentifierNames: boolean;
+
+  /**
+   * Controls whether to retain the original name of classes and functions.
+   */
+  keepNames: boolean;
+  /**
+   * Controls whether license text is removed from the output code.
+   * Within the CLI, this option is linked to the license extraction functionality.
+   */
+  removeLicenses?: boolean;
+  /**
+   * Controls whether source maps should be generated.
+   */
+  sourcemap?: boolean;
+  /**
+   * Specifies the target ECMAScript version for the output code.
+   */
+  target: 5 | 2015 | 2016 | 2017 | 2018 | 2019 | 2020;
+  /**
+   * Controls whether esbuild should only use the WASM-variant instead of trying to
+   * use the native variant. Some platforms may not support the native-variant and
+   * this option allows one support test to be conducted prior to all the workers starting.
+   */
+  alwaysUseWasm: boolean;
+}
+
 /**
  * A request to optimize JavaScript using the supplied options.
  */
 interface OptimizeRequest {
   /**
    * The options to use when optimizing.
    */
-  options: {
-    /**
-     * Controls advanced optimizations.
-     * Currently these are only terser related:
-     * * terser compress passes are set to 2
-     * * terser pure_getters option is enabled
-     */
-    advanced: boolean;
-    /**
-     * Specifies the string tokens that should be replaced with a defined value.
-     */
-    define?: Record<string, string>;
-    /**
-     * Controls whether class, function, and variable names should be left intact
-     * throughout the output code.
-     */
-    keepNames: boolean;
-    /**
-     * Controls whether license text is removed from the output code.
-     * Within the CLI, this option is linked to the license extraction functionality.
-     */
-    removeLicenses: boolean;
-    /**
-     * Controls whether source maps should be generated.
-     */
-    sourcemap: boolean;
-    /**
-     * Specifies the target ECMAScript version for the output code.
-     */
-    target: 5 | 2015 | 2016 | 2017 | 2018 | 2019 | 2020;
-    /**
-     * Controls whether esbuild should only use the WASM-variant instead of trying to
-     * use the native variant. Some platforms may not support the native-variant and
-     * this option allows one support test to be conducted prior to all the workers starting.
-     */
-    alwaysUseWasm: boolean;
-  };
+  options: OptimizeRequestOptions;
 
   /**
    * The JavaScript asset to optimize.
@@ -142,7 +152,7 @@ async function optimizeWithEsbuild(
   let result: TransformResult;
   try {
     result = await esbuild.transform(content, {
-      minifyIdentifiers: !options.keepNames,
+      minifyIdentifiers: !options.keepIdentifierNames,
       minifySyntax: true,
       // NOTE: Disabling whitespace ensures unused pure annotations are kept
       minifyWhitespace: false,
@@ -151,6 +161,10 @@ async function optimizeWithEsbuild(
       sourcefile: name,
       sourcemap: options.sourcemap && 'external',
       define: options.define,
+      // This option should always be disabled for browser builds as we don't rely on `.name`
+      // and causes deadcode to be retained which makes `NG_BUILD_MANGLE` unusable to investigate tree-shaking issues.
+      // We enable `keepNames` only for server builds as Domino relies on `.name`.
+      // Once we no longer rely on Domino for SSR we should be able to remove this.
       keepNames: options.keepNames,
       target: `es${options.target}`,
     });
@@ -193,9 +207,9 @@ async function optimizeWithEsbuild(
 async function optimizeWithTerser(
   name: string,
   code: string,
-  sourcemaps: boolean,
+  sourcemaps: boolean | undefined,
   target: OptimizeRequest['options']['target'],
-  advanced: boolean,
+  advanced: boolean | undefined,
 ): Promise<{ code: string; map?: object }> {
   const result = await minify(
     { [name]: code },
@@ -207,6 +221,8 @@ async function optimizeWithTerser(
       ecma: target,
       // esbuild in the first pass is used to minify identifiers instead of mangle here
       mangle: false,
+      // esbuild in the first pass is used to minify function names
+      keep_fnames: true,
       format: {
         // ASCII output is enabled here as well to prevent terser from converting back to UTF-8
         ascii_only: true,