From a8a60bfadacae1ed4fe7f08d2111695a8bac8448 Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Tue, 29 Oct 2024 13:13:22 -0400 Subject: [PATCH] refactor(@angular/build): directly bundle external component file-based style changes When using the development server with the application builder, file-based component styles will now be bundled during the main builder execution instead of within the application code bundling step. This allows for these styles to be processed independently from any code bundling steps and will support future changes that will allow the builder to completely skip code bundling if only file-based component stylesheets are changed. --- .../src/builders/application/execute-build.ts | 28 ++++- .../tools/esbuild/angular/compiler-plugin.ts | 59 +++++----- .../esbuild/angular/component-stylesheets.ts | 107 +++++++++++------- .../esbuild/angular/jit-plugin-callbacks.ts | 12 +- 4 files changed, 134 insertions(+), 72 deletions(-) diff --git a/packages/angular/build/src/builders/application/execute-build.ts b/packages/angular/build/src/builders/application/execute-build.ts index c03d8dc00bb6..2a18068cc10e 100644 --- a/packages/angular/build/src/builders/application/execute-build.ts +++ b/packages/angular/build/src/builders/application/execute-build.ts @@ -10,7 +10,11 @@ import { BuilderContext } from '@angular-devkit/architect'; import assert from 'node:assert'; import { SourceFileCache } from '../../tools/esbuild/angular/source-file-cache'; import { generateBudgetStats } from '../../tools/esbuild/budget-stats'; -import { BuildOutputFileType, BundlerContext } from '../../tools/esbuild/bundler-context'; +import { + BuildOutputFileType, + BundleContextResult, + BundlerContext, +} from '../../tools/esbuild/bundler-context'; import { ExecutionResult, RebuildState } from '../../tools/esbuild/bundler-execution-result'; import { checkCommonJSModules } from '../../tools/esbuild/commonjs-checker'; import { extractLicenses } from '../../tools/esbuild/license-extractor'; @@ -86,6 +90,23 @@ export async function executeBuild( rebuildState?.fileChanges.all, ); + 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); + } + + bundlingResult = BundlerContext.mergeResults([bundlingResult, ...componentResults]); + } + } + if (options.optimizationOptions.scripts && shouldOptimizeChunks) { bundlingResult = await profileAsync('OPTIMIZE_CHUNKS', () => optimizeChunks( @@ -102,6 +123,11 @@ export async function executeBuild( ); executionResult.addWarnings(bundlingResult.warnings); + // Add used external component style referenced files to be watched + if (options.externalRuntimeStyles) { + executionResult.extraWatchFiles.push(...componentStyleBundler.collectReferencedFiles()); + } + // Return if the bundling has errors if (bundlingResult.errors) { executionResult.addErrors(bundlingResult.errors); diff --git a/packages/angular/build/src/tools/esbuild/angular/compiler-plugin.ts b/packages/angular/build/src/tools/esbuild/angular/compiler-plugin.ts index fda63af4b56c..7f838e50307e 100644 --- a/packages/angular/build/src/tools/esbuild/angular/compiler-plugin.ts +++ b/packages/angular/build/src/tools/esbuild/angular/compiler-plugin.ts @@ -150,7 +150,6 @@ export function createCompilerPlugin( // Angular compiler which does not have direct knowledge of transitive resource // dependencies or web worker processing. let modifiedFiles; - let invalidatedStylesheetEntries; if ( pluginOptions.sourceFileCache?.modifiedFiles.size && referencedFileTracker && @@ -159,7 +158,11 @@ export function createCompilerPlugin( // TODO: Differentiate between changed input files and stale output files modifiedFiles = referencedFileTracker.update(pluginOptions.sourceFileCache.modifiedFiles); pluginOptions.sourceFileCache.invalidate(modifiedFiles); - invalidatedStylesheetEntries = stylesheetBundler.invalidate(modifiedFiles); + // External runtime styles are invalidated and rebuilt at the beginning of a rebuild to avoid + // the need to execute the application bundler for component style only changes. + if (!pluginOptions.externalRuntimeStyles) { + stylesheetBundler.invalidate(modifiedFiles); + } } if ( @@ -201,12 +204,14 @@ export function createCompilerPlugin( ); } - const { contents, outputFiles, metafile, referencedFiles, errors, warnings } = - stylesheetResult; - if (errors) { - (result.errors ??= []).push(...errors); + (result.warnings ??= []).push(...stylesheetResult.warnings); + if (stylesheetResult.errors) { + (result.errors ??= []).push(...stylesheetResult.errors); + + return ''; } - (result.warnings ??= []).push(...warnings); + + const { contents, outputFiles, metafile, referencedFiles } = stylesheetResult; additionalResults.set(stylesheetFile ?? containingFile, { outputFiles, metafile, @@ -332,19 +337,6 @@ export function createCompilerPlugin( additionalResults, ); } - // Process any updated stylesheets - if (invalidatedStylesheetEntries) { - for (const stylesheetFile of invalidatedStylesheetEntries) { - // externalId is already linked in the bundler context so only enabling is required here - await bundleExternalStylesheet( - stylesheetBundler, - stylesheetFile, - true, - result, - additionalResults, - ); - } - } } // Update TypeScript file output cache for all affected files @@ -565,18 +557,23 @@ async function bundleExternalStylesheet( { outputFiles?: OutputFile[]; metafile?: Metafile; errors?: PartialMessage[] } >, ) { - const { outputFiles, metafile, errors, warnings } = await stylesheetBundler.bundleFile( - stylesheetFile, - externalId, - ); - if (errors) { - (result.errors ??= []).push(...errors); + const styleResult = await stylesheetBundler.bundleFile(stylesheetFile, externalId); + + (result.warnings ??= []).push(...styleResult.warnings); + if (styleResult.errors) { + (result.errors ??= []).push(...styleResult.errors); + } else { + const { outputFiles, metafile } = styleResult; + // Clear inputs to prevent triggering a rebuild of the application code for component + // stylesheet file only changes when the dev server enables the internal-only external + // stylesheet option. This does not affect builds since only the dev server can enable + // the internal option. + metafile.inputs = {}; + additionalResults.set(stylesheetFile, { + outputFiles, + metafile, + }); } - (result.warnings ??= []).push(...warnings); - additionalResults.set(stylesheetFile, { - outputFiles, - metafile, - }); } function createCompilerOptionsTransformer( 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 37434a68b949..5d6f88fb99a9 100644 --- a/packages/angular/build/src/tools/esbuild/angular/component-stylesheets.ts +++ b/packages/angular/build/src/tools/esbuild/angular/component-stylesheets.ts @@ -6,11 +6,15 @@ * found in the LICENSE file at https://angular.dev/license */ -import { OutputFile } from 'esbuild'; import assert from 'node:assert'; import { createHash } from 'node:crypto'; import path from 'node:path'; -import { BuildOutputFileType, BundleContextResult, BundlerContext } from '../bundler-context'; +import { + BuildOutputFile, + BuildOutputFileType, + BundleContextResult, + BundlerContext, +} from '../bundler-context'; import { MemoryCache } from '../cache'; import { BundleStylesheetOptions, @@ -37,7 +41,14 @@ export class ComponentStylesheetBundler { private readonly incremental: boolean, ) {} - async bundleFile(entry: string, externalId?: string | boolean) { + /** + * Bundle a file-based component stylesheet for use within an AOT compiled Angular application. + * @param entry The file path of the stylesheet. + * @param externalId Either an external identifier string for initial bundling or a boolean for rebuilds, if external. + * @param direct If true, the output will be used directly by the builder; false if used inside the compiler plugin. + * @returns A component bundle result object. + */ + async bundleFile(entry: string, externalId?: string | boolean, direct?: boolean) { const bundlerContext = await this.#fileContexts.getOrCreate(entry, () => { return new BundlerContext(this.options.workspaceRoot, this.incremental, (loadCache) => { const buildOptions = createStylesheetBundleOptions(this.options, loadCache); @@ -62,6 +73,7 @@ export class ComponentStylesheetBundler { await bundlerContext.bundle(), bundlerContext.watchFiles, !!externalId, + !!direct, ); } @@ -127,6 +139,7 @@ export class ComponentStylesheetBundler { await bundlerContext.bundle(), bundlerContext.watchFiles, !!externalId, + false, ); } @@ -156,6 +169,15 @@ export class ComponentStylesheetBundler { return entries; } + collectReferencedFiles(): string[] { + const files = []; + for (const context of this.#fileContexts.values()) { + files.push(...context.watchFiles); + } + + return files; + } + async dispose(): Promise { const contexts = [...this.#fileContexts.values(), ...this.#inlineContexts.values()]; this.#fileContexts.clear(); @@ -168,61 +190,70 @@ export class ComponentStylesheetBundler { result: BundleContextResult, referencedFiles: Set | undefined, external: boolean, + direct: boolean, ) { let contents = ''; - let metafile; - const outputFiles: OutputFile[] = []; + const outputFiles: BuildOutputFile[] = []; - if (!result.errors) { - for (const outputFile of result.outputFiles) { - const filename = path.basename(outputFile.path); + const { errors, warnings } = result; + if (errors) { + return { errors, warnings, referencedFiles }; + } - if (outputFile.type === BuildOutputFileType.Media || filename.endsWith('.css.map')) { - // The output files could also contain resources (images/fonts/etc.) that were referenced and the map files. + for (const outputFile of result.outputFiles) { + const filename = path.basename(outputFile.path); - // Clone the output file to avoid amending the original path which would causes problems during rebuild. - const clonedOutputFile = outputFile.clone(); + if (outputFile.type === BuildOutputFileType.Media || filename.endsWith('.css.map')) { + // The output files could also contain resources (images/fonts/etc.) that were referenced and the map files. + + // Clone the output file to avoid amending the original path which would causes problems during rebuild. + const clonedOutputFile = outputFile.clone(); - // Needed for Bazel as otherwise the files will not be written in the correct place, - // this is because esbuild will resolve the output file from the outdir which is currently set to `workspaceRoot` twice, - // once in the stylesheet and the other in the application code bundler. - // Ex: `../../../../../app.component.css.map`. + // Needed for Bazel as otherwise the files will not be written in the correct place, + // this is because esbuild will resolve the output file from the outdir which is currently set to `workspaceRoot` twice, + // once in the stylesheet and the other in the application code bundler. + // Ex: `../../../../../app.component.css.map`. + if (!direct) { clonedOutputFile.path = path.join(this.options.workspaceRoot, outputFile.path); + } - outputFiles.push(clonedOutputFile); - } else if (filename.endsWith('.css')) { - if (external) { - const clonedOutputFile = outputFile.clone(); + outputFiles.push(clonedOutputFile); + } else if (filename.endsWith('.css')) { + if (external) { + const clonedOutputFile = outputFile.clone(); + if (!direct) { clonedOutputFile.path = path.join(this.options.workspaceRoot, outputFile.path); - outputFiles.push(clonedOutputFile); - contents = path.posix.join(this.options.publicPath ?? '', filename); - } else { - contents = outputFile.text; } + outputFiles.push(clonedOutputFile); + contents = path.posix.join(this.options.publicPath ?? '', filename); } else { - throw new Error( - `Unexpected non CSS/Media file "${filename}" outputted during component stylesheet processing.`, - ); + contents = outputFile.text; } + } else { + throw new Error( + `Unexpected non CSS/Media file "${filename}" outputted during component stylesheet processing.`, + ); } - - metafile = result.metafile; - // Remove entryPoint fields from outputs to prevent the internal component styles from being - // treated as initial files. Also mark the entry as a component resource for stat reporting. - Object.values(metafile.outputs).forEach((output) => { - delete output.entryPoint; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (output as any)['ng-component'] = true; - }); } + const metafile = result.metafile; + // Remove entryPoint fields from outputs to prevent the internal component styles from being + // treated as initial files. Also mark the entry as a component resource for stat reporting. + Object.values(metafile.outputs).forEach((output) => { + delete output.entryPoint; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (output as any)['ng-component'] = true; + }); + return { - errors: result.errors, - warnings: result.warnings, + errors, + warnings, contents, outputFiles, metafile, referencedFiles, + externalImports: result.externalImports, + initialFiles: new Map(), }; } } diff --git a/packages/angular/build/src/tools/esbuild/angular/jit-plugin-callbacks.ts b/packages/angular/build/src/tools/esbuild/angular/jit-plugin-callbacks.ts index b70baa48d1d6..2be8670f61b6 100644 --- a/packages/angular/build/src/tools/esbuild/angular/jit-plugin-callbacks.ts +++ b/packages/angular/build/src/tools/esbuild/angular/jit-plugin-callbacks.ts @@ -116,8 +116,16 @@ export function setupJitPluginCallbacks( stylesheetResult = await stylesheetBundler.bundleInline(entry.contents, entry.path); } - const { contents, outputFiles, errors, warnings, metafile, referencedFiles } = - stylesheetResult; + const { errors, warnings, referencedFiles } = stylesheetResult; + if (stylesheetResult.errors) { + return { + errors, + warnings, + watchFiles: referencedFiles && [...referencedFiles], + }; + } + + const { contents, outputFiles, metafile } = stylesheetResult; additionalResultFiles.set(entry.path, { outputFiles, metafile });