Skip to content

Commit

Permalink
fix(@angular-devkit/build-angular): ensure proper display of build lo…
Browse files Browse the repository at this point in the history
…gs 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
  • Loading branch information
alan-agius4 committed Mar 7, 2024
1 parent b6986a7 commit cccc220
Show file tree
Hide file tree
Showing 6 changed files with 79 additions and 44 deletions.
2 changes: 1 addition & 1 deletion goldens/public-api/angular_devkit/build_angular/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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;
Expand All @@ -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 {
Expand All @@ -68,6 +75,8 @@ export async function* runEsBuildBuildAction(
workspaceRoot,
progress,
preserveSymlinks,
colors,
jsonLogs,
} = options;

if (deleteOutputPath && writeToFileSystem) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ export async function executeBuild(
}

if (!jsonLogs) {
context.logger.info(
executionResult.addLog(
logBuildStats(
metafile,
initialFiles,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -18,6 +24,7 @@ import { executeBuild } from './execute-build';
import {
ApplicationBuilderExtensions,
ApplicationBuilderInternalOptions,
NormalizedApplicationBuildOptions,
normalizeOptions,
} from './options';
import { Schema as ApplicationBuilderOptions } from './schema';
Expand Down Expand Up @@ -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;
},
{
Expand All @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export class ExecutionResult {
errors: (Message | PartialMessage)[] = [];
prerenderedRoutes: string[] = [];
warnings: (Message | PartialMessage)[] = [];
logs: string[] = [];
externalMetadata?: ExternalResultMetadata;

constructor(
Expand All @@ -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 });
Expand Down
67 changes: 37 additions & 30 deletions packages/angular_devkit/build_angular/src/tools/esbuild/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'));
}
}

0 comments on commit cccc220

Please sign in to comment.