Skip to content

Commit 518afd8

Browse files
alan-agius4alan-agius4
committed
fix(@angular-devkit/build-angular): ensure proper display of build logs in the presence of warnings or errors
Previously, a race condition could occur due to the spinner, resulting in the deletion of the last printed log when warnings or errors were present. With this update, we ensure that logs are printed after the spinner has stopped. Fixes #27233
1 parent 5acaf5d commit 518afd8

File tree

5 files changed

+70
-42
lines changed

5 files changed

+70
-42
lines changed

‎packages/angular_devkit/build_angular/src/builders/application/build-action.ts‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,12 @@ import path from 'node:path';
1313
import { BuildOutputFile } from '../../tools/esbuild/bundler-context';
1414
import { ExecutionResult, RebuildState } from '../../tools/esbuild/bundler-execution-result';
1515
import { shutdownSassWorkerPool } from '../../tools/esbuild/stylesheets/sass-language';
16-
import { withNoProgress, withSpinner, writeResultFiles } from '../../tools/esbuild/utils';
16+
import {
17+
logMessages,
18+
withNoProgress,
19+
withSpinner,
20+
writeResultFiles,
21+
} from '../../tools/esbuild/utils';
1722
import { deleteOutputDir } from '../../utils/delete-output-dir';
1823
import { shouldWatchRoot } from '../../utils/environment-options';
1924
import { NormalizedCachedOptions } from '../../utils/normalize-cache';
@@ -34,7 +39,7 @@ const packageWatchFiles = [
3439
];
3540

