Skip to content

Commit c987b2a

Browse files
committed
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.
1 parent 5f14bf3 commit c987b2a

File tree

4 files changed

+48
-33
lines changed

4 files changed

+48
-33
lines changed

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -169,11 +169,9 @@ export async function executeBuild(
169169

170170
// Analyze external imports if external options are enabled
171171
if (options.externalPackages || options.externalDependencies?.length) {
172+
const { browser = new Set(), server = new Set() } = bundlingResult.externalImports;
172173
// TODO: Filter externalImports to generate second argument to support wildcard externalDependency values
173-
executionResult.setExternalMetadata(
174-
[...bundlingResult.externalImports],
175-
options.externalDependencies,
176-
);
174+
executionResult.setExternalMetadata([...browser], [...server], options.externalDependencies);
177175
}
178176

179177
const { metafile, initialFiles, outputFiles } = bundlingResult;

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

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import { ServerResponse } from 'node:http';
1818
import path from 'node:path';
1919
import type { Connect, DepOptimizationConfig, InlineConfig, ViteDevServer } from 'vite';
2020
import { BuildOutputFile, BuildOutputFileType } from '../../tools/esbuild/bundler-context';
21+
import { ExecutionResultMetadata } from '../../tools/esbuild/bundler-execution-result';
2122
import { JavaScriptTransformer } from '../../tools/esbuild/javascript-transformer';
2223
import { createRxjsEsmResolutionPlugin } from '../../tools/esbuild/rxjs-esm-resolution-plugin';
2324
import { getFeatureSupport, transformSupportedBrowsersToTargets } from '../../tools/esbuild/utils';
@@ -116,8 +117,9 @@ export async function* serveWithVite(
116117
let hadError = false;
117118
const generatedFiles = new Map<string, OutputFileRecord>();
118119
const assetFiles = new Map<string, string>();
119-
const externalMetadata: { implicit: string[]; explicit: string[] } = {
120-
implicit: [],
120+
const externalMetadata: ExecutionResultMetadata = {
121+
implicitBrowser: [],
122+
implicitServer: [],
121123
explicit: [],
122124
};
123125
const build =
@@ -171,17 +173,12 @@ export async function* serveWithVite(
171173
}
172174
}
173175

174-
// To avoid disconnecting the array objects from the option, these arrays need to be mutated
175-
// instead of replaced.
176-
// TODO: split explicit imports by platform to avoid having Vite optimize server-only/browser-only
177-
// dependencies twice when SSR is enabled.
176+
// To avoid disconnecting the array objects from the option, these arrays need to be mutated instead of replaced.
178177
if (result.externalMetadata) {
179-
if (result.externalMetadata.explicit) {
180-
externalMetadata.explicit.push(...result.externalMetadata.explicit);
181-
}
182-
if (result.externalMetadata.implicit) {
183-
externalMetadata.implicit.push(...result.externalMetadata.implicit);
184-
}
178+
const { implicitBrowser, implicitServer, explicit } = result.externalMetadata;
179+
externalMetadata.explicit.push(...explicit);
180+
externalMetadata.implicitServer.push(...implicitServer);
181+
externalMetadata.implicitBrowser.push(...implicitBrowser);
185182
}
186183

187184
if (server) {
@@ -362,7 +359,7 @@ export async function setupServer(
362359
outputFiles: Map<string, OutputFileRecord>,
363360
assets: Map<string, string>,
364361
preserveSymlinks: boolean | undefined,
365-
externalMetadata: { implicit: string[]; explicit: string[] },
362+
externalMetadata: ExecutionResultMetadata,
366363
ssr: boolean,
367364
prebundleTransformer: JavaScriptTransformer,
368365
target: string[],
@@ -426,7 +423,7 @@ export async function setupServer(
426423
// Exclude any explicitly defined dependencies (currently build defined externals)
427424
exclude: externalMetadata.explicit,
428425
// Include all implict dependencies from the external packages internal option
429-
include: externalMetadata.implicit,
426+
include: externalMetadata.implicitServer,
430427
ssr: true,
431428
prebundleTransformer,
432429
target,
@@ -669,7 +666,7 @@ export async function setupServer(
669666
// Exclude any explicitly defined dependencies (currently build defined externals)
670667
exclude: externalMetadata.explicit,
671668
// Include all implict dependencies from the external packages internal option
672-
include: externalMetadata.implicit,
669+
include: externalMetadata.implicitBrowser,
673670
ssr: false,
674671
prebundleTransformer,
675672
target,

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

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,10 @@ export type BundleContextResult =
2929
metafile: Metafile;
3030
outputFiles: BuildOutputFile[];
3131
initialFiles: Map<string, InitialFileRecord>;
32-
externalImports: Set<string>;
32+
externalImports: {
33+
server?: Set<string>;
34+
browser?: Set<string>;
35+
};
3336
};
3437

3538
export interface InitialFileRecord {
@@ -117,7 +120,9 @@ export class BundlerContext {
117120
const warnings: Message[] = [];
118121
const metafile: Metafile = { inputs: {}, outputs: {} };
119122
const initialFiles = new Map<string, InitialFileRecord>();
120-
const externalImports = new Set<string>();
123+
const externalImportsBrowser = new Set<string>();
124+
const externalImportsServer = new Set<string>();
125+
121126
const outputFiles = [];
122127
for (const result of individualResults) {
123128
warnings.push(...result.warnings);
@@ -135,7 +140,8 @@ export class BundlerContext {
135140

136141
result.initialFiles.forEach((value, key) => initialFiles.set(key, value));
137142
outputFiles.push(...result.outputFiles);
138-
result.externalImports.forEach((value) => externalImports.add(value));
143+
result.externalImports.browser?.forEach((value) => externalImportsBrowser.add(value));
144+
result.externalImports.server?.forEach((value) => externalImportsServer.add(value));
139145
}
140146

141147
if (errors !== undefined) {
@@ -148,7 +154,10 @@ export class BundlerContext {
148154
metafile,
149155
initialFiles,
150156
outputFiles,
151-
externalImports,
157+
externalImports: {
158+
browser: externalImportsBrowser,
159+
server: externalImportsServer,
160+
},
152161
};
153162
}
154163

@@ -175,7 +184,7 @@ export class BundlerContext {
175184
return result;
176185
}
177186

178-
async #performBundle() {
187+
async #performBundle(): Promise<BundleContextResult> {
179188
// Create esbuild options if not present
180189
if (this.#esbuildOptions === undefined) {
181190
if (this.incremental) {
@@ -313,15 +322,13 @@ export class BundlerContext {
313322
}
314323
}
315324

325+
const platformIsServer = this.#esbuildOptions?.platform === 'node';
316326
const outputFiles = result.outputFiles.map((file) => {
317327
let fileType: BuildOutputFileType;
318328
if (dirname(file.path) === 'media') {
319329
fileType = BuildOutputFileType.Media;
320330
} else {
321-
fileType =
322-
this.#esbuildOptions?.platform === 'node'
323-
? BuildOutputFileType.Server
324-
: BuildOutputFileType.Browser;
331+
fileType = platformIsServer ? BuildOutputFileType.Server : BuildOutputFileType.Browser;
325332
}
326333

327334
return convertOutputFile(file, fileType);
@@ -332,7 +339,9 @@ export class BundlerContext {
332339
...result,
333340
outputFiles,
334341
initialFiles,
335-
externalImports,
342+
externalImports: {
343+
[platformIsServer ? 'server' : 'browser']: externalImports,
344+
},
336345
errors: undefined,
337346
};
338347
}

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

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,20 @@ export interface RebuildState {
2424
previousOutputHashes: Map<string, string>;
2525
}
2626

27+
export interface ExecutionResultMetadata {
28+
implicitBrowser: string[];
29+
implicitServer: string[];
30+
explicit: string[];
31+
}
32+
2733
/**
2834
* Represents the result of a single builder execute call.
2935
*/
3036
export class ExecutionResult {
3137
outputFiles: BuildOutputFile[] = [];
3238
assetFiles: BuildOutputAsset[] = [];
3339
errors: (Message | PartialMessage)[] = [];
34-
externalMetadata?: { implicit: string[]; explicit?: string[] };
40+
externalMetadata?: ExecutionResultMetadata;
3541

3642
constructor(
3743
private rebuildContexts: BundlerContext[],
@@ -53,11 +59,16 @@ export class ExecutionResult {
5359
/**
5460
* Add external JavaScript import metadata to the result. This is currently used
5561
* by the development server to optimize the prebundling process.
56-
* @param implicit External dependencies due to the external packages option.
62+
* @param implicitBrowser External dependencies for the browser bundles due to the external packages option.
63+
* @param implicitServer External dependencies for the server bundles due to the external packages option.
5764
* @param explicit External dependencies due to explicit project configuration.
5865
*/
59-
setExternalMetadata(implicit: string[], explicit: string[] | undefined) {
60-
this.externalMetadata = { implicit, explicit };
66+
setExternalMetadata(
67+
implicitBrowser: string[],
68+
implicitServer: string[],
69+
explicit: string[] | undefined,
70+
): void {
71+
this.externalMetadata = { implicitBrowser, implicitServer, explicit: explicit ?? [] };
6172
}
6273

6374
get output() {

0 commit comments

Comments
 (0)