From 421b6e75b4bafae1e67d59c9343b4a8f98d19400 Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Wed, 17 Jul 2024 13:36:20 -0400 Subject: [PATCH] refactor(@angular/build): provide structured application builder result types The application builder now provides structured output types to its internal consumers. The architect builders themselves and the programmatic API is not changed. These output result types allow for the development server to receive additional information regarding the build and update the active browser appropriately. This functionality is not yet implemented but the additional result types provide the base infrastructure to enable future features. The result types also allow for reduced complexity inside other builders such as i18n extraction and the browser compatibility builder. The usage is not yet fully optimized and will be refined in future changes. --- .../src/builders/application/build-action.ts | 63 +++++++-- .../src/builders/application/execute-build.ts | 2 + .../build/src/builders/application/index.ts | 29 ++-- .../build/src/builders/application/results.ts | 74 ++++++++++ .../tests/options/output-path_spec.ts | 20 ++- .../src/builders/dev-server/vite-server.ts | 132 ++++++++++-------- .../extract-i18n/application-extraction.ts | 77 +++++----- .../src/builders/extract-i18n/builder.ts | 5 +- packages/angular/build/src/private.ts | 1 + .../tools/esbuild/bundler-execution-result.ts | 2 + .../src/builders/browser-esbuild/index.ts | 102 ++++++-------- .../tests/options/assets_spec.ts | 4 +- .../tests/options/delete-output-path_spec.ts | 4 +- .../tests/options/deploy-url_spec.ts | 4 +- .../tests/options/main_spec.ts | 4 +- .../extract-i18n/application-extraction.ts | 72 +++++----- .../src/builders/extract-i18n/builder.ts | 12 +- .../src/builders/web-test-runner/index.ts | 8 +- 18 files changed, 368 insertions(+), 247 deletions(-) create mode 100644 packages/angular/build/src/builders/application/results.ts diff --git a/packages/angular/build/src/builders/application/build-action.ts b/packages/angular/build/src/builders/application/build-action.ts index 668516477d4e..f511503bfb6c 100644 --- a/packages/angular/build/src/builders/application/build-action.ts +++ b/packages/angular/build/src/builders/application/build-action.ts @@ -6,10 +6,10 @@ * found in the LICENSE file at https://angular.dev/license */ -import { BuilderContext, BuilderOutput } from '@angular-devkit/architect'; +import { BuilderContext } from '@angular-devkit/architect'; import { existsSync } from 'node:fs'; import path from 'node:path'; -import { BuildOutputFile } from '../../tools/esbuild/bundler-context'; +import { BuildOutputFile, BuildOutputFileType } from '../../tools/esbuild/bundler-context'; import { ExecutionResult, RebuildState } from '../../tools/esbuild/bundler-execution-result'; import { shutdownSassWorkerPool } from '../../tools/esbuild/stylesheets/sass-language'; import { @@ -22,6 +22,7 @@ import { deleteOutputDir } from '../../utils/delete-output-dir'; import { shouldWatchRoot } from '../../utils/environment-options'; import { NormalizedCachedOptions } from '../../utils/normalize-cache'; import { NormalizedApplicationBuildOptions, NormalizedOutputOptions } from './options'; +import { FullResult, Result, ResultKind, ResultMessage } from './results'; // Watch workspace for package manager changes const packageWatchFiles = [ @@ -37,9 +38,6 @@ const packageWatchFiles = [ '.pnp.data.json', ]; -type BuildActionOutput = (ExecutionResult['outputWithFiles'] | ExecutionResult['output']) & - BuilderOutput; - export async function* runEsBuildBuildAction( action: (rebuildState?: RebuildState) => Promise, options: { @@ -61,7 +59,7 @@ export async function* runEsBuildBuildAction( colors?: boolean; jsonLogs?: boolean; }, -): AsyncIterable { +): AsyncIterable { const { writeToFileSystemFilter, writeToFileSystem, @@ -226,10 +224,24 @@ export async function* runEsBuildBuildAction( async function writeAndEmitOutput( writeToFileSystem: boolean, - { outputFiles, output, outputWithFiles, assetFiles }: ExecutionResult, + { + outputFiles, + outputWithFiles, + assetFiles, + externalMetadata, + htmlIndexPath, + htmlBaseHref, + }: ExecutionResult, outputOptions: NormalizedApplicationBuildOptions['outputOptions'], writeToFileSystemFilter: ((file: BuildOutputFile) => boolean) | undefined, -): Promise { +): Promise { + if (!outputWithFiles.success) { + return { + kind: ResultKind.Failure, + errors: outputWithFiles.errors as ResultMessage[], + }; + } + if (writeToFileSystem) { // Write output files const outputFilesToWrite = writeToFileSystemFilter @@ -238,10 +250,37 @@ async function writeAndEmitOutput( await writeResultFiles(outputFilesToWrite, assetFiles, outputOptions); - return output; + // Currently unused other than indicating success if writing to disk. + return { + kind: ResultKind.Full, + files: {}, + }; } else { - // Requires casting due to unneeded `JsonObject` requirement. Remove once fixed. - // eslint-disable-next-line @typescript-eslint/no-explicit-any - return outputWithFiles as any; + const result: FullResult = { + kind: ResultKind.Full, + files: {}, + detail: { + externalMetadata, + htmlIndexPath, + htmlBaseHref, + }, + }; + for (const file of outputWithFiles.assetFiles) { + result.files[file.destination] = { + type: BuildOutputFileType.Browser, + inputPath: file.source, + origin: 'disk', + }; + } + for (const file of outputWithFiles.outputFiles) { + result.files[file.path] = { + type: file.type, + contents: file.contents, + origin: 'memory', + hash: file.hash, + }; + } + + return result; } } diff --git a/packages/angular/build/src/builders/application/execute-build.ts b/packages/angular/build/src/builders/application/execute-build.ts index 08f3934e28d1..5e77e2c48037 100644 --- a/packages/angular/build/src/builders/application/execute-build.ts +++ b/packages/angular/build/src/builders/application/execute-build.ts @@ -156,6 +156,8 @@ export async function executeBuild( // Watch input index HTML file if configured if (options.indexHtmlOptions) { executionResult.extraWatchFiles.push(options.indexHtmlOptions.input); + executionResult.htmlIndexPath = options.indexHtmlOptions.output; + executionResult.htmlBaseHref = options.baseHref; } // Perform i18n translation inlining if enabled diff --git a/packages/angular/build/src/builders/application/index.ts b/packages/angular/build/src/builders/application/index.ts index 4f8e34ec238e..9feb008ab4dd 100644 --- a/packages/angular/build/src/builders/application/index.ts +++ b/packages/angular/build/src/builders/application/index.ts @@ -20,6 +20,7 @@ import { ApplicationBuilderInternalOptions, normalizeOptions, } from './options'; +import { Result, ResultKind } from './results'; import { Schema as ApplicationBuilderOptions } from './schema'; export type { ApplicationBuilderOptions }; @@ -32,7 +33,7 @@ export async function* buildApplicationInternal( write?: boolean; }, extensions?: ApplicationBuilderExtensions, -): AsyncIterable { +): AsyncIterable { const { workspaceRoot, logger, target } = context; // Check Angular version. @@ -44,7 +45,9 @@ export async function* buildApplicationInternal( // Determine project name from builder context target const projectName = target?.project; if (!projectName) { - yield { success: false, error: `The 'application' builder requires a target to be specified.` }; + context.logger.error(`The 'application' builder requires a target to be specified.`); + // Only the vite-based dev server current uses the errors value + yield { kind: ResultKind.Failure, errors: [] }; return; } @@ -57,19 +60,19 @@ export async function* buildApplicationInternal( if (writeServerBundles) { const { browser, server } = normalizedOptions.outputOptions; if (browser === '') { - yield { - success: false, - error: `'outputPath.browser' cannot be configured to an empty string when SSR is enabled.`, - }; + context.logger.error( + `'outputPath.browser' cannot be configured to an empty string when SSR is enabled.`, + ); + yield { kind: ResultKind.Failure, errors: [] }; return; } if (browser === server) { - yield { - success: false, - error: `'outputPath.browser' and 'outputPath.server' cannot be configured to the same value.`, - }; + context.logger.error( + `'outputPath.browser' and 'outputPath.server' cannot be configured to the same value.`, + ); + yield { kind: ResultKind.Failure, errors: [] }; return; } @@ -185,7 +188,7 @@ export function buildApplication( extensions?: ApplicationBuilderExtensions, ): AsyncIterable; -export function buildApplication( +export async function* buildApplication( options: ApplicationBuilderOptions, context: BuilderContext, pluginsOrExtensions?: Plugin[] | ApplicationBuilderExtensions, @@ -199,7 +202,9 @@ export function buildApplication( extensions = pluginsOrExtensions; } - return buildApplicationInternal(options, context, undefined, extensions); + for await (const result of buildApplicationInternal(options, context, undefined, extensions)) { + yield { success: result.kind !== ResultKind.Failure }; + } } export default createBuilder(buildApplication); diff --git a/packages/angular/build/src/builders/application/results.ts b/packages/angular/build/src/builders/application/results.ts new file mode 100644 index 000000000000..165315c2657b --- /dev/null +++ b/packages/angular/build/src/builders/application/results.ts @@ -0,0 +1,74 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.dev/license + */ + +import { BuildOutputFileType } from '../../tools/esbuild/bundler-context'; + +export enum ResultKind { + Failure, + Full, + Incremental, + ComponentUpdate, +} + +export type Result = FailureResult | FullResult | IncrementalResult | ComponentUpdateResult; + +export interface BaseResult { + kind: ResultKind; + warnings?: ResultMessage[]; + duration?: number; + detail?: Record; +} + +export interface FailureResult extends BaseResult { + kind: ResultKind.Failure; + errors: ResultMessage[]; +} + +export interface FullResult extends BaseResult { + kind: ResultKind.Full; + files: Record; +} + +export interface IncrementalResult extends BaseResult { + kind: ResultKind.Incremental; + added: string[]; + removed: string[]; + modified: string[]; + files: Record; +} + +export type ResultFile = DiskFile | MemoryFile; + +export interface BaseResultFile { + origin: 'memory' | 'disk'; + type: BuildOutputFileType; +} + +export interface DiskFile extends BaseResultFile { + origin: 'disk'; + inputPath: string; +} + +export interface MemoryFile extends BaseResultFile { + origin: 'memory'; + hash: string; + contents: Uint8Array; +} + +export interface ResultMessage { + text: string; + location?: { file: string; line: number; column: number } | null; + notes?: { text: string }[]; +} + +export interface ComponentUpdateResult extends BaseResult { + kind: ResultKind.ComponentUpdate; + id: string; + type: 'style' | 'template'; + content: string; +} diff --git a/packages/angular/build/src/builders/application/tests/options/output-path_spec.ts b/packages/angular/build/src/builders/application/tests/options/output-path_spec.ts index b08314b53427..f8d4513c7de7 100644 --- a/packages/angular/build/src/builders/application/tests/options/output-path_spec.ts +++ b/packages/angular/build/src/builders/application/tests/options/output-path_spec.ts @@ -286,10 +286,14 @@ describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => { }, }); - const { result } = await harness.executeOnce(); + const { result, logs } = await harness.executeOnce({ outputLogsOnFailure: false }); expect(result?.success).toBeFalse(); - expect(result?.error).toContain( - `'outputPath.browser' cannot be configured to an empty string when SSR is enabled`, + expect(logs).toContain( + jasmine.objectContaining({ + message: jasmine.stringMatching( + `'outputPath.browser' cannot be configured to an empty string when SSR is enabled`, + ), + }), ); }); }); @@ -349,10 +353,14 @@ describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => { }, }); - const { result } = await harness.executeOnce(); + const { result, logs } = await harness.executeOnce({ outputLogsOnFailure: false }); expect(result?.success).toBeFalse(); - expect(result?.error).toContain( - `'outputPath.browser' and 'outputPath.server' cannot be configured to the same value`, + expect(logs).toContain( + jasmine.objectContaining({ + message: jasmine.stringMatching( + `'outputPath.browser' and 'outputPath.server' cannot be configured to the same value`, + ), + }), ); }); }); diff --git a/packages/angular/build/src/builders/dev-server/vite-server.ts b/packages/angular/build/src/builders/dev-server/vite-server.ts index 239abd4401c7..7e8983900a65 100644 --- a/packages/angular/build/src/builders/dev-server/vite-server.ts +++ b/packages/angular/build/src/builders/dev-server/vite-server.ts @@ -12,17 +12,16 @@ import assert from 'node:assert'; import { readFile } from 'node:fs/promises'; import inspector from 'node:inspector'; import { builtinModules } from 'node:module'; -import { basename, join } from 'node:path'; +import { join } from 'node:path'; import type { Connect, DepOptimizationConfig, InlineConfig, ViteDevServer } from 'vite'; import { createAngularMemoryPlugin } from '../../tools/vite/angular-memory-plugin'; import { createAngularLocaleDataPlugin } from '../../tools/vite/i18n-locale-plugin'; import { createRemoveIdPrefixPlugin } from '../../tools/vite/id-prefix-plugin'; import { loadProxyConfiguration, normalizeSourceMaps } from '../../utils'; import { loadEsmModule } from '../../utils/load-esm'; -import { ApplicationBuilderOutput } from '../application'; +import { Result, ResultFile, ResultKind } from '../application/results'; import { type ApplicationBuilderInternalOptions, - type BuildOutputFile, BuildOutputFileType, type ExternalResultMetadata, JavaScriptTransformer, @@ -46,7 +45,7 @@ export type BuilderAction = ( options: ApplicationBuilderInternalOptions, context: BuilderContext, plugins?: Plugin[], -) => AsyncIterable; +) => AsyncIterable; /** * Build options that are also present on the dev server but are only passed @@ -103,13 +102,6 @@ export async function* serveWithVite( // Set all packages as external to support Vite's prebundle caching browserOptions.externalPackages = serverOptions.prebundle; - const baseHref = browserOptions.baseHref; - if (serverOptions.servePath === undefined && baseHref !== undefined) { - // Remove trailing slash - serverOptions.servePath = - baseHref !== './' && baseHref[baseHref.length - 1] === '/' ? baseHref.slice(0, -1) : baseHref; - } - // The development server currently only supports a single locale when localizing. // This matches the behavior of the Webpack-based development server but could be expanded in the future. if ( @@ -136,15 +128,8 @@ export async function* serveWithVite( 1, ); - // Extract output index from options - // TODO: Provide this info from the build results + // The index HTML path will be updated from the build results if provided by the builder let htmlIndexPath = 'index.html'; - if (browserOptions.index && typeof browserOptions.index !== 'boolean') { - htmlIndexPath = - typeof browserOptions.index === 'string' - ? basename(browserOptions.index) - : browserOptions.index.output || 'index.html'; - } // dynamically import Vite for ESM compatibility const { createServer, normalizePath } = await loadEsmModule('vite'); @@ -170,24 +155,57 @@ export async function* serveWithVite( // TODO: Switch this to an architect schedule call when infrastructure settings are supported for await (const result of builderAction(browserOptions, context, extensions?.buildPlugins)) { - assert(result.outputFiles, 'Builder did not provide result files.'); - - // If build failed, nothing to serve - if (!result.success) { - // If server is active, send an error notification - if (result.errors?.length && server) { - hadError = true; - server.hot.send({ - type: 'error', - err: { - message: result.errors[0].text, - stack: '', - loc: result.errors[0].location, - }, - }); - } - continue; - } else if (hadError && server) { + switch (result.kind) { + case ResultKind.Failure: + if (result.errors.length && server) { + hadError = true; + server.hot.send({ + type: 'error', + err: { + message: result.errors[0].text, + stack: '', + loc: result.errors[0].location ?? undefined, + }, + }); + } + continue; + case ResultKind.Full: + if (result.detail?.['htmlIndexPath']) { + htmlIndexPath = result.detail['htmlIndexPath'] as string; + } + if (serverOptions.servePath === undefined && result.detail?.['htmlBaseHref']) { + const baseHref = result.detail['htmlBaseHref'] as string; + // Remove trailing slash + serverOptions.servePath = + baseHref !== './' && baseHref[baseHref.length - 1] === '/' + ? baseHref.slice(0, -1) + : baseHref; + } + + assetFiles.clear(); + for (const [outputPath, file] of Object.entries(result.files)) { + if (file.origin === 'disk') { + assetFiles.set('/' + normalizePath(outputPath), normalizePath(file.inputPath)); + } + } + // Analyze result files for changes + analyzeResultFiles(normalizePath, htmlIndexPath, result.files, generatedFiles); + break; + case ResultKind.Incremental: + assert(server, 'Builder must provide an initial full build before incremental results.'); + // TODO: Implement support -- application builder currently does not use + break; + case ResultKind.ComponentUpdate: + assert(serverOptions.hmr, 'Component updates are only supported with HMR enabled.'); + // TODO: Implement support -- application builder currently does not use + break; + default: + context.logger.warn(`Unknown result kind [${(result as Result).kind}] provided by build.`); + continue; + } + + // Clear existing error overlay on successful result + if (hadError && server) { hadError = false; // Send an empty update to clear the error overlay server.hot.send({ @@ -196,24 +214,16 @@ export async function* serveWithVite( }); } - // Analyze result files for changes - analyzeResultFiles(normalizePath, htmlIndexPath, result.outputFiles, generatedFiles); - - assetFiles.clear(); - if (result.assetFiles) { - for (const asset of result.assetFiles) { - assetFiles.set('/' + normalizePath(asset.destination), normalizePath(asset.source)); - } - } - // To avoid disconnecting the array objects from the option, these arrays need to be mutated instead of replaced. let requiresServerRestart = false; - if (result.externalMetadata) { - const { implicitBrowser, implicitServer, explicit } = result.externalMetadata; - const implicitServerFiltered = (implicitServer as string[]).filter( + if (result.detail?.['externalMetadata']) { + const { implicitBrowser, implicitServer, explicit } = result.detail[ + 'externalMetadata' + ] as ExternalResultMetadata; + const implicitServerFiltered = implicitServer.filter( (m) => removeNodeJsBuiltinModules(m) && removeAbsoluteUrls(m), ); - const implicitBrowserFiltered = (implicitBrowser as string[]).filter(removeAbsoluteUrls); + const implicitBrowserFiltered = implicitBrowser.filter(removeAbsoluteUrls); if (browserOptions.ssr && serverOptions.prebundle !== false) { const previousImplicitServer = new Set(externalMetadata.implicitServer); @@ -400,28 +410,33 @@ function handleUpdate( function analyzeResultFiles( normalizePath: (id: string) => string, htmlIndexPath: string, - resultFiles: BuildOutputFile[], + resultFiles: Record, generatedFiles: Map, ) { const seen = new Set(['/index.html']); - for (const file of resultFiles) { + for (const [outputPath, file] of Object.entries(resultFiles)) { + if (file.origin === 'disk') { + continue; + } let filePath; - if (file.path === htmlIndexPath) { + if (outputPath === htmlIndexPath) { // Convert custom index output path to standard index path for dev-server usage. // This mimics the Webpack dev-server behavior. filePath = '/index.html'; } else { - filePath = '/' + normalizePath(file.path); + filePath = '/' + normalizePath(outputPath); } seen.add(filePath); + const servable = + file.type === BuildOutputFileType.Browser || file.type === BuildOutputFileType.Media; + // Skip analysis of sourcemaps if (filePath.endsWith('.map')) { generatedFiles.set(filePath, { contents: file.contents, - servable: - file.type === BuildOutputFileType.Browser || file.type === BuildOutputFileType.Media, + servable, size: file.contents.byteLength, updated: false, }); @@ -446,8 +461,7 @@ function analyzeResultFiles( size: file.contents.byteLength, hash: file.hash, updated: true, - servable: - file.type === BuildOutputFileType.Browser || file.type === BuildOutputFileType.Media, + servable, }); } diff --git a/packages/angular/build/src/builders/extract-i18n/application-extraction.ts b/packages/angular/build/src/builders/extract-i18n/application-extraction.ts index 45259b723e32..7a146722794d 100644 --- a/packages/angular/build/src/builders/extract-i18n/application-extraction.ts +++ b/packages/angular/build/src/builders/extract-i18n/application-extraction.ts @@ -8,14 +8,14 @@ import type { ɵParsedMessage as LocalizeMessage } from '@angular/localize'; import type { MessageExtractor } from '@angular/localize/tools'; -import type { BuilderContext, BuilderOutput } from '@angular-devkit/architect'; -import assert from 'node:assert'; +import type { BuilderContext } from '@angular-devkit/architect'; import nodePath from 'node:path'; import { buildApplicationInternal } from '../application'; import type { ApplicationBuilderExtensions, ApplicationBuilderInternalOptions, } from '../application/options'; +import { ResultFile, ResultKind } from '../application/results'; import type { NormalizedExtractI18nOptions } from './options'; export async function extractMessages( @@ -25,7 +25,7 @@ export async function extractMessages( extractorConstructor: typeof MessageExtractor, extensions?: ApplicationBuilderExtensions, ): Promise<{ - builderResult: BuilderOutput; + success: boolean; basePath: string; messages: LocalizeMessage[]; useLegacyIds: boolean; @@ -49,44 +49,22 @@ export async function extractMessages( buildOptions.prerender = false; // Build the application with the build options - let builderResult; - try { - for await (const result of buildApplicationInternal( - buildOptions, - context, - { write: false }, - extensions, - )) { - builderResult = result; - break; - } - - assert(builderResult !== undefined, 'Application builder did not provide a result.'); - } catch (err) { - builderResult = { - success: false, - error: (err as Error).message, - }; - } - - // Extract messages from each output JavaScript file. - // Output files are only present on a successful build. - if (builderResult.outputFiles) { - // Store the JS and JS map files for lookup during extraction - const files = new Map(); - for (const outputFile of builderResult.outputFiles) { - if (outputFile.path.endsWith('.js')) { - files.set(outputFile.path, outputFile.text); - } else if (outputFile.path.endsWith('.js.map')) { - files.set(outputFile.path, outputFile.text); - } - } - + const builderResult = await first( + buildApplicationInternal(buildOptions, context, { write: false }, extensions), + ); + + let success = false; + if (!builderResult || builderResult.kind === ResultKind.Failure) { + context.logger.error('Application build failed.'); + } else if (builderResult.kind !== ResultKind.Full) { + context.logger.error('Application build did not provide a full output.'); + } else { // Setup the localize message extractor based on the in-memory files - const extractor = setupLocalizeExtractor(extractorConstructor, files, context); + const extractor = setupLocalizeExtractor(extractorConstructor, builderResult.files, context); - // Attempt extraction of all output JS files - for (const filePath of files.keys()) { + // Extract messages from each output JavaScript file. + // Output files are only present on a successful build. + for (const filePath of Object.keys(builderResult.files)) { if (!filePath.endsWith('.js')) { continue; } @@ -94,10 +72,12 @@ export async function extractMessages( const fileMessages = extractor.extractMessages(filePath); messages.push(...fileMessages); } + + success = true; } return { - builderResult, + success, basePath: context.workspaceRoot, messages, // Legacy i18n identifiers are not supported with the new application builder @@ -107,9 +87,10 @@ export async function extractMessages( function setupLocalizeExtractor( extractorConstructor: typeof MessageExtractor, - files: Map, + files: Record, context: BuilderContext, ): MessageExtractor { + const textDecoder = new TextDecoder(); // Setup a virtual file system instance for the extractor // * MessageExtractor itself uses readFile, relative and resolve // * Internal SourceFileLoader (sourcemap support) uses dirname, exists, readFile, and resolve @@ -118,7 +99,11 @@ function setupLocalizeExtractor( // Output files are stored as relative to the workspace root const requestedPath = nodePath.relative(context.workspaceRoot, path); - const content = files.get(requestedPath); + const file = files[requestedPath]; + let content; + if (file?.origin === 'memory') { + content = textDecoder.decode(file.contents); + } if (content === undefined) { throw new Error('Unknown file requested: ' + requestedPath); } @@ -135,7 +120,7 @@ function setupLocalizeExtractor( // Output files are stored as relative to the workspace root const requestedPath = nodePath.relative(context.workspaceRoot, path); - return files.has(requestedPath); + return files[requestedPath] !== undefined; }, dirname(path: string): string { return nodePath.dirname(path); @@ -169,3 +154,9 @@ function setupLocalizeExtractor( return extractor; } + +async function first(iterable: AsyncIterable): Promise { + for await (const value of iterable) { + return value; + } +} diff --git a/packages/angular/build/src/builders/extract-i18n/builder.ts b/packages/angular/build/src/builders/extract-i18n/builder.ts index 78ced2c7e18b..ab4397c5d505 100644 --- a/packages/angular/build/src/builders/extract-i18n/builder.ts +++ b/packages/angular/build/src/builders/extract-i18n/builder.ts @@ -64,9 +64,8 @@ export async function execute( extensions, ); - // Return the builder result if it failed - if (!extractionResult.builderResult.success) { - return extractionResult.builderResult; + if (!extractionResult.success) { + return { success: false }; } // Perform duplicate message checks diff --git a/packages/angular/build/src/private.ts b/packages/angular/build/src/private.ts index 746747d44db2..2f2fea08b385 100644 --- a/packages/angular/build/src/private.ts +++ b/packages/angular/build/src/private.ts @@ -16,6 +16,7 @@ // Builders export { buildApplicationInternal } from './builders/application'; export type { ApplicationBuilderInternalOptions } from './builders/application/options'; +export { type Result, type ResultFile, ResultKind } from './builders/application/results'; export { serveWithVite } from './builders/dev-server/vite-server'; // Tools diff --git a/packages/angular/build/src/tools/esbuild/bundler-execution-result.ts b/packages/angular/build/src/tools/esbuild/bundler-execution-result.ts index 5fd41064103f..ae2642e51d7d 100644 --- a/packages/angular/build/src/tools/esbuild/bundler-execution-result.ts +++ b/packages/angular/build/src/tools/esbuild/bundler-execution-result.ts @@ -43,6 +43,8 @@ export class ExecutionResult { logs: string[] = []; externalMetadata?: ExternalResultMetadata; extraWatchFiles: string[] = []; + htmlIndexPath?: string; + htmlBaseHref?: string; constructor( private rebuildContexts: BundlerContext[], diff --git a/packages/angular_devkit/build_angular/src/builders/browser-esbuild/index.ts b/packages/angular_devkit/build_angular/src/builders/browser-esbuild/index.ts index af935d770328..6bb9142c900e 100644 --- a/packages/angular_devkit/build_angular/src/builders/browser-esbuild/index.ts +++ b/packages/angular_devkit/build_angular/src/builders/browser-esbuild/index.ts @@ -6,9 +6,15 @@ * found in the LICENSE file at https://angular.dev/license */ -import type { ApplicationBuilderOptions, BuildOutputAsset, BuildOutputFile } from '@angular/build'; -import { buildApplicationInternal, deleteOutputDir, emitFilesToDisk } from '@angular/build/private'; -import { BuilderContext, BuilderOutput, createBuilder } from '@angular-devkit/architect'; +import type { ApplicationBuilderOptions } from '@angular/build'; +import { + Result, + ResultKind, + buildApplicationInternal, + deleteOutputDir, + emitFilesToDisk, +} from '@angular/build/private'; +import { BuilderContext, createBuilder } from '@angular-devkit/architect'; import type { Plugin } from 'esbuild'; import fs from 'node:fs/promises'; import path from 'node:path'; @@ -31,12 +37,7 @@ export async function* buildEsbuildBrowser( write?: boolean; }, plugins?: Plugin[], -): AsyncIterable< - BuilderOutput & { - outputFiles?: BuildOutputFile[]; - assetFiles?: { source: string; destination: string }[]; - } -> { +): AsyncIterable { // Inform user of status of builder and options logBuilderStatusWarnings(userOptions, context); const normalizedOptions = normalizeOptions(userOptions); @@ -55,21 +56,37 @@ export async function* buildEsbuildBrowser( }, plugins && { codePlugins: plugins }, )) { - if (infrastructureSettings?.write !== false && result.outputFiles) { - // Write output files - await writeResultFiles(result.outputFiles, result.assetFiles, fullOutputPath); + // Write the file directly from this builder to maintain webpack output compatibility + // and not output browser files into '/browser'. + if ( + infrastructureSettings?.write !== false && + (result.kind === ResultKind.Full || result.kind === ResultKind.Incremental) + ) { + const directoryExists = new Set(); + // Writes the output file to disk and ensures the containing directories are present + await emitFilesToDisk(Object.entries(result.files), async ([filePath, file]) => { + // Ensure output subdirectories exist + const basePath = path.dirname(filePath); + if (basePath && !directoryExists.has(basePath)) { + await fs.mkdir(path.join(fullOutputPath, basePath), { recursive: true }); + directoryExists.add(basePath); + } + + if (file.origin === 'memory') { + // Write file contents + await fs.writeFile(path.join(fullOutputPath, filePath), file.contents); + } else { + // Copy file contents + await fs.copyFile( + file.inputPath, + path.join(fullOutputPath, filePath), + fs.constants.COPYFILE_FICLONE, + ); + } + }); } - // The builder system (architect) currently attempts to treat all results as JSON and - // attempts to validate the object with a JSON schema validator. This can lead to slow - // build completion (even after the actual build is fully complete) or crashes if the - // size and/or quantity of output files is large. Architect only requires a `success` - // property so that is all that will be passed here if the infrastructure settings have - // not been explicitly set to avoid writes. Writing is only disabled when used directly - // by the dev server which bypasses the architect behavior. - const builderResult = - infrastructureSettings?.write === false ? result : { success: result.success }; - yield builderResult; + yield result; } } @@ -97,42 +114,13 @@ function normalizeOptions( }; } -// We write the file directly from this builder to maintain webpack output compatibility -// and not output browser files into '/browser'. -async function writeResultFiles( - outputFiles: BuildOutputFile[], - assetFiles: BuildOutputAsset[] | undefined, - outputPath: string, +export async function* buildEsbuildBrowserArchitect( + options: BrowserBuilderOptions, + context: BuilderContext, ) { - const directoryExists = new Set(); - const ensureDirectoryExists = async (basePath: string) => { - if (basePath && !directoryExists.has(basePath)) { - await fs.mkdir(path.join(outputPath, basePath), { recursive: true }); - directoryExists.add(basePath); - } - }; - - // Writes the output file to disk and ensures the containing directories are present - await emitFilesToDisk(outputFiles, async (file: BuildOutputFile) => { - // Ensure output subdirectories exist - const basePath = path.dirname(file.path); - await ensureDirectoryExists(basePath); - - // Write file contents - await fs.writeFile(path.join(outputPath, file.path), file.contents); - }); - - if (assetFiles?.length) { - await emitFilesToDisk(assetFiles, async ({ source, destination }) => { - const basePath = path.dirname(destination); - - // Ensure output subdirectories exist - await ensureDirectoryExists(basePath); - - // Copy file contents - await fs.copyFile(source, path.join(outputPath, destination), fs.constants.COPYFILE_FICLONE); - }); + for await (const result of buildEsbuildBrowser(options, context)) { + yield { success: result.kind !== ResultKind.Failure }; } } -export default createBuilder(buildEsbuildBrowser); +export default createBuilder(buildEsbuildBrowserArchitect); diff --git a/packages/angular_devkit/build_angular/src/builders/browser-esbuild/tests/options/assets_spec.ts b/packages/angular_devkit/build_angular/src/builders/browser-esbuild/tests/options/assets_spec.ts index 740612d19478..62dec5a44fc6 100644 --- a/packages/angular_devkit/build_angular/src/builders/browser-esbuild/tests/options/assets_spec.ts +++ b/packages/angular_devkit/build_angular/src/builders/browser-esbuild/tests/options/assets_spec.ts @@ -6,10 +6,10 @@ * found in the LICENSE file at https://angular.dev/license */ -import { buildEsbuildBrowser } from '../../index'; +import { buildEsbuildBrowserArchitect } from '../../index'; import { BASE_OPTIONS, BROWSER_BUILDER_INFO, describeBuilder } from '../setup'; -describeBuilder(buildEsbuildBrowser, BROWSER_BUILDER_INFO, (harness) => { +describeBuilder(buildEsbuildBrowserArchitect, BROWSER_BUILDER_INFO, (harness) => { describe('Option: "assets"', () => { beforeEach(async () => { // Application code is not needed for asset tests diff --git a/packages/angular_devkit/build_angular/src/builders/browser-esbuild/tests/options/delete-output-path_spec.ts b/packages/angular_devkit/build_angular/src/builders/browser-esbuild/tests/options/delete-output-path_spec.ts index 5a16d020555d..e4ceda3f4828 100644 --- a/packages/angular_devkit/build_angular/src/builders/browser-esbuild/tests/options/delete-output-path_spec.ts +++ b/packages/angular_devkit/build_angular/src/builders/browser-esbuild/tests/options/delete-output-path_spec.ts @@ -6,10 +6,10 @@ * found in the LICENSE file at https://angular.dev/license */ -import { buildEsbuildBrowser } from '../../index'; +import { buildEsbuildBrowserArchitect } from '../../index'; import { BASE_OPTIONS, BROWSER_BUILDER_INFO, describeBuilder } from '../setup'; -describeBuilder(buildEsbuildBrowser, BROWSER_BUILDER_INFO, (harness) => { +describeBuilder(buildEsbuildBrowserArchitect, BROWSER_BUILDER_INFO, (harness) => { describe('Option: "deleteOutputPath"', () => { beforeEach(async () => { // Application code is not needed for asset tests diff --git a/packages/angular_devkit/build_angular/src/builders/browser-esbuild/tests/options/deploy-url_spec.ts b/packages/angular_devkit/build_angular/src/builders/browser-esbuild/tests/options/deploy-url_spec.ts index 4c3629a400e4..692d7e684636 100644 --- a/packages/angular_devkit/build_angular/src/builders/browser-esbuild/tests/options/deploy-url_spec.ts +++ b/packages/angular_devkit/build_angular/src/builders/browser-esbuild/tests/options/deploy-url_spec.ts @@ -6,10 +6,10 @@ * found in the LICENSE file at https://angular.dev/license */ -import { buildEsbuildBrowser } from '../../index'; +import { buildEsbuildBrowserArchitect } from '../../index'; import { BASE_OPTIONS, BROWSER_BUILDER_INFO, describeBuilder } from '../setup'; -describeBuilder(buildEsbuildBrowser, BROWSER_BUILDER_INFO, (harness) => { +describeBuilder(buildEsbuildBrowserArchitect, BROWSER_BUILDER_INFO, (harness) => { describe('Option: "deployUrl"', () => { beforeEach(async () => { // Add a global stylesheet to test link elements diff --git a/packages/angular_devkit/build_angular/src/builders/browser-esbuild/tests/options/main_spec.ts b/packages/angular_devkit/build_angular/src/builders/browser-esbuild/tests/options/main_spec.ts index 1111ed01554a..c3af469b7d75 100644 --- a/packages/angular_devkit/build_angular/src/builders/browser-esbuild/tests/options/main_spec.ts +++ b/packages/angular_devkit/build_angular/src/builders/browser-esbuild/tests/options/main_spec.ts @@ -6,10 +6,10 @@ * found in the LICENSE file at https://angular.dev/license */ -import { buildEsbuildBrowser } from '../../index'; +import { buildEsbuildBrowserArchitect } from '../../index'; import { BASE_OPTIONS, BROWSER_BUILDER_INFO, describeBuilder } from '../setup'; -describeBuilder(buildEsbuildBrowser, BROWSER_BUILDER_INFO, (harness) => { +describeBuilder(buildEsbuildBrowserArchitect, BROWSER_BUILDER_INFO, (harness) => { describe('Option: "main"', () => { it('uses a provided TypeScript file', async () => { harness.useTarget('build', { diff --git a/packages/angular_devkit/build_angular/src/builders/extract-i18n/application-extraction.ts b/packages/angular_devkit/build_angular/src/builders/extract-i18n/application-extraction.ts index e720d8f7543f..7a7daaa8af15 100644 --- a/packages/angular_devkit/build_angular/src/builders/extract-i18n/application-extraction.ts +++ b/packages/angular_devkit/build_angular/src/builders/extract-i18n/application-extraction.ts @@ -8,12 +8,13 @@ import { type ApplicationBuilderInternalOptions, + ResultFile, + ResultKind, buildApplicationInternal, } from '@angular/build/private'; import type { ɵParsedMessage as LocalizeMessage } from '@angular/localize'; import type { MessageExtractor } from '@angular/localize/tools'; -import type { BuilderContext, BuilderOutput } from '@angular-devkit/architect'; -import assert from 'node:assert'; +import type { BuilderContext } from '@angular-devkit/architect'; import nodePath from 'node:path'; import { buildEsbuildBrowser } from '../browser-esbuild'; import type { NormalizedExtractI18nOptions } from './options'; @@ -24,7 +25,7 @@ export async function extractMessages( context: BuilderContext, extractorConstructor: typeof MessageExtractor, ): Promise<{ - builderResult: BuilderOutput; + success: boolean; basePath: string; messages: LocalizeMessage[]; useLegacyIds: boolean; @@ -54,41 +55,21 @@ export async function extractMessages( build = buildEsbuildBrowser; } - // Build the application with the build options - let builderResult; - try { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - for await (const result of build(buildOptions as any, context, { write: false })) { - builderResult = result; - break; - } - - assert(builderResult !== undefined, 'Application builder did not provide a result.'); - } catch (err) { - builderResult = { - success: false, - error: (err as Error).message, - }; - } - - // Extract messages from each output JavaScript file. - // Output files are only present on a successful build. - if (builderResult.outputFiles) { - // Store the JS and JS map files for lookup during extraction - const files = new Map(); - for (const outputFile of builderResult.outputFiles) { - if (outputFile.path.endsWith('.js')) { - files.set(outputFile.path, outputFile.text); - } else if (outputFile.path.endsWith('.js.map')) { - files.set(outputFile.path, outputFile.text); - } - } + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const builderResult = await first(build(buildOptions as any, context, { write: false })); + let success = false; + if (!builderResult || builderResult.kind === ResultKind.Failure) { + context.logger.error('Application build failed.'); + } else if (builderResult.kind !== ResultKind.Full) { + context.logger.error('Application build did not provide a full output.'); + } else { // Setup the localize message extractor based on the in-memory files - const extractor = setupLocalizeExtractor(extractorConstructor, files, context); + const extractor = setupLocalizeExtractor(extractorConstructor, builderResult.files, context); - // Attempt extraction of all output JS files - for (const filePath of files.keys()) { + // Extract messages from each output JavaScript file. + // Output files are only present on a successful build. + for (const filePath of Object.keys(builderResult.files)) { if (!filePath.endsWith('.js')) { continue; } @@ -96,10 +77,12 @@ export async function extractMessages( const fileMessages = extractor.extractMessages(filePath); messages.push(...fileMessages); } + + success = true; } return { - builderResult, + success, basePath: context.workspaceRoot, messages, // Legacy i18n identifiers are not supported with the new application builder @@ -109,9 +92,10 @@ export async function extractMessages( function setupLocalizeExtractor( extractorConstructor: typeof MessageExtractor, - files: Map, + files: Record, context: BuilderContext, ): MessageExtractor { + const textDecoder = new TextDecoder(); // Setup a virtual file system instance for the extractor // * MessageExtractor itself uses readFile, relative and resolve // * Internal SourceFileLoader (sourcemap support) uses dirname, exists, readFile, and resolve @@ -120,7 +104,11 @@ function setupLocalizeExtractor( // Output files are stored as relative to the workspace root const requestedPath = nodePath.relative(context.workspaceRoot, path); - const content = files.get(requestedPath); + const file = files[requestedPath]; + let content; + if (file?.origin === 'memory') { + content = textDecoder.decode(file.contents); + } if (content === undefined) { throw new Error('Unknown file requested: ' + requestedPath); } @@ -137,7 +125,7 @@ function setupLocalizeExtractor( // Output files are stored as relative to the workspace root const requestedPath = nodePath.relative(context.workspaceRoot, path); - return files.has(requestedPath); + return files[requestedPath] !== undefined; }, dirname(path: string): string { return nodePath.dirname(path); @@ -171,3 +159,9 @@ function setupLocalizeExtractor( return extractor; } + +async function first(iterable: AsyncIterable): Promise { + for await (const value of iterable) { + return value; + } +} diff --git a/packages/angular_devkit/build_angular/src/builders/extract-i18n/builder.ts b/packages/angular_devkit/build_angular/src/builders/extract-i18n/builder.ts index e115ada85c04..81edd08febf0 100644 --- a/packages/angular_devkit/build_angular/src/builders/extract-i18n/builder.ts +++ b/packages/angular_devkit/build_angular/src/builders/extract-i18n/builder.ts @@ -70,6 +70,10 @@ export async function execute( context, localizeToolsModule.MessageExtractor, ); + + if (!extractionResult.success) { + return { success: false }; + } } else { // Purge old build disk cache. // Other build systems handle stale cache purging directly. @@ -77,11 +81,11 @@ export async function execute( const { extractMessages } = await import('./webpack-extraction'); extractionResult = await extractMessages(normalizedOptions, builderName, context, transforms); - } - // Return the builder result if it failed - if (!extractionResult.builderResult.success) { - return extractionResult.builderResult; + // Return the builder result if it failed + if (!extractionResult.builderResult.success) { + return extractionResult.builderResult; + } } // Perform duplicate message checks diff --git a/packages/angular_devkit/build_angular/src/builders/web-test-runner/index.ts b/packages/angular_devkit/build_angular/src/builders/web-test-runner/index.ts index c7f48be47664..70abef478fc0 100644 --- a/packages/angular_devkit/build_angular/src/builders/web-test-runner/index.ts +++ b/packages/angular_devkit/build_angular/src/builders/web-test-runner/index.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.dev/license */ -import { buildApplicationInternal } from '@angular/build/private'; +import { Result, ResultKind, buildApplicationInternal } from '@angular/build/private'; import { BuilderContext, BuilderOutput, createBuilder } from '@angular-devkit/architect'; import type * as WebTestRunner from '@web/test-runner'; import { promises as fs } from 'node:fs'; @@ -53,8 +53,8 @@ export default createBuilder( // Build the tests and abort on any build failure. const buildOutput = await buildTests(testFiles, testDir, options, ctx); - if (!buildOutput.success) { - return buildOutput; + if (buildOutput.kind === ResultKind.Failure) { + return { success: false }; } // Run the built tests. @@ -68,7 +68,7 @@ async function buildTests( outputPath: string, options: WtrBuilderOptions, ctx: BuilderContext, -): Promise { +): Promise { const entryPoints = new Set([ ...testFiles, 'jasmine-core/lib/jasmine-core/jasmine.js',