refactor(@angular-devkit/build-angular): reduce custom code in browser-esbuild implementation

clydin · clydin · commit 7af63b4aba32 · 2024-08-13T07:54:24.000-04:00
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.
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
@@ -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<ApplicationBuilderOptions['outputPath'], string | undefined>;
 
 /**
@@ -37,57 +30,15 @@ export async function* buildEsbuildBrowser(
     write?: boolean;
   },
   plugins?: Plugin[],
-): AsyncIterable<Result> {
+): AsyncIterable<BuilderOutput> {
   // 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<string>();
-      // 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<ApplicationBuilderOptions, 'outputPath'> & { 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<BrowserBuilderOptions>(buildEsbuildBrowserArchitect);
+export default createBuilder<BrowserBuilderOptions>(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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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) {