3641
export async function* runEsBuildBuildAction(
37-
action: (rebuildState?: RebuildState) => ExecutionResult | Promise<ExecutionResult>,
42+
action: (rebuildState?: RebuildState) => Promise<ExecutionResult>,
3843
options: {
3944
workspaceRoot: string;
4045
projectRoot: string;
@@ -51,6 +56,8 @@ export async function* runEsBuildBuildAction(
5156
signal?: AbortSignal;
5257
preserveSymlinks?: boolean;
5358
clearScreen?: boolean;
59+
colors?: boolean;
60+
jsonLogs?: boolean;
5461
},
5562
): AsyncIterable<(ExecutionResult['outputWithFiles'] | ExecutionResult['output']) & BuilderOutput> {
5663
const {
@@ -68,6 +75,8 @@ export async function* runEsBuildBuildAction(
6875
workspaceRoot,
6976
progress,
7077
preserveSymlinks,
78+
colors,
79+
jsonLogs,
7180
} = options;
7281

7382
if (deleteOutputPath && writeToFileSystem) {
@@ -84,6 +93,9 @@ export async function* runEsBuildBuildAction(
8493
try {
8594
// Perform the build action
8695
result = await withProgress('Building...', () => action());
96+
97+
// Log all diagnostic (error/warning/logs) messages
98+
await logMessages(logger, result, colors, jsonLogs);
8799
} finally {
88100
// Ensure Sass workers are shutdown if not watching
89101
if (!watch) {
@@ -179,6 +191,9 @@ export async function* runEsBuildBuildAction(
179191
action(result.createRebuildState(changes)),
180192
);
181193

194+
// Log all diagnostic (error/warning/logs) messages
195+
await logMessages(logger, result, colors, jsonLogs);
196+
182197
// Update watched locations provided by the new build result.
183198
// Keep watching all previous files if there are any errors; otherwise consider all
184199
// files stale until confirmed present in the new result's watch files.

‎packages/angular_devkit/build_angular/src/builders/application/execute-build.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ export async function executeBuild(
185185
}
186186

187187
if (!jsonLogs) {
188-
context.logger.info(
188+
executionResult.addLog(
189189
logBuildStats(
190190
metafile,
191191
initialFiles,

‎packages/angular_devkit/build_angular/src/builders/application/index.ts‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
import { BuilderContext, BuilderOutput, createBuilder } from '@angular-devkit/architect';
1010
import type { Plugin } from 'esbuild';
1111
import { BuildOutputFile, BuildOutputFileType } from '../../tools/esbuild/bundler-context';
12-
import { logMessages } from '../../tools/esbuild/utils';
12+
import { createJsonBuildManifest } from '../../tools/esbuild/utils';
1313
import { colors as ansiColors } from '../../utils/color';
1414
import { purgeStaleBuildCache } from '../../utils/purge-cache';
1515
import { assertCompatibleAngularVersion } from '../../utils/version';
@@ -90,29 +90,28 @@ export async function* buildApplicationInternal(
9090
const startTime = process.hrtime.bigint();
9191
const result = await executeBuild(normalizedOptions, context, rebuildState);
9292

93-
if (!jsonLogs) {
93+
if (jsonLogs) {
94+
result.addLog(await createJsonBuildManifest(result, normalizedOptions));
95+
} else {
9496
if (prerenderOptions) {
9597
const prerenderedRoutesLength = result.prerenderedRoutes.length;
9698
let prerenderMsg = `Prerendered ${prerenderedRoutesLength} static route`;
9799
prerenderMsg += prerenderedRoutesLength !== 1 ? 's.' : '.';
98100

99-
logger.info(ansiColors.magenta(prerenderMsg));
101+
result.addLog(ansiColors.magenta(prerenderMsg));
100102
}
101103

102104
const buildTime = Number(process.hrtime.bigint() - startTime) / 10 ** 9;
103105
const hasError = result.errors.length > 0;
104106
if (writeToFileSystem && !hasError) {
105-
logger.info(`Output location: ${outputOptions.base}\n`);
107+
result.addLog(`Output location: ${outputOptions.base}\n`);
106108
}
107109

108-
logger.info(
109-
`Application bundle generation ${hasError ? 'failed' : 'complete'}. [${buildTime.toFixed(3)} seconds]`,
110+
result.addLog(
111+
`Application bundle generation ${hasError ? 'failed' : 'complete'}. [${buildTime.toFixed(3)} seconds]\n`,
110112
);
111113
}
112114

113-
// Log all diagnostic (error/warning) messages
114-
await logMessages(logger, result, normalizedOptions);
115-
116115
return result;
117116
},
118117
{
@@ -127,6 +126,8 @@ export async function* buildApplicationInternal(
127126
workspaceRoot: normalizedOptions.workspaceRoot,
128127
progress: normalizedOptions.progress,
129128
clearScreen: normalizedOptions.clearScreen,
129+
colors: normalizedOptions.colors,
130+
jsonLogs: normalizedOptions.jsonLogs,
130131
writeToFileSystem,
131132
// For app-shell and SSG server files are not required by users.
132133
// Omit these when SSR is not enabled.

‎packages/angular_devkit/build_angular/src/tools/esbuild/bundler-execution-result.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ export class ExecutionResult {
4040
errors: (Message | PartialMessage)[] = [];
4141
prerenderedRoutes: string[] = [];
4242
warnings: (Message | PartialMessage)[] = [];
43+
logs: string[] = [];
4344
externalMetadata?: ExternalResultMetadata;
4445

4546
constructor(
@@ -55,6 +56,10 @@ export class ExecutionResult {
5556
this.assetFiles.push(...assets);
5657
}
5758

59+
addLog(value: string): void {
60+
this.logs.push(value);
61+
}
62+
5863
addError(error: PartialMessage | string): void {
5964
if (typeof error === 'string') {
6065
this.errors.push({ text: error, location: null });

‎packages/angular_devkit/build_angular/src/tools/esbuild/utils.ts‎

Lines changed: 37 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -429,46 +429,53 @@ interface BuildManifest {
429429
prerenderedRoutes?: string[];
430430
}
431431

432-
export async function logMessages(
433-
logger: logging.LoggerApi,
434-
executionResult: ExecutionResult,
435-
options: NormalizedApplicationBuildOptions,
436-
): Promise<void> {
432+
export async function createJsonBuildManifest(
433+
result: ExecutionResult,
434+
normalizedOptions: NormalizedApplicationBuildOptions,
435+
): Promise<string> {
437436
const {
437+
colors: color,
438438
outputOptions: { base, server, browser },
439439
ssrOptions,
440-
jsonLogs,
441-
colors: color,
442-
} = options;
443-
const { warnings, errors, prerenderedRoutes } = executionResult;
444-
const warningMessages = warnings.length
445-
? await formatMessages(warnings, { kind: 'warning', color })
446-
: [];
447-
const errorMessages = errors.length ? await formatMessages(errors, { kind: 'error', color }) : [];
440+
} = normalizedOptions;
448441

449-
if (jsonLogs) {
450-
// JSON format output
451-
const manifest: BuildManifest = {
452-
errors: errorMessages,
453-
warnings: warningMessages,
454-
outputPaths: {
455-
root: pathToFileURL(base),
456-
browser: pathToFileURL(join(base, browser)),
457-
server: ssrOptions ? pathToFileURL(join(base, server)) : undefined,
458-
},
459-
prerenderedRoutes,
460-
};
442+
const { warnings, errors, prerenderedRoutes } = result;
443+
444+
const manifest: BuildManifest = {
445+
errors: errors.length ? await formatMessages(errors, { kind: 'error', color }) : [],
446+
warnings: warnings.length ? await formatMessages(warnings, { kind: 'warning', color }) : [],
447+
outputPaths: {
448+
root: pathToFileURL(base),
449+
browser: pathToFileURL(join(base, browser)),
450+
server: ssrOptions ? pathToFileURL(join(base, server)) : undefined,
451+
},
452+
prerenderedRoutes,
453+
};
454+
455+
return JSON.stringify(manifest, undefined, 2);
456+
}
457+
458+
export async function logMessages(
459+
logger: logging.LoggerApi,
460+
executionResult: ExecutionResult,
461+
color?: boolean,
462+
jsonLogs?: boolean,
463+
): Promise<void> {
464+
const { warnings, errors, logs } = executionResult;
461465

462-
logger.info(JSON.stringify(manifest, undefined, 2));
466+
if (logs.length) {
467+
logger.info(logs.join('\n'));
468+
}
463469

470+
if (jsonLogs) {
464471
return;
465472
}
466473

467-
if (warningMessages.length) {
468-
logger.warn(warningMessages.join('\n'));
474+
if (warnings.length) {
475+
logger.warn((await formatMessages(warnings, { kind: 'warning', color })).join('\n'));
469476
}
470477

471-
if (errorMessages.length) {
472-
logger.error(errorMessages.join('\n'));
478+
if (errors.length) {
479+
logger.error((await formatMessages(errors, { kind: 'error', color })).join('\n'));
473480
}
474481
}

0 commit comments

Comments
 (0)