From 7751600bea92175f443ad6cce5c3c9fd795b49b3 Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Mon, 19 Oct 2026 15:30:49 -0400 Subject: [PATCH] refactor(@angular/build): extract shared breadth-first initial file analysis Previously, both `BundlerContext` and `optimizeChunks` maintained separate implementations for discovering transitive initial files and calculating their import depths from the esbuild metafile. Both implementations used a depth-first stack traversal via `Array.prototype.pop()`, which visited deeper branches in reverse import order and failed to propagate updated minimum depths to the transitive dependencies of already-visited chunks in diamond dependency graphs. This commit extracts a shared `findTransitiveInitialFiles` helper that uses a breadth-first queue traversal over direct static imports. Processing chunks in breadth-first order guarantees that each file is discovered at its true shortest import depth on first visit, preserves the natural declaration order of imports at each depth level, and eliminates the duplicated metafile traversal logic across initial bundling and chunk optimization. --- .../builders/application/chunk-optimizer.ts | 54 +--- .../src/tools/esbuild/bundler-context.ts | 37 +-- .../build/src/tools/esbuild/bundler-files.ts | 53 +++- .../src/tools/esbuild/bundler-files_spec.ts | 298 ++++++++++++++++++ 4 files changed, 358 insertions(+), 84 deletions(-) create mode 100644 packages/angular/build/src/tools/esbuild/bundler-files_spec.ts diff --git a/packages/angular/build/src/builders/application/chunk-optimizer.ts b/packages/angular/build/src/builders/application/chunk-optimizer.ts index cd5e4e1814ec..1c9cb8eb51d6 100644 --- a/packages/angular/build/src/builders/application/chunk-optimizer.ts +++ b/packages/angular/build/src/builders/application/chunk-optimizer.ts @@ -24,8 +24,8 @@ import { BundleMergedContextResult } from '../../tools/esbuild/bundler-context'; import { type BuildOutputFile, BuildOutputFileType, - type InitialFileRecord, createOutputFile, + findTransitiveInitialFiles, } from '../../tools/esbuild/bundler-files'; import { useRolldownChunks } from '../../utils/environment-options'; import { assertIsError } from '../../utils/error'; @@ -166,21 +166,6 @@ function bundleOutputToEsbuildMetafile( return newMetafile; } -/** - * Creates an InitialFileRecord object with a specified depth. - * @param depth The depth of the file in the dependency graph. - * @returns An InitialFileRecord object. - */ -function createInitialFileRecord(depth: number): InitialFileRecord { - return { - type: 'script', - entrypoint: false, - external: false, - serverFile: false, - depth, - }; -} - /** * Creates an esbuild message object for a chunk optimization failure. * @param message The error message detailing the cause of the failure. @@ -369,14 +354,11 @@ export async function optimizeChunks( ); // Add new optimized chunks - const importsPerFile: Record = {}; for (const optimizedFile of optimizedOutput) { if (optimizedFile.type !== 'chunk') { continue; } - importsPerFile[optimizedFile.fileName] = optimizedFile.imports; - original.outputFiles.push( createOutputFile(optimizedFile.fileName, optimizedFile.code, BuildOutputFileType.Browser), ); @@ -392,41 +374,17 @@ export async function optimizeChunks( } // Update initial files to reflect optimized chunks - const entriesToAnalyze: [string, InitialFileRecord][] = []; for (const usedFile of usedChunks) { // Leave the main file since its information did not change - if (usedFile === mainFile) { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - entriesToAnalyze.push([mainFile, original.initialFiles.get(mainFile)!]); - continue; + if (usedFile !== mainFile) { + original.initialFiles.delete(usedFile); } - - // Remove all other used chunks - original.initialFiles.delete(usedFile); } // Analyze for transitive initial files - let currentEntry; - while ((currentEntry = entriesToAnalyze.pop())) { - const [entryPath, entryRecord] = currentEntry; - - for (const importPath of importsPerFile[entryPath]) { - const existingRecord = original.initialFiles.get(importPath); - if (existingRecord) { - // Store the smallest value depth - if (existingRecord.depth > entryRecord.depth + 1) { - existingRecord.depth = entryRecord.depth + 1; - } - - continue; - } - - const record = createInitialFileRecord(entryRecord.depth + 1); - - original.initialFiles.set(importPath, record); - entriesToAnalyze.push([importPath, record]); - } - } + const mainRecord = original.initialFiles.get(mainFile); + assert(mainRecord, `Main file record should always be present in initialFiles [${mainFile}].`); + findTransitiveInitialFiles(newMetafile, original.initialFiles, [[mainFile, mainRecord]]); return original; } diff --git a/packages/angular/build/src/tools/esbuild/bundler-context.ts b/packages/angular/build/src/tools/esbuild/bundler-context.ts index 202ab514cbfe..c2a53df67b8c 100644 --- a/packages/angular/build/src/tools/esbuild/bundler-context.ts +++ b/packages/angular/build/src/tools/esbuild/bundler-context.ts @@ -24,6 +24,7 @@ import { BuildOutputFileType, type InitialFileRecord, convertOutputFile, + findTransitiveInitialFiles, } from './bundler-files'; import { LoadResultCache, MemoryLoadResultCache } from './load-result-cache'; @@ -363,41 +364,7 @@ export class BundlerContext { } // Analyze for transitive initial files - const entriesToAnalyze = [...initialFiles]; - let currentEntry; - while ((currentEntry = entriesToAnalyze.pop())) { - const [entryPath, entryRecord] = currentEntry; - - for (const initialImport of result.metafile.outputs[entryPath].imports) { - const existingRecord = initialFiles.get(initialImport.path); - if (existingRecord) { - // Store the smallest value depth - if (existingRecord.depth > entryRecord.depth + 1) { - existingRecord.depth = entryRecord.depth + 1; - } - - continue; - } - - if (initialImport.kind === 'import-statement' || initialImport.kind === 'import-rule') { - const record: InitialFileRecord = { - type: initialImport.kind === 'import-rule' ? 'style' : 'script', - entrypoint: false, - external: initialImport.external, - serverFile: isPlatformServer, - depth: entryRecord.depth + 1, - }; - - if (!this.initialFilter || this.initialFilter(record)) { - initialFiles.set(initialImport.path, record); - } - - if (!initialImport.external) { - entriesToAnalyze.push([initialImport.path, record]); - } - } - } - } + findTransitiveInitialFiles(result.metafile, initialFiles, undefined, this.initialFilter); // Collect all external package names const externalImports = new Set(); diff --git a/packages/angular/build/src/tools/esbuild/bundler-files.ts b/packages/angular/build/src/tools/esbuild/bundler-files.ts index 168c41e65940..af4b04ad9e56 100644 --- a/packages/angular/build/src/tools/esbuild/bundler-files.ts +++ b/packages/angular/build/src/tools/esbuild/bundler-files.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.dev/license */ -import type { OutputFile } from 'esbuild'; +import type { Metafile, OutputFile } from 'esbuild'; import { calculateHash } from '../../utils/hash'; export interface InitialFileRecord { @@ -18,6 +18,57 @@ export interface InitialFileRecord { depth: number; } +/** + * Analyzes a metafile to find and record all transitive initial files starting from the given + * initial entries using breadth-first traversal so each file is recorded at its minimum depth. + * + * @param metafile The esbuild metafile containing output import relationships. + * @param initialFiles The map of initial files to populate. + * @param entriesToAnalyze Optional starting entries to traverse; defaults to all current `initialFiles`. + * Entries must share the same starting depth (or be ordered by non-decreasing depth) to guarantee + * that each transitive file is visited at its minimum depth first. + * @param initialFilter Optional filter predicate to determine whether a discovered record should be added. + */ +export function findTransitiveInitialFiles( + metafile: Metafile, + initialFiles: Map, + entriesToAnalyze: Iterable<[string, InitialFileRecord]> = initialFiles, + initialFilter?: (initial: Readonly) => boolean, +): void { + const queue = [...entriesToAnalyze]; + for (let i = 0; i < queue.length; i++) { + const [entryPath, entryRecord] = queue[i]; + const entryOutput = metafile.outputs[entryPath]; + if (!entryOutput) { + continue; + } + + for (const initialImport of entryOutput.imports) { + if (initialFiles.has(initialImport.path)) { + continue; + } + + if (initialImport.kind === 'import-statement' || initialImport.kind === 'import-rule') { + const record: InitialFileRecord = { + type: initialImport.kind === 'import-rule' ? 'style' : 'script', + entrypoint: false, + external: initialImport.external, + serverFile: entryRecord.serverFile, + depth: entryRecord.depth + 1, + }; + + if (!initialFilter || initialFilter(record)) { + initialFiles.set(initialImport.path, record); + + if (!initialImport.external) { + queue.push([initialImport.path, record]); + } + } + } + } + } +} + export enum BuildOutputFileType { Browser, Media, diff --git a/packages/angular/build/src/tools/esbuild/bundler-files_spec.ts b/packages/angular/build/src/tools/esbuild/bundler-files_spec.ts new file mode 100644 index 000000000000..be21f7ded608 --- /dev/null +++ b/packages/angular/build/src/tools/esbuild/bundler-files_spec.ts @@ -0,0 +1,298 @@ +/** + * @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 type { Metafile } from 'esbuild'; +import { type InitialFileRecord, findTransitiveInitialFiles } from './bundler-files'; + +function createMetafile( + outputs: Record< + string, + { + imports: { + path: string; + kind: 'import-statement' | 'import-rule' | 'dynamic-import'; + external?: boolean; + }[]; + } + >, +): Metafile { + return { + inputs: {}, + outputs: Object.fromEntries( + Object.entries(outputs).map(([key, value]) => [ + key, + { + bytes: 0, + inputs: {}, + imports: value.imports, + exports: [], + }, + ]), + ), + }; +} + +describe('findTransitiveInitialFiles', () => { + it('should compute minimum depth and propagate it to transitive children in a DAG', () => { + // main (0) -> [a (1), b (1)] + // b (1) -> c (2) -> d (via b: 3, via a: 2) -> e (should be 3, not 4) + // a (1) -> d (2) + const metafile = createMetafile({ + 'main.js': { + imports: [ + { path: 'chunk-a.js', kind: 'import-statement' }, + { path: 'chunk-b.js', kind: 'import-statement' }, + ], + }, + 'chunk-a.js': { + imports: [{ path: 'chunk-d.js', kind: 'import-statement' }], + }, + 'chunk-b.js': { + imports: [{ path: 'chunk-c.js', kind: 'import-statement' }], + }, + 'chunk-c.js': { + imports: [{ path: 'chunk-d.js', kind: 'import-statement' }], + }, + 'chunk-d.js': { + imports: [{ path: 'chunk-e.js', kind: 'import-statement' }], + }, + 'chunk-e.js': { + imports: [], + }, + }); + + const initialFiles = new Map([ + ['main.js', { entrypoint: true, name: 'main', type: 'script', serverFile: false, depth: 0 }], + ]); + + findTransitiveInitialFiles(metafile, initialFiles); + + expect(initialFiles.get('chunk-a.js')?.depth).toBe(1); + expect(initialFiles.get('chunk-b.js')?.depth).toBe(1); + expect(initialFiles.get('chunk-c.js')?.depth).toBe(2); + expect(initialFiles.get('chunk-d.js')?.depth).toBe(2); + expect(initialFiles.get('chunk-e.js')?.depth).toBe(3); + }); + + it('should record files in breadth-first forward import order', () => { + const metafile = createMetafile({ + 'main.js': { + imports: [ + { path: 'chunk-a.js', kind: 'import-statement' }, + { path: 'chunk-b.js', kind: 'import-statement' }, + ], + }, + 'chunk-a.js': { + imports: [{ path: 'chunk-a1.js', kind: 'import-statement' }], + }, + 'chunk-b.js': { + imports: [{ path: 'chunk-b1.js', kind: 'import-statement' }], + }, + 'chunk-a1.js': { imports: [] }, + 'chunk-b1.js': { imports: [] }, + }); + + const initialFiles = new Map([ + ['main.js', { entrypoint: true, name: 'main', type: 'script', serverFile: false, depth: 0 }], + ]); + + findTransitiveInitialFiles(metafile, initialFiles); + + expect([...initialFiles.keys()]).toEqual([ + 'main.js', + 'chunk-a.js', + 'chunk-b.js', + 'chunk-a1.js', + 'chunk-b1.js', + ]); + }); + + it('should ignore dynamic imports and handle external and style imports', () => { + const metafile = createMetafile({ + 'main.js': { + imports: [ + { path: 'lazy.js', kind: 'dynamic-import' }, + { path: '@angular/core', kind: 'import-statement', external: true }, + ], + }, + 'styles.css': { + imports: [{ path: 'https://example.com/font.css', kind: 'import-rule', external: true }], + }, + 'lazy.js': { + imports: [{ path: 'lazy-dep.js', kind: 'import-statement' }], + }, + }); + + const initialFiles = new Map([ + ['main.js', { entrypoint: true, name: 'main', type: 'script', serverFile: false, depth: 0 }], + [ + 'styles.css', + { entrypoint: true, name: 'styles', type: 'style', serverFile: false, depth: 0 }, + ], + ]); + + findTransitiveInitialFiles(metafile, initialFiles); + + expect(initialFiles.has('lazy.js')).toBeFalse(); + expect(initialFiles.has('lazy-dep.js')).toBeFalse(); + expect(initialFiles.get('@angular/core')).toEqual({ + type: 'script', + entrypoint: false, + external: true, + serverFile: false, + depth: 1, + }); + expect(initialFiles.get('https://example.com/font.css')).toEqual({ + type: 'style', + entrypoint: false, + external: true, + serverFile: false, + depth: 1, + }); + }); + + it('should only traverse from provided entriesToAnalyze while preserving existing initialFiles', () => { + const metafile = createMetafile({ + 'main.js': { + imports: [{ path: 'chunk-main.js', kind: 'import-statement' }], + }, + 'polyfills.js': { + imports: [{ path: 'chunk-polyfills.js', kind: 'import-statement' }], + }, + 'chunk-main.js': { imports: [] }, + 'chunk-polyfills.js': { imports: [] }, + }); + + const mainRecord: InitialFileRecord = { + entrypoint: true, + name: 'main', + type: 'script', + serverFile: false, + depth: 0, + }; + const polyfillsRecord: InitialFileRecord = { + entrypoint: true, + name: 'polyfills', + type: 'script', + serverFile: false, + depth: 0, + }; + const initialFiles = new Map([ + ['main.js', mainRecord], + ['polyfills.js', polyfillsRecord], + ]); + + const entriesToAnalyze: [string, InitialFileRecord][] = [['main.js', mainRecord]]; + findTransitiveInitialFiles(metafile, initialFiles, entriesToAnalyze); + + expect(entriesToAnalyze.length).toBe(1); + expect(initialFiles.has('main.js')).toBeTrue(); + expect(initialFiles.has('polyfills.js')).toBeTrue(); + expect(initialFiles.has('chunk-main.js')).toBeTrue(); + expect(initialFiles.has('chunk-polyfills.js')).toBeFalse(); + }); + + it('should filter discovered records using initialFilter when provided', () => { + const metafile = createMetafile({ + 'main.js': { + imports: [ + { path: 'chunk-a.js', kind: 'import-statement' }, + { path: 'external-lib', kind: 'import-statement', external: true }, + ], + }, + 'chunk-a.js': { + imports: [{ path: 'chunk-b.js', kind: 'import-statement' }], + }, + 'chunk-b.js': { + imports: [{ path: 'chunk-c.js', kind: 'import-statement' }], + }, + 'chunk-c.js': { + imports: [{ path: 'chunk-b.js', kind: 'import-statement' }], + }, + }); + + const initialFiles = new Map([ + ['main.js', { entrypoint: true, name: 'main', type: 'script', serverFile: false, depth: 0 }], + ]); + + findTransitiveInitialFiles( + metafile, + initialFiles, + undefined, + (record) => !record.external && record.depth <= 1, + ); + + expect(initialFiles.has('chunk-a.js')).toBeTrue(); + expect(initialFiles.has('external-lib')).toBeFalse(); + expect(initialFiles.has('chunk-b.js')).toBeFalse(); + expect(initialFiles.has('chunk-c.js')).toBeFalse(); + }); + + it('should handle circular static imports between chunks without infinite recursion', () => { + const metafile = createMetafile({ + 'main.js': { + imports: [{ path: 'chunk-a.js', kind: 'import-statement' }], + }, + 'chunk-a.js': { + imports: [{ path: 'chunk-b.js', kind: 'import-statement' }], + }, + 'chunk-b.js': { + imports: [ + { path: 'chunk-a.js', kind: 'import-statement' }, + { path: 'main.js', kind: 'import-statement' }, + ], + }, + }); + + const initialFiles = new Map([ + ['main.js', { entrypoint: true, name: 'main', type: 'script', serverFile: false, depth: 0 }], + ]); + + findTransitiveInitialFiles(metafile, initialFiles); + + expect(initialFiles.get('main.js')?.depth).toBe(0); + expect(initialFiles.get('chunk-a.js')?.depth).toBe(1); + expect(initialFiles.get('chunk-b.js')?.depth).toBe(2); + }); + + it('should propagate serverFile flag from entry record to transitive imports', () => { + const metafile = createMetafile({ + 'server.mjs': { + imports: [{ path: 'chunk-server-1.mjs', kind: 'import-statement' }], + }, + 'chunk-server-1.mjs': { + imports: [{ path: 'chunk-server-2.mjs', kind: 'import-statement' }], + }, + 'chunk-server-2.mjs': { imports: [] }, + }); + + const initialFiles = new Map([ + [ + 'server.mjs', + { entrypoint: true, name: 'server', type: 'script', serverFile: true, depth: 0 }, + ], + ]); + + findTransitiveInitialFiles(metafile, initialFiles); + + expect(initialFiles.get('chunk-server-1.mjs')).toEqual({ + type: 'script', + entrypoint: false, + external: undefined, + serverFile: true, + depth: 1, + }); + expect(initialFiles.get('chunk-server-2.mjs')).toEqual({ + type: 'script', + entrypoint: false, + external: undefined, + serverFile: true, + depth: 2, + }); + }); +});