From adf7781114992284cc96a686492abee1e2ae48e4 Mon Sep 17 00:00:00 2001
From: Alan Agius <alanagius@google.com>
Date: Wed, 21 Dec 2022 13:24:06 +0000
Subject: [PATCH] fix(@angular-devkit/build-angular): format esbuild error
 messages to include more information

Prior to this change esbuild errors during the javascript optimization phase in the Webpack builder were not being formatting properly which caused meaningful information to be lost.

Closes #24457
---
 .../plugins/javascript-optimizer-plugin.ts    | 10 ++-
 .../plugins/javascript-optimizer-worker.ts    | 62 +++++++++++++------
 2 files changed, 52 insertions(+), 20 deletions(-)

diff --git a/packages/angular_devkit/build_angular/src/webpack/plugins/javascript-optimizer-plugin.ts b/packages/angular_devkit/build_angular/src/webpack/plugins/javascript-optimizer-plugin.ts
index b6f02d40a544..a7bb0402a73e 100644
--- a/packages/angular_devkit/build_angular/src/webpack/plugins/javascript-optimizer-plugin.ts
+++ b/packages/angular_devkit/build_angular/src/webpack/plugins/javascript-optimizer-plugin.ts
@@ -204,7 +204,15 @@ export class JavaScriptOptimizerPlugin {
                     options: optimizeOptions,
                   })
                   .then(
-                    ({ code, name, map }) => {
+                    async ({ code, name, map, errors }) => {
+                      if (errors?.length) {
+                        for (const error of errors) {
+                          addError(compilation, `Optimization error [${name}]: ${error}`);
+                        }
+
+                        return;
+                      }
+
                       const optimizedAsset = map
                         ? new SourceMapSource(code, name, map)
                         : new OriginalSource(code, name);
diff --git a/packages/angular_devkit/build_angular/src/webpack/plugins/javascript-optimizer-worker.ts b/packages/angular_devkit/build_angular/src/webpack/plugins/javascript-optimizer-worker.ts
index 240519f33218..75b67b0e333f 100644
--- a/packages/angular_devkit/build_angular/src/webpack/plugins/javascript-optimizer-worker.ts
+++ b/packages/angular_devkit/build_angular/src/webpack/plugins/javascript-optimizer-worker.ts
@@ -7,7 +7,7 @@
  */
 
 import remapping from '@ampproject/remapping';
-import type { TransformResult } from 'esbuild';
+import type { BuildFailure, TransformResult } from 'esbuild';
 import { minify } from 'terser';
 import { EsbuildExecutor } from './esbuild-executor';
 
@@ -100,6 +100,13 @@ let esbuild: EsbuildExecutor | undefined;
 export default async function ({ asset, options }: OptimizeRequest) {
   // esbuild is used as a first pass
   const esbuildResult = await optimizeWithEsbuild(asset.code, asset.name, options);
+  if (isEsBuildFailure(esbuildResult)) {
+    return {
+      name: asset.name,
+      // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+      errors: await esbuild!.formatMessages(esbuildResult.errors, { kind: 'error' }),
+    };
+  }
 
   // terser is used as a second pass
   const terserResult = await optimizeWithTerser(
@@ -144,28 +151,36 @@ async function optimizeWithEsbuild(
   content: string,
   name: string,
   options: OptimizeRequest['options'],
-): Promise<TransformResult> {
+): Promise<TransformResult | BuildFailure> {
   if (!esbuild) {
     esbuild = new EsbuildExecutor(options.alwaysUseWasm);
   }
 
-  return esbuild.transform(content, {
-    minifyIdentifiers: !options.keepIdentifierNames,
-    minifySyntax: true,
-    // NOTE: Disabling whitespace ensures unused pure annotations are kept
-    minifyWhitespace: false,
-    pure: ['forwardRef'],
-    legalComments: options.removeLicenses ? 'none' : 'inline',
-    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: options.target,
-  });
+  try {
+    return await esbuild.transform(content, {
+      minifyIdentifiers: !options.keepIdentifierNames,
+      minifySyntax: true,
+      // NOTE: Disabling whitespace ensures unused pure annotations are kept
+      minifyWhitespace: false,
+      pure: ['forwardRef'],
+      legalComments: options.removeLicenses ? 'none' : 'inline',
+      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: options.target,
+    });
+  } catch (error) {
+    if (isEsBuildFailure(error)) {
+      return error;
+    }
+
+    throw error;
+  }
 }
 
 /**
@@ -218,3 +233,12 @@ async function optimizeWithTerser(
 
   return { code: result.code, map: result.map as object };
 }
+
+/**
+ * Determines if an unknown value is an esbuild BuildFailure error object thrown by esbuild.
+ * @param value A potential esbuild BuildFailure error object.
+ * @returns `true` if the object is determined to be a BuildFailure object; otherwise, `false`.
+ */
+function isEsBuildFailure(value: unknown): value is BuildFailure {
+  return !!value && typeof value === 'object' && 'errors' in value && 'warnings' in value;
+}