From d14c9ab0d91411fd20c216d1e27b0233ae48e877 Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Mon, 12 Aug 2024 11:39:20 -0400 Subject: [PATCH] refactor(@angular-devkit/build-angular): reduce custom code in browser-esbuild implementation The implementation of the `browser-esbuild` builder is now a small wrapper around the `application` builder. The custom file writing code is no longer required with the availability of the additional output path options for `application` builder. This also allows the internal `browser-esbuild` programmatic interface to retain its architect-based signature. (cherry picked from commit 7af63b4aba32e9a715a67b43f756d04cdc65d251) --- .../src/builders/browser-esbuild/index.ts | 79 +++---------------- .../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 +- .../src/builders/dev-server/builder.ts | 6 +- .../extract-i18n/application-extraction.ts | 46 ++++++----- 7 files changed, 46 insertions(+), 101 deletions(-) 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 6a595e43aa1f..6fcc8feb1535 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,21 +6,14 @@ * found in the LICENSE file at https://angular.dev/license */ -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 ApplicationBuilderOptions, buildApplication } from '@angular/build'; +import { BuilderContext, BuilderOutput, createBuilder } from '@angular-devkit/architect'; import type { Plugin } from 'esbuild'; -import fs from 'node:fs/promises'; -import path from 'node:path'; import { logBuilderStatusWarnings } from './builder-status-warnings'; import type { Schema as BrowserBuilderOptions } from './schema'; +export type { BrowserBuilderOptions }; + type OutputPathClass = Exclude; /** @@ -37,57 +30,15 @@ export async function* buildEsbuildBrowser( write?: boolean; }, plugins?: Plugin[], -): AsyncIterable { +): AsyncIterable { // Inform user of status of builder and options logBuilderStatusWarnings(userOptions, context); - const normalizedOptions = normalizeOptions(userOptions); - const { deleteOutputPath, outputPath } = normalizedOptions; - const fullOutputPath = path.join(context.workspaceRoot, outputPath.base); - - if (deleteOutputPath && infrastructureSettings?.write !== false) { - await deleteOutputDir(context.workspaceRoot, outputPath.base); - } - - for await (const result of buildApplicationInternal( - normalizedOptions, - context, - plugins && { codePlugins: plugins }, - )) { - // 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, - ); - } - }); - } - yield result; - } + const normalizedOptions = convertBrowserOptions(userOptions); + yield* buildApplication(normalizedOptions, context, { codePlugins: plugins }); } -function normalizeOptions( +export function convertBrowserOptions( options: BrowserBuilderOptions, ): Omit & { outputPath: OutputPathClass } { const { @@ -96,6 +47,7 @@ function normalizeOptions( ngswConfigPath, serviceWorker, polyfills, + resourcesOutputPath, ...otherOptions } = options; @@ -106,18 +58,11 @@ function normalizeOptions( outputPath: { base: outputPath, browser: '', + server: '', + media: resourcesOutputPath ?? 'media', }, ...otherOptions, }; } -export async function* buildEsbuildBrowserArchitect( - options: BrowserBuilderOptions, - context: BuilderContext, -) { - for await (const result of buildEsbuildBrowser(options, context)) { - yield { success: result.kind !== ResultKind.Failure }; - } -} - -export default createBuilder(buildEsbuildBrowserArchitect); +export default createBuilder(buildEsbuildBrowser); 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 62dec5a44fc6..740612d19478 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 { buildEsbuildBrowserArchitect } from '../../index'; +import { buildEsbuildBrowser } from '../../index'; import { BASE_OPTIONS, BROWSER_BUILDER_INFO, describeBuilder } from '../setup'; -describeBuilder(buildEsbuildBrowserArchitect, BROWSER_BUILDER_INFO, (harness) => { +describeBuilder(buildEsbuildBrowser, 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 e4ceda3f4828..5a16d020555d 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 { buildEsbuildBrowserArchitect } from '../../index'; +import { buildEsbuildBrowser } from '../../index'; import { BASE_OPTIONS, BROWSER_BUILDER_INFO, describeBuilder } from '../setup'; -describeBuilder(buildEsbuildBrowserArchitect, BROWSER_BUILDER_INFO, (harness) => { +describeBuilder(buildEsbuildBrowser, 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 692d7e684636..4c3629a400e4 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 { buildEsbuildBrowserArchitect } from '../../index'; +import { buildEsbuildBrowser } from '../../index'; import { BASE_OPTIONS, BROWSER_BUILDER_INFO, describeBuilder } from '../setup'; -describeBuilder(buildEsbuildBrowserArchitect, BROWSER_BUILDER_INFO, (harness) => { +describeBuilder(buildEsbuildBrowser, 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 c3af469b7d75..1111ed01554a 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 { buildEsbuildBrowserArchitect } from '../../index'; +import { buildEsbuildBrowser } from '../../index'; import { BASE_OPTIONS, BROWSER_BUILDER_INFO, describeBuilder } from '../setup'; -describeBuilder(buildEsbuildBrowserArchitect, BROWSER_BUILDER_INFO, (harness) => { +describeBuilder(buildEsbuildBrowser, 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/dev-server/builder.ts b/packages/angular_devkit/build_angular/src/builders/dev-server/builder.ts index 37a1502200a8..3b244a008c2c 100644 --- a/packages/angular_devkit/build_angular/src/builders/dev-server/builder.ts +++ b/packages/angular_devkit/build_angular/src/builders/dev-server/builder.ts @@ -88,14 +88,16 @@ export function execute( return defer(() => Promise.all([import('@angular/build/private'), import('../browser-esbuild')]), ).pipe( - switchMap(([{ serveWithVite, buildApplicationInternal }, { buildEsbuildBrowser }]) => + switchMap(([{ serveWithVite, buildApplicationInternal }, { convertBrowserOptions }]) => serveWithVite( normalizedOptions, builderName, (options, context, codePlugins) => { return builderName === '@angular-devkit/build-angular:browser-esbuild' ? // eslint-disable-next-line @typescript-eslint/no-explicit-any - buildEsbuildBrowser(options as any, context, { write: false }, codePlugins) + buildApplicationInternal(convertBrowserOptions(options as any), context, { + codePlugins, + }) : buildApplicationInternal(options, context, { codePlugins }); }, context, 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 7a7daaa8af15..4e89fad52fd1 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 @@ -6,17 +6,13 @@ * found in the LICENSE file at https://angular.dev/license */ -import { - type ApplicationBuilderInternalOptions, - ResultFile, - ResultKind, - buildApplicationInternal, -} from '@angular/build/private'; +import type { ApplicationBuilderOptions } from '@angular/build'; +import { 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 } from '@angular-devkit/architect'; import nodePath from 'node:path'; -import { buildEsbuildBrowser } from '../browser-esbuild'; +import { BrowserBuilderOptions, convertBrowserOptions } from '../browser-esbuild'; import type { NormalizedExtractI18nOptions } from './options'; export async function extractMessages( @@ -33,30 +29,32 @@ export async function extractMessages( const messages: LocalizeMessage[] = []; // Setup the build options for the application based on the buildTarget option - const buildOptions = (await context.validateOptions( - await context.getTargetOptions(options.buildTarget), - builderName, - )) as unknown as ApplicationBuilderInternalOptions; + let buildOptions; + if (builderName === '@angular-devkit/build-angular:application') { + buildOptions = (await context.validateOptions( + await context.getTargetOptions(options.buildTarget), + builderName, + )) as unknown as ApplicationBuilderOptions; + } else { + buildOptions = convertBrowserOptions( + (await context.validateOptions( + await context.getTargetOptions(options.buildTarget), + builderName, + )) as unknown as BrowserBuilderOptions, + ); + } + buildOptions.optimization = false; buildOptions.sourceMap = { scripts: true, vendor: true, styles: false }; buildOptions.localize = false; buildOptions.budgets = undefined; buildOptions.index = false; buildOptions.serviceWorker = false; + buildOptions.ssr = false; + buildOptions.appShell = false; + buildOptions.prerender = false; - let build; - if (builderName === '@angular-devkit/build-angular:application') { - build = buildApplicationInternal; - - buildOptions.ssr = false; - buildOptions.appShell = false; - buildOptions.prerender = false; - } else { - build = buildEsbuildBrowser; - } - - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const builderResult = await first(build(buildOptions as any, context, { write: false })); + const builderResult = await first(buildApplicationInternal(buildOptions, context)); let success = false; if (!builderResult || builderResult.kind === ResultKind.Failure) {