From 80e6b7b260933a82011db48a79a89f6947c17eb1 Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Thu, 31 Oct 2024 10:25:48 -0400 Subject: [PATCH] refactor(@angular/build): improve typescript bundling context rebuild checking The TypeScript-based bundling contexts have now been separated from the other bundling contexts that may be present in an application build. The later of which include global styles, scripts, and non-TypeScript polyfills. This allows for the TypeScript bundling contexts to perform additional checks during a rebuild to determine if they actually need to be rebundled. The bundling context caching for the TypeScript contexts has not yet been enabled but these changes prepare for the switch to allow conditional rebundling on file changes. --- .../src/builders/application/execute-build.ts | 47 ++++++++++++------- .../builders/application/setup-bundling.ts | 37 +++++++++------ .../esbuild/angular/component-stylesheets.ts | 8 ++++ .../esbuild/angular/source-file-cache.ts | 14 ++++-- .../tools/esbuild/application-code-bundle.ts | 9 ++-- .../tools/esbuild/bundler-execution-result.ts | 23 ++++++--- 6 files changed, 92 insertions(+), 46 deletions(-) diff --git a/packages/angular/build/src/builders/application/execute-build.ts b/packages/angular/build/src/builders/application/execute-build.ts index 2a18068cc10e..ce2276a4e326 100644 --- a/packages/angular/build/src/builders/application/execute-build.ts +++ b/packages/angular/build/src/builders/application/execute-build.ts @@ -74,37 +74,48 @@ export async function executeBuild( let bundlerContexts; let componentStyleBundler; let codeBundleCache; + let bundlingResult: BundleContextResult; if (rebuildState) { bundlerContexts = rebuildState.rebuildContexts; componentStyleBundler = rebuildState.componentStyleBundler; codeBundleCache = rebuildState.codeBundleCache; + const allFileChanges = rebuildState.fileChanges.all; + + // Bundle all contexts that do not require TypeScript changed file checks. + // These will automatically use cached results based on the changed files. + bundlingResult = await BundlerContext.bundleAll(bundlerContexts.otherContexts, allFileChanges); + + // Check the TypeScript code bundling cache for changes. If invalid, force a rebundle of + // all TypeScript related contexts. + // TODO: Enable cached bundling for the typescript contexts + const forceTypeScriptRebuild = codeBundleCache?.invalidate(allFileChanges); + const typescriptResults: BundleContextResult[] = []; + for (const typescriptContext of bundlerContexts.typescriptContexts) { + typescriptContext.invalidate(allFileChanges); + const result = await typescriptContext.bundle(forceTypeScriptRebuild); + typescriptResults.push(result); + } + bundlingResult = BundlerContext.mergeResults([bundlingResult, ...typescriptResults]); } else { const target = transformSupportedBrowsersToTargets(browsers); codeBundleCache = new SourceFileCache(cacheOptions.enabled ? cacheOptions.path : undefined); componentStyleBundler = createComponentStyleBundler(options, target); bundlerContexts = setupBundlerContexts(options, target, codeBundleCache, componentStyleBundler); - } - let bundlingResult = await BundlerContext.bundleAll( - bundlerContexts, - rebuildState?.fileChanges.all, - ); + // Bundle everything on initial build + bundlingResult = await BundlerContext.bundleAll([ + ...bundlerContexts.typescriptContexts, + ...bundlerContexts.otherContexts, + ]); + } + // Update any external component styles if enabled and rebuilding. + // TODO: Only attempt rebundling of invalidated styles once incremental build results are supported. if (rebuildState && options.externalRuntimeStyles) { - const invalidatedStylesheetEntries = componentStyleBundler.invalidate( - rebuildState.fileChanges.all, - ); - - if (invalidatedStylesheetEntries?.length) { - const componentResults: BundleContextResult[] = []; - for (const stylesheetFile of invalidatedStylesheetEntries) { - // externalId is already linked in the bundler context so only enabling is required here - const result = await componentStyleBundler.bundleFile(stylesheetFile, true, true); - componentResults.push(result); - } + componentStyleBundler.invalidate(rebuildState.fileChanges.all); - bundlingResult = BundlerContext.mergeResults([bundlingResult, ...componentResults]); - } + const componentResults = await componentStyleBundler.bundleAllFiles(true, true); + bundlingResult = BundlerContext.mergeResults([bundlingResult, ...componentResults]); } if (options.optimizationOptions.scripts && shouldOptimizeChunks) { diff --git a/packages/angular/build/src/builders/application/setup-bundling.ts b/packages/angular/build/src/builders/application/setup-bundling.ts index 8f38137ff3ee..323e3783439d 100644 --- a/packages/angular/build/src/builders/application/setup-bundling.ts +++ b/packages/angular/build/src/builders/application/setup-bundling.ts @@ -34,7 +34,10 @@ export function setupBundlerContexts( target: string[], codeBundleCache: SourceFileCache, stylesheetBundler: ComponentStylesheetBundler, -): BundlerContext[] { +): { + typescriptContexts: BundlerContext[]; + otherContexts: BundlerContext[]; +} { const { outputMode, serverEntryPoint, @@ -44,10 +47,11 @@ export function setupBundlerContexts( workspaceRoot, watch = false, } = options; - const bundlerContexts = []; + const typescriptContexts = []; + const otherContexts = []; // Browser application code - bundlerContexts.push( + typescriptContexts.push( new BundlerContext( workspaceRoot, watch, @@ -63,7 +67,16 @@ export function setupBundlerContexts( stylesheetBundler, ); if (browserPolyfillBundleOptions) { - bundlerContexts.push(new BundlerContext(workspaceRoot, watch, browserPolyfillBundleOptions)); + const browserPolyfillContext = new BundlerContext( + workspaceRoot, + watch, + browserPolyfillBundleOptions, + ); + if (typeof browserPolyfillBundleOptions === 'function') { + otherContexts.push(browserPolyfillContext); + } else { + typescriptContexts.push(browserPolyfillContext); + } } // Global Stylesheets @@ -71,9 +84,7 @@ export function setupBundlerContexts( for (const initial of [true, false]) { const bundleOptions = createGlobalStylesBundleOptions(options, target, initial); if (bundleOptions) { - bundlerContexts.push( - new BundlerContext(workspaceRoot, watch, bundleOptions, () => initial), - ); + otherContexts.push(new BundlerContext(workspaceRoot, watch, bundleOptions, () => initial)); } } } @@ -83,9 +94,7 @@ export function setupBundlerContexts( for (const initial of [true, false]) { const bundleOptions = createGlobalScriptsBundleOptions(options, target, initial); if (bundleOptions) { - bundlerContexts.push( - new BundlerContext(workspaceRoot, watch, bundleOptions, () => initial), - ); + otherContexts.push(new BundlerContext(workspaceRoot, watch, bundleOptions, () => initial)); } } } @@ -94,7 +103,7 @@ export function setupBundlerContexts( if (serverEntryPoint && (outputMode || prerenderOptions || appShellOptions || ssrOptions)) { const nodeTargets = [...target, ...getSupportedNodeTargets()]; - bundlerContexts.push( + typescriptContexts.push( new BundlerContext( workspaceRoot, watch, @@ -104,7 +113,7 @@ export function setupBundlerContexts( if (outputMode && ssrOptions?.entry) { // New behavior introduced: 'server.ts' is now bundled separately from 'main.server.ts'. - bundlerContexts.push( + typescriptContexts.push( new BundlerContext( workspaceRoot, watch, @@ -121,11 +130,11 @@ export function setupBundlerContexts( ); if (serverPolyfillBundleOptions) { - bundlerContexts.push(new BundlerContext(workspaceRoot, watch, serverPolyfillBundleOptions)); + otherContexts.push(new BundlerContext(workspaceRoot, watch, serverPolyfillBundleOptions)); } } - return bundlerContexts; + return { typescriptContexts, otherContexts }; } export function createComponentStyleBundler( diff --git a/packages/angular/build/src/tools/esbuild/angular/component-stylesheets.ts b/packages/angular/build/src/tools/esbuild/angular/component-stylesheets.ts index dc6686a6c2c9..ce9cca6c7529 100644 --- a/packages/angular/build/src/tools/esbuild/angular/component-stylesheets.ts +++ b/packages/angular/build/src/tools/esbuild/angular/component-stylesheets.ts @@ -86,6 +86,14 @@ export class ComponentStylesheetBundler { ); } + bundleAllFiles(external: boolean, direct: boolean) { + return Promise.all( + Array.from(this.#fileContexts.entries()).map(([entry]) => + this.bundleFile(entry, external, direct), + ), + ); + } + async bundleInline( data: string, filename: string, diff --git a/packages/angular/build/src/tools/esbuild/angular/source-file-cache.ts b/packages/angular/build/src/tools/esbuild/angular/source-file-cache.ts index f3bf3ca4b94e..3b733cb412d5 100644 --- a/packages/angular/build/src/tools/esbuild/angular/source-file-cache.ts +++ b/packages/angular/build/src/tools/esbuild/angular/source-file-cache.ts @@ -25,21 +25,29 @@ export class SourceFileCache extends Map { super(); } - invalidate(files: Iterable): void { + invalidate(files: Iterable): boolean { if (files !== this.modifiedFiles) { this.modifiedFiles.clear(); } + + const extraWatchFiles = new Set(this.referencedFiles?.map(path.normalize)); + + let invalid = false; for (let file of files) { file = path.normalize(file); - this.loadResultCache.invalidate(file); + invalid = this.loadResultCache.invalidate(file) || invalid; // Normalize separators to allow matching TypeScript Host paths if (USING_WINDOWS) { file = file.replace(WINDOWS_SEP_REGEXP, path.posix.sep); } - this.delete(file); + invalid = this.delete(file) || invalid; this.modifiedFiles.add(file); + + invalid = extraWatchFiles.has(file) || invalid; } + + return invalid; } } diff --git a/packages/angular/build/src/tools/esbuild/application-code-bundle.ts b/packages/angular/build/src/tools/esbuild/application-code-bundle.ts index 6c6eba9ae06a..38c0e864233a 100644 --- a/packages/angular/build/src/tools/esbuild/application-code-bundle.ts +++ b/packages/angular/build/src/tools/esbuild/application-code-bundle.ts @@ -24,6 +24,7 @@ import { BundlerOptionsFactory } from './bundler-context'; import { createCompilerPluginOptions } from './compiler-plugin-options'; import { createExternalPackagesPlugin } from './external-packages-plugin'; import { createAngularLocaleDataPlugin } from './i18n-locale-plugin'; +import type { LoadResultCache } from './load-result-cache'; import { createLoaderImportAttributePlugin } from './loader-import-attribute-plugin'; import { createRxjsEsmResolutionPlugin } from './rxjs-esm-resolution-plugin'; import { createServerBundleMetadata } from './server-bundle-metadata-plugin'; @@ -106,7 +107,7 @@ export function createBrowserPolyfillBundleOptions( options, namespace, true, - sourceFileCache, + sourceFileCache.loadResultCache, ); if (!polyfillBundleOptions) { return; @@ -184,7 +185,7 @@ export function createServerPolyfillBundleOptions( }, namespace, false, - sourceFileCache, + sourceFileCache?.loadResultCache, ); if (!polyfillBundleOptions) { @@ -602,7 +603,7 @@ function getEsBuildCommonPolyfillsOptions( options: NormalizedApplicationBuildOptions, namespace: string, tryToResolvePolyfillsAsRelative: boolean, - sourceFileCache: SourceFileCache | undefined, + loadResultCache: LoadResultCache | undefined, ): BuildOptions | undefined { const { jit, workspaceRoot, i18nOptions } = options; const buildOptions: BuildOptions = { @@ -647,7 +648,7 @@ function getEsBuildCommonPolyfillsOptions( buildOptions.plugins?.push( createVirtualModulePlugin({ namespace, - cache: sourceFileCache?.loadResultCache, + cache: loadResultCache, loadContent: async (_, build) => { let polyfillPaths = polyfills; let warnings: PartialMessage[] | undefined; diff --git a/packages/angular/build/src/tools/esbuild/bundler-execution-result.ts b/packages/angular/build/src/tools/esbuild/bundler-execution-result.ts index 5cc37c139e5f..15164146b9c8 100644 --- a/packages/angular/build/src/tools/esbuild/bundler-execution-result.ts +++ b/packages/angular/build/src/tools/esbuild/bundler-execution-result.ts @@ -20,7 +20,10 @@ export interface BuildOutputAsset { } export interface RebuildState { - rebuildContexts: BundlerContext[]; + rebuildContexts: { + typescriptContexts: BundlerContext[]; + otherContexts: BundlerContext[]; + }; componentStyleBundler: ComponentStylesheetBundler; codeBundleCache?: SourceFileCache; fileChanges: ChangedFiles; @@ -51,7 +54,10 @@ export class ExecutionResult { htmlBaseHref?: string; constructor( - private rebuildContexts: BundlerContext[], + private rebuildContexts: { + typescriptContexts: BundlerContext[]; + otherContexts: BundlerContext[]; + }, private componentStyleBundler: ComponentStylesheetBundler, private codeBundleCache?: SourceFileCache, ) {} @@ -141,7 +147,9 @@ export class ExecutionResult { get watchFiles() { // Bundler contexts internally normalize file dependencies - const files = this.rebuildContexts.flatMap((context) => [...context.watchFiles]); + const files = this.rebuildContexts.typescriptContexts + .flatMap((context) => [...context.watchFiles]) + .concat(this.rebuildContexts.otherContexts.flatMap((context) => [...context.watchFiles])); if (this.codeBundleCache?.referencedFiles) { // These files originate from TS/NG and can have POSIX path separators even on Windows. // To ensure path comparisons are valid, all these paths must be normalized. @@ -156,8 +164,6 @@ export class ExecutionResult { } createRebuildState(fileChanges: ChangedFiles): RebuildState { - this.codeBundleCache?.invalidate([...fileChanges.modified, ...fileChanges.removed]); - return { rebuildContexts: this.rebuildContexts, codeBundleCache: this.codeBundleCache, @@ -180,7 +186,10 @@ export class ExecutionResult { } async dispose(): Promise { - await Promise.allSettled(this.rebuildContexts.map((context) => context.dispose())); - await this.componentStyleBundler.dispose(); + await Promise.allSettled([ + ...this.rebuildContexts.typescriptContexts.map((context) => context.dispose()), + ...this.rebuildContexts.otherContexts.map((context) => context.dispose()), + this.componentStyleBundler.dispose(), + ]); } }