perf(@angular-devkit/build-angular): optimize server or browser only dependencies once

This commit splits the retrieval of external dependencies into two. Server imports and browser imports. This is so that we avoid vite from optimizing server or browser only dependencies twice.

This also fixes an issue were in some cases Vite would issue a warning like `Cannot optimize dependency: path, present in 'optimizeDeps.include'`. This was caused because of server only dependencies ended up trying to be optimized for a browser build.

(cherry picked from commit 5990875)

Author: alan-agius4

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

@@ -169,11 +169,9 @@ export async function executeBuild(
 
   // Analyze external imports if external options are enabled
   if (options.externalPackages || options.externalDependencies?.length) {
+    const { browser = new Set(), server = new Set() } = bundlingResult.externalImports;
     // TODO: Filter externalImports to generate second argument to support wildcard externalDependency values
-    executionResult.setExternalMetadata(
-      [...bundlingResult.externalImports],
-      options.externalDependencies,
-    );
+    executionResult.setExternalMetadata([...browser], [...server], options.externalDependencies);
   }
 
   const { metafile, initialFiles, outputFiles } = bundlingResult;

packages/angular_devkit/build_angular/src/builders/dev-server/vite-server.ts

@@ -18,6 +18,7 @@ import { ServerResponse } from 'node:http';
 import { dirname, extname, join, relative } from 'node:path';
 import type { Connect, DepOptimizationConfig, InlineConfig, ViteDevServer } from 'vite';
 import { BuildOutputFile, BuildOutputFileType } from '../../tools/esbuild/bundler-context';
+import { ExternalResultMetadata } from '../../tools/esbuild/bundler-execution-result';
 import { JavaScriptTransformer } from '../../tools/esbuild/javascript-transformer';
 import { createRxjsEsmResolutionPlugin } from '../../tools/esbuild/rxjs-esm-resolution-plugin';
 import { getFeatureSupport, transformSupportedBrowsersToTargets } from '../../tools/esbuild/utils';
@@ -123,8 +124,9 @@ export async function* serveWithVite(
   let hadError = false;
   const generatedFiles = new Map<string, OutputFileRecord>();
   const assetFiles = new Map<string, string>();
-  const externalMetadata: { implicit: string[]; explicit: string[] } = {
-    implicit: [],
+  const externalMetadata: ExternalResultMetadata = {
+    implicitBrowser: [],
+    implicitServer: [],
     explicit: [],
   };
   const build =
@@ -178,17 +180,12 @@ export async function* serveWithVite(
       }
     }
 
-    // To avoid disconnecting the array objects from the option, these arrays need to be mutated
-    // instead of replaced.
-    // TODO: split explicit imports by platform to avoid having Vite optimize server-only/browser-only
-    // dependencies twice when SSR is enabled.
+    // To avoid disconnecting the array objects from the option, these arrays need to be mutated instead of replaced.
     if (result.externalMetadata) {
-      if (result.externalMetadata.explicit) {
-        externalMetadata.explicit.push(...result.externalMetadata.explicit);
-      }
-      if (result.externalMetadata.implicit) {
-        externalMetadata.implicit.push(...result.externalMetadata.implicit);
-      }
+      const { implicitBrowser, implicitServer, explicit } = result.externalMetadata;
+      externalMetadata.explicit.push(...explicit);
+      externalMetadata.implicitServer.push(...implicitServer);
+      externalMetadata.implicitBrowser.push(...implicitBrowser);
     }
 
     if (server) {
@@ -371,7 +368,7 @@ export async function setupServer(
   outputFiles: Map<string, OutputFileRecord>,
   assets: Map<string, string>,
   preserveSymlinks: boolean | undefined,
-  externalMetadata: { implicit: string[]; explicit: string[] },
+  externalMetadata: ExternalResultMetadata,
   ssr: boolean,
   prebundleTransformer: JavaScriptTransformer,
   target: string[],
@@ -437,7 +434,7 @@ export async function setupServer(
         // Exclude any explicitly defined dependencies (currently build defined externals)
         exclude: externalMetadata.explicit,
         // Include all implict dependencies from the external packages internal option
-        include: externalMetadata.implicit,
+        include: externalMetadata.implicitServer,
         ssr: true,
         prebundleTransformer,
         target,
@@ -698,7 +695,7 @@ export async function setupServer(
       // Exclude any explicitly defined dependencies (currently build defined externals)
       exclude: externalMetadata.explicit,
       // Include all implict dependencies from the external packages internal option
-      include: externalMetadata.implicit,
+      include: externalMetadata.implicitBrowser,
       ssr: false,
       prebundleTransformer,
       target,

packages/angular_devkit/build_angular/src/tools/esbuild/bundler-context.ts

@@ -29,7 +29,10 @@ export type BundleContextResult =
       metafile: Metafile;
       outputFiles: BuildOutputFile[];
       initialFiles: Map<string, InitialFileRecord>;
-      externalImports: Set<string>;
+      externalImports: {
+        server?: Set<string>;
+        browser?: Set<string>;
+      };
     };
 
 export interface InitialFileRecord {
@@ -117,7 +120,9 @@ export class BundlerContext {
     const warnings: Message[] = [];
     const metafile: Metafile = { inputs: {}, outputs: {} };
     const initialFiles = new Map<string, InitialFileRecord>();
-    const externalImports = new Set<string>();
+    const externalImportsBrowser = new Set<string>();
+    const externalImportsServer = new Set<string>();
+
     const outputFiles = [];
     for (const result of individualResults) {
       warnings.push(...result.warnings);
@@ -135,7 +140,8 @@ export class BundlerContext {
 
       result.initialFiles.forEach((value, key) => initialFiles.set(key, value));
       outputFiles.push(...result.outputFiles);
-      result.externalImports.forEach((value) => externalImports.add(value));
+      result.externalImports.browser?.forEach((value) => externalImportsBrowser.add(value));
+      result.externalImports.server?.forEach((value) => externalImportsServer.add(value));
     }
 
     if (errors !== undefined) {
@@ -148,7 +154,10 @@ export class BundlerContext {
       metafile,
       initialFiles,
       outputFiles,
-      externalImports,
+      externalImports: {
+        browser: externalImportsBrowser,
+        server: externalImportsServer,
+      },
     };
   }
 
@@ -175,7 +184,7 @@ export class BundlerContext {
     return result;
   }
 
-  async #performBundle() {
+  async #performBundle(): Promise<BundleContextResult> {
     // Create esbuild options if not present
     if (this.#esbuildOptions === undefined) {
       if (this.incremental) {
@@ -313,15 +322,13 @@ export class BundlerContext {
       }
     }
 
+    const platformIsServer = this.#esbuildOptions?.platform === 'node';
     const outputFiles = result.outputFiles.map((file) => {
       let fileType: BuildOutputFileType;
       if (dirname(file.path) === 'media') {
         fileType = BuildOutputFileType.Media;
       } else {
-        fileType =
-          this.#esbuildOptions?.platform === 'node'
-            ? BuildOutputFileType.Server
-            : BuildOutputFileType.Browser;
+        fileType = platformIsServer ? BuildOutputFileType.Server : BuildOutputFileType.Browser;
       }
 
       return convertOutputFile(file, fileType);
@@ -332,7 +339,9 @@ export class BundlerContext {
       ...result,
       outputFiles,
       initialFiles,
-      externalImports,
+      externalImports: {
+        [platformIsServer ? 'server' : 'browser']: externalImports,
+      },
       errors: undefined,
     };
   }

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

@@ -24,14 +24,20 @@ export interface RebuildState {
   previousOutputHashes: Map<string, string>;
 }
 
+export interface ExternalResultMetadata {
+  implicitBrowser: string[];
+  implicitServer: string[];
+  explicit: string[];
+}
+
 /**
  * Represents the result of a single builder execute call.
  */
 export class ExecutionResult {
   outputFiles: BuildOutputFile[] = [];
   assetFiles: BuildOutputAsset[] = [];
   errors: (Message | PartialMessage)[] = [];
-  externalMetadata?: { implicit: string[]; explicit?: string[] };
+  externalMetadata?: ExternalResultMetadata;
 
   constructor(
     private rebuildContexts: BundlerContext[],
@@ -53,11 +59,16 @@ export class ExecutionResult {
   /**
    * Add external JavaScript import metadata to the result. This is currently used
    * by the development server to optimize the prebundling process.
-   * @param implicit External dependencies due to the external packages option.
+   * @param implicitBrowser External dependencies for the browser bundles due to the external packages option.
+   * @param implicitServer External dependencies for the server bundles due to the external packages option.
    * @param explicit External dependencies due to explicit project configuration.
    */
-  setExternalMetadata(implicit: string[], explicit: string[] | undefined) {
-    this.externalMetadata = { implicit, explicit };
+  setExternalMetadata(
+    implicitBrowser: string[],
+    implicitServer: string[],
+    explicit: string[] | undefined,
+  ): void {
+    this.externalMetadata = { implicitBrowser, implicitServer, explicit: explicit ?? [] };
   }
 
   get output() {