Skip to content

Commit 6785cae

Browse files
committed
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
1 parent fbfbca5 commit 6785cae

File tree

2 files changed

+54
-20
lines changed

2 files changed

+54
-20
lines changed

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

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9+
import { BuildFailure } from 'esbuild';
910
import Piscina from 'piscina';
1011
import type { Compiler, sources } from 'webpack';
1112
import { maxWorkers } from '../../utils/environment-options';
@@ -204,7 +205,15 @@ export class JavaScriptOptimizerPlugin {
204205
options: optimizeOptions,
205206
})
206207
.then(
207-
({ code, name, map }) => {
208+
async ({ code, name, map, errors }) => {
209+
if (errors?.length) {
210+
for (const error of errors) {
211+
addError(compilation, `Optimization error [${name}]: ${error}`);
212+
}
213+
214+
return;
215+
}
216+
208217
const optimizedAsset = map
209218
? new SourceMapSource(code, name, map)
210219
: new OriginalSource(code, name);
@@ -240,3 +249,12 @@ export class JavaScriptOptimizerPlugin {
240249
});
241250
}
242251
}
252+
253+
/**
254+
* Determines if an unknown value is an esbuild BuildFailure error object thrown by esbuild.
255+
* @param value A potential esbuild BuildFailure error object.
256+
* @returns `true` if the object is determined to be a BuildFailure object; otherwise, `false`.
257+
*/
258+
export function isEsBuildFailure(value: unknown): value is BuildFailure {
259+
return !!value && typeof value === 'object' && 'errors' in value && 'warnings' in value;
260+
}

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

Lines changed: 35 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,10 @@
77
*/
88

99
import remapping from '@ampproject/remapping';
10-
import type { TransformResult } from 'esbuild';
10+
import type { BuildFailure, TransformResult } from 'esbuild';
1111
import { minify } from 'terser';
1212
import { EsbuildExecutor } from './esbuild-executor';
13+
import { isEsBuildFailure } from './javascript-optimizer-plugin';
1314

1415
/**
1516
* The options to use when optimizing.
@@ -100,6 +101,13 @@ let esbuild: EsbuildExecutor | undefined;
100101
export default async function ({ asset, options }: OptimizeRequest) {
101102
// esbuild is used as a first pass
102103
const esbuildResult = await optimizeWithEsbuild(asset.code, asset.name, options);
104+
if (isEsBuildFailure(esbuildResult)) {
105+
return {
106+
name: asset.name,
107+
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
108+
errors: await esbuild!.formatMessages(esbuildResult.errors, { kind: 'error' }),
109+
};
110+
}
103111

104112
// terser is used as a second pass
105113
const terserResult = await optimizeWithTerser(
@@ -144,28 +152,36 @@ async function optimizeWithEsbuild(
144152
content: string,
145153
name: string,
146154
options: OptimizeRequest['options'],
147-
): Promise<TransformResult> {
155+
): Promise<TransformResult | BuildFailure> {
148156
if (!esbuild) {
149157
esbuild = new EsbuildExecutor(options.alwaysUseWasm);
150158
}
151159

152-
return esbuild.transform(content, {
153-
minifyIdentifiers: !options.keepIdentifierNames,
154-
minifySyntax: true,
155-
// NOTE: Disabling whitespace ensures unused pure annotations are kept
156-
minifyWhitespace: false,
157-
pure: ['forwardRef'],
158-
legalComments: options.removeLicenses ? 'none' : 'inline',
159-
sourcefile: name,
160-
sourcemap: options.sourcemap && 'external',
161-
define: options.define,
162-
// This option should always be disabled for browser builds as we don't rely on `.name`
163-
// and causes deadcode to be retained which makes `NG_BUILD_MANGLE` unusable to investigate tree-shaking issues.
164-
// We enable `keepNames` only for server builds as Domino relies on `.name`.
165-
// Once we no longer rely on Domino for SSR we should be able to remove this.
166-
keepNames: options.keepNames,
167-
target: options.target,
168-
});
160+
try {
161+
return await esbuild.transform(content, {
162+
minifyIdentifiers: !options.keepIdentifierNames,
163+
minifySyntax: true,
164+
// NOTE: Disabling whitespace ensures unused pure annotations are kept
165+
minifyWhitespace: false,
166+
pure: ['forwardRef'],
167+
legalComments: options.removeLicenses ? 'none' : 'inline',
168+
sourcefile: name,
169+
sourcemap: options.sourcemap && 'external',
170+
define: options.define,
171+
// This option should always be disabled for browser builds as we don't rely on `.name`
172+
// and causes deadcode to be retained which makes `NG_BUILD_MANGLE` unusable to investigate tree-shaking issues.
173+
// We enable `keepNames` only for server builds as Domino relies on `.name`.
174+
// Once we no longer rely on Domino for SSR we should be able to remove this.
175+
keepNames: options.keepNames,
176+
target: options.target,
177+
});
178+
} catch (error) {
179+
if (isEsBuildFailure(error)) {
180+
return error;
181+
}
182+
183+
throw error;
184+
}
169185
}
170186

171187
/**

0 commit comments

Comments
 (0)