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 angular#27233

Author: alan-agius4

goldens/public-api/angular_devkit/build_angular/index.md

@@ -17,7 +17,7 @@ import type http from 'node:http';
 import { json } from '@angular-devkit/core';
 import { Observable } from 'rxjs';
 import { OutputFile } from 'esbuild';
-import type { Plugin as Plugin_2 } from 'esbuild';
+import { Plugin as Plugin_2 } from 'esbuild';
 import webpack from 'webpack';
 import { WebpackLoggingCallback } from '@angular-devkit/build-webpack';
 

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

@@ -13,7 +13,12 @@ import path from 'node:path';
 import { BuildOutputFile } from '../../tools/esbuild/bundler-context';
 import { ExecutionResult, RebuildState } from '../../tools/esbuild/bundler-execution-result';
 import { shutdownSassWorkerPool } from '../../tools/esbuild/stylesheets/sass-language';
-import { withNoProgress, withSpinner, writeResultFiles } from '../../tools/esbuild/utils';
+import {
+  logMessages,
+  withNoProgress,
+  withSpinner,
+  writeResultFiles,
+} from '../../tools/esbuild/utils';
 import { deleteOutputDir } from '../../utils/delete-output-dir';
 import { shouldWatchRoot } from '../../utils/environment-options';
 import { NormalizedCachedOptions } from '../../utils/normalize-cache';
@@ -34,7 +39,7 @@ const packageWatchFiles = [
 ];
 
 export async function* runEsBuildBuildAction(
-  action: (rebuildState?: RebuildState) => ExecutionResult | Promise<ExecutionResult>,
+  action: (rebuildState?: RebuildState) => Promise<ExecutionResult>,
   options: {
     workspaceRoot: string;
     projectRoot: string;
@@ -51,6 +56,8 @@ export async function* runEsBuildBuildAction(
     signal?: AbortSignal;
     preserveSymlinks?: boolean;
     clearScreen?: boolean;
+    colors?: boolean;
+    jsonLogs?: boolean;
   },
 ): AsyncIterable<(ExecutionResult['outputWithFiles'] | ExecutionResult['output']) & BuilderOutput> {
   const {
@@ -68,6 +75,8 @@ export async function* runEsBuildBuildAction(
     workspaceRoot,
     progress,
     preserveSymlinks,
+    colors,
+    jsonLogs,
   } = options;
 
   if (deleteOutputPath && writeToFileSystem) {
@@ -84,6 +93,9 @@ export async function* runEsBuildBuildAction(
   try {
     // Perform the build action
     result = await withProgress('Building...', () => action());
+
+    // Log all diagnostic (error/warning/logs) messages
+    await logMessages(logger, result, colors, jsonLogs);
   } finally {
     // Ensure Sass workers are shutdown if not watching
     if (!watch) {
@@ -179,6 +191,9 @@ export async function* runEsBuildBuildAction(
         action(result.createRebuildState(changes)),
       );
 
+      // Log all diagnostic (error/warning/logs) messages
+      await logMessages(logger, result, colors, jsonLogs);
+
       // Update watched locations provided by the new build result.
       // Keep watching all previous files if there are any errors; otherwise consider all
       // files stale until confirmed present in the new result's watch files.

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

@@ -185,7 +185,7 @@ export async function executeBuild(
   }
 
   if (!jsonLogs) {
-    context.logger.info(
+    executionResult.addLog(
       logBuildStats(
         metafile,
         initialFiles,

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

@@ -7,9 +7,15 @@
  */
 
 import { BuilderContext, BuilderOutput, createBuilder } from '@angular-devkit/architect';
-import type { Plugin } from 'esbuild';
+import { type Plugin, formatMessages } from 'esbuild';
+import { join } from 'node:path';
+import { pathToFileURL } from 'node:url';
 import { BuildOutputFile, BuildOutputFileType } from '../../tools/esbuild/bundler-context';
-import { logMessages } from '../../tools/esbuild/utils';
+import { ExecutionResult } from '../../tools/esbuild/bundler-execution-result';
+import {
+  createJsonBuildManifest as createJsonBuildManifest,
+  logMessages,
+} from '../../tools/esbuild/utils';
 import { colors as ansiColors } from '../../utils/color';
 import { purgeStaleBuildCache } from '../../utils/purge-cache';
 import { assertCompatibleAngularVersion } from '../../utils/version';
@@ -18,6 +24,7 @@ import { executeBuild } from './execute-build';
 import {
   ApplicationBuilderExtensions,
   ApplicationBuilderInternalOptions,
+  NormalizedApplicationBuildOptions,
   normalizeOptions,
 } from './options';
 import { Schema as ApplicationBuilderOptions } from './schema';
@@ -90,29 +97,28 @@ export async function* buildApplicationInternal(
       const startTime = process.hrtime.bigint();
       const result = await executeBuild(normalizedOptions, context, rebuildState);
 
-      if (!jsonLogs) {
+      if (jsonLogs) {
+        result.addLog(await createJsonBuildManifest(result, normalizedOptions));
+      } else {
         if (prerenderOptions) {
           const prerenderedRoutesLength = result.prerenderedRoutes.length;
           let prerenderMsg = `Prerendered ${prerenderedRoutesLength} static route`;
           prerenderMsg += prerenderedRoutesLength !== 1 ? 's.' : '.';
 
-          logger.info(ansiColors.magenta(prerenderMsg));
+          result.addLog(ansiColors.magenta(prerenderMsg));
         }
 
         const buildTime = Number(process.hrtime.bigint() - startTime) / 10 ** 9;
         const hasError = result.errors.length > 0;
         if (writeToFileSystem && !hasError) {
-          logger.info(`Output location: ${outputOptions.base}\n`);
+          result.addLog(`Output location: ${outputOptions.base}\n`);
         }
 
-        logger.info(
-          `Application bundle generation ${hasError ? 'failed' : 'complete'}. [${buildTime.toFixed(3)} seconds]`,
+        result.addLog(
+          `Application bundle generation ${hasError ? 'failed' : 'complete'}. [${buildTime.toFixed(3)} seconds]\n`,
         );
       }
 
-      // Log all diagnostic (error/warning) messages
-      await logMessages(logger, result, normalizedOptions);
-
       return result;
     },
     {
@@ -127,6 +133,8 @@ export async function* buildApplicationInternal(
       workspaceRoot: normalizedOptions.workspaceRoot,
       progress: normalizedOptions.progress,
       clearScreen: normalizedOptions.clearScreen,
+      colors: normalizedOptions.colors,
+      jsonLogs: normalizedOptions.jsonLogs,
       writeToFileSystem,
       // For app-shell and SSG server files are not required by users.
       // Omit these when SSR is not enabled.

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

@@ -40,6 +40,7 @@ export class ExecutionResult {
   errors: (Message | PartialMessage)[] = [];
   prerenderedRoutes: string[] = [];
   warnings: (Message | PartialMessage)[] = [];
+  logs: string[] = [];
   externalMetadata?: ExternalResultMetadata;
 
   constructor(
@@ -55,6 +56,10 @@ export class ExecutionResult {
     this.assetFiles.push(...assets);
   }
 
+  addLog(value: string): void {
+    this.logs.push(value);
+  }
+
   addError(error: PartialMessage | string): void {
     if (typeof error === 'string') {
       this.errors.push({ text: error, location: null });

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

@@ -429,46 +429,53 @@ interface BuildManifest {
   prerenderedRoutes?: string[];
 }
 
-export async function logMessages(
-  logger: logging.LoggerApi,
-  executionResult: ExecutionResult,
-  options: NormalizedApplicationBuildOptions,
-): Promise<void> {
+export async function createJsonBuildManifest(
+  result: ExecutionResult,
+  normalizedOptions: NormalizedApplicationBuildOptions,
+): Promise<string> {
   const {
+    colors: color,
     outputOptions: { base, server, browser },
     ssrOptions,
-    jsonLogs,
-    colors: color,
-  } = options;
-  const { warnings, errors, prerenderedRoutes } = executionResult;
-  const warningMessages = warnings.length
-    ? await formatMessages(warnings, { kind: 'warning', color })
-    : [];
-  const errorMessages = errors.length ? await formatMessages(errors, { kind: 'error', color }) : [];
+  } = normalizedOptions;
 
-  if (jsonLogs) {
-    // JSON format output
-    const manifest: BuildManifest = {
-      errors: errorMessages,
-      warnings: warningMessages,
-      outputPaths: {
-        root: pathToFileURL(base),
-        browser: pathToFileURL(join(base, browser)),
-        server: ssrOptions ? pathToFileURL(join(base, server)) : undefined,
-      },
-      prerenderedRoutes,
-    };
+  const { warnings, errors, prerenderedRoutes } = result;
+
+  const manifest: BuildManifest = {
+    errors: errors.length ? await formatMessages(errors, { kind: 'error', color }) : [],
+    warnings: warnings.length ? await formatMessages(warnings, { kind: 'warning', color }) : [],
+    outputPaths: {
+      root: pathToFileURL(base),
+      browser: pathToFileURL(join(base, browser)),
+      server: ssrOptions ? pathToFileURL(join(base, server)) : undefined,
+    },
+    prerenderedRoutes,
+  };
+
+  return JSON.stringify(manifest, undefined, 2);
+}
+
+export async function logMessages(
+  logger: logging.LoggerApi,
+  executionResult: ExecutionResult,
+  color?: boolean,
+  jsonLogs?: boolean,
+): Promise<void> {
+  const { warnings, errors, logs } = executionResult;
 
-    logger.info(JSON.stringify(manifest, undefined, 2));
+  if (logs.length) {
+    logger.info(logs.join('\n'));
+  }
 
+  if (jsonLogs) {
     return;
   }
 
-  if (warningMessages.length) {
-    logger.warn(warningMessages.join('\n'));
+  if (warnings.length) {
+    logger.warn((await formatMessages(warnings, { kind: 'warning', color })).join('\n'));
   }
 
-  if (errorMessages.length) {
-    logger.error(errorMessages.join('\n'));
+  if (errors.length) {
+    logger.error((await formatMessages(errors, { kind: 'error', color })).join('\n'));
   }
 }