From 44efd604747553a13f9f34ab51d0cb78ba2e0b8a Mon Sep 17 00:00:00 2001 From: Alan Agius Date: Wed, 8 Jan 2025 08:04:20 +0000 Subject: [PATCH 1/2] fix(@angular/build): remove deleted assets from output during watch mode This commit ensures that assets deleted from the source are also removed from the output directory while in watch mode. Previously, deleted assets could persist in the output folder, potentially causing inconsistencies or outdated files to be served. This fix improves the accuracy of the build output by maintaining synchronization between the source and the output directory during development. --- .../src/builders/application/build-action.ts | 49 +++++++++++++------ .../tests/behavior/rebuild-assets_spec.ts | 45 +++++++++++++++++ .../tools/esbuild/bundler-execution-result.ts | 8 ++- 3 files changed, 84 insertions(+), 18 deletions(-) diff --git a/packages/angular/build/src/builders/application/build-action.ts b/packages/angular/build/src/builders/application/build-action.ts index f5726746825c..2b32e4a4bfba 100644 --- a/packages/angular/build/src/builders/application/build-action.ts +++ b/packages/angular/build/src/builders/application/build-action.ts @@ -13,6 +13,7 @@ import { BuildOutputFileType } from '../../tools/esbuild/bundler-context'; import { ExecutionResult, RebuildState } from '../../tools/esbuild/bundler-execution-result'; import { shutdownSassWorkerPool } from '../../tools/esbuild/stylesheets/sass-language'; import { logMessages, withNoProgress, withSpinner } from '../../tools/esbuild/utils'; +import { ChangedFiles } from '../../tools/esbuild/watcher'; import { shouldWatchRoot } from '../../utils/environment-options'; import { NormalizedCachedOptions } from '../../utils/normalize-cache'; import { NormalizedApplicationBuildOptions, NormalizedOutputOptions } from './options'; @@ -199,7 +200,8 @@ export async function* runEsBuildBuildAction( for (const outputResult of emitOutputResults( result, outputOptions, - incrementalResults ? rebuildState.previousOutputInfo : undefined, + changes, + incrementalResults ? rebuildState : undefined, )) { yield outputResult; } @@ -224,7 +226,8 @@ function* emitOutputResults( templateUpdates, }: ExecutionResult, outputOptions: NormalizedApplicationBuildOptions['outputOptions'], - previousOutputInfo?: ReadonlyMap, + changes?: ChangedFiles, + rebuildState?: RebuildState, ): Iterable { if (errors.length > 0) { yield { @@ -255,7 +258,9 @@ function* emitOutputResults( } // Use an incremental result if previous output information is available - if (previousOutputInfo) { + if (rebuildState && changes) { + const { previousAssetsInfo, previousOutputInfo } = rebuildState; + const incrementalResult: IncrementalResult = { kind: ResultKind.Incremental, warnings: warnings as ResultMessage[], @@ -273,7 +278,6 @@ function* emitOutputResults( // Initially assume all previous output files have been removed const removedOutputFiles = new Map(previousOutputInfo); - for (const file of outputFiles) { removedOutputFiles.delete(file.path); @@ -304,24 +308,37 @@ function* emitOutputResults( } } - // Include the removed output files + // Initially assume all previous assets files have been removed + const removedAssetFiles = new Map(previousAssetsInfo); + for (const { source, destination } of assetFiles) { + removedAssetFiles.delete(source); + + if (changes.modified.has(source)) { + incrementalResult.modified.push(destination); + } else if (!previousAssetsInfo.has(source)) { + incrementalResult.added.push(destination); + } else { + continue; + } + + incrementalResult.files[destination] = { + type: BuildOutputFileType.Browser, + inputPath: source, + origin: 'disk', + }; + } + + // Include the removed output and asset files incrementalResult.removed.push( ...Array.from(removedOutputFiles, ([file, { type }]) => ({ path: file, type, })), - ); - - // Always consider asset files as added to ensure new/modified assets are available. - // TODO: Consider more comprehensive asset analysis. - for (const file of assetFiles) { - incrementalResult.added.push(file.destination); - incrementalResult.files[file.destination] = { + ...Array.from(removedAssetFiles.values(), (file) => ({ + path: file, type: BuildOutputFileType.Browser, - inputPath: file.source, - origin: 'disk', - }; - } + })), + ); yield incrementalResult; diff --git a/packages/angular/build/src/builders/application/tests/behavior/rebuild-assets_spec.ts b/packages/angular/build/src/builders/application/tests/behavior/rebuild-assets_spec.ts index 6bf7763d2e4b..a48c19fd1baf 100644 --- a/packages/angular/build/src/builders/application/tests/behavior/rebuild-assets_spec.ts +++ b/packages/angular/build/src/builders/application/tests/behavior/rebuild-assets_spec.ts @@ -61,5 +61,50 @@ describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => { expect(buildCount).toBe(2); }); + + it('remove deleted asset from output', async () => { + await Promise.all([ + harness.writeFile('public/asset-two.txt', 'bar'), + harness.writeFile('public/asset-one.txt', 'foo'), + ]); + + harness.useTarget('build', { + ...BASE_OPTIONS, + assets: [ + { + glob: '**/*', + input: 'public', + }, + ], + watch: true, + }); + + const buildCount = await harness + .execute({ outputLogsOnFailure: false }) + .pipe( + timeout(BUILD_TIMEOUT), + concatMap(async ({ result }, index) => { + switch (index) { + case 0: + expect(result?.success).toBeTrue(); + harness.expectFile('dist/browser/asset-one.txt').toExist(); + harness.expectFile('dist/browser/asset-two.txt').toExist(); + + await harness.removeFile('public/asset-two.txt'); + break; + case 1: + expect(result?.success).toBeTrue(); + harness.expectFile('dist/browser/asset-one.txt').toExist(); + harness.expectFile('dist/browser/asset-two.txt').toNotExist(); + break; + } + }), + take(2), + count(), + ) + .toPromise(); + + expect(buildCount).toBe(2); + }); }); }); 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 60fe2966c5c7..41fb62721992 100644 --- a/packages/angular/build/src/tools/esbuild/bundler-execution-result.ts +++ b/packages/angular/build/src/tools/esbuild/bundler-execution-result.ts @@ -27,7 +27,8 @@ export interface RebuildState { componentStyleBundler: ComponentStylesheetBundler; codeBundleCache?: SourceFileCache; fileChanges: ChangedFiles; - previousOutputInfo: Map; + previousOutputInfo: ReadonlyMap; + previousAssetsInfo: ReadonlyMap; templateUpdates?: Map; } @@ -172,12 +173,15 @@ export class ExecutionResult { previousOutputInfo: new Map( this.outputFiles.map(({ path, hash, type }) => [path, { hash, type }]), ), + previousAssetsInfo: new Map( + this.assetFiles.map(({ source, destination }) => [source, destination]), + ), templateUpdates: this.templateUpdates, }; } findChangedFiles( - previousOutputHashes: Map, + previousOutputHashes: ReadonlyMap, ): Set { const changed = new Set(); for (const file of this.outputFiles) { From 925e22bf17748a9ab4ba2c01c7174756bf3a0b37 Mon Sep 17 00:00:00 2001 From: Alan Agius Date: Fri, 10 Jan 2025 18:50:10 +0000 Subject: [PATCH 2/2] fix(@angular/build): trigger browser reload on asset changes with Vite dev server Ensures the Vite-based development server automatically reloads the browser when asset files are modified. Closes #26141 --- .../src/builders/application/build-action.ts | 6 +-- .../src/builders/dev-server/vite-server.ts | 52 +++++++++++++++---- .../vite/middlewares/assets-middleware.ts | 12 ++--- .../vite/plugins/setup-middlewares-plugin.ts | 4 +- .../angular/build/src/tools/vite/utils.ts | 2 + 5 files changed, 54 insertions(+), 22 deletions(-) diff --git a/packages/angular/build/src/builders/application/build-action.ts b/packages/angular/build/src/builders/application/build-action.ts index 2b32e4a4bfba..4d3120da10f4 100644 --- a/packages/angular/build/src/builders/application/build-action.ts +++ b/packages/angular/build/src/builders/application/build-action.ts @@ -313,10 +313,10 @@ function* emitOutputResults( for (const { source, destination } of assetFiles) { removedAssetFiles.delete(source); - if (changes.modified.has(source)) { - incrementalResult.modified.push(destination); - } else if (!previousAssetsInfo.has(source)) { + if (!previousAssetsInfo.has(source)) { incrementalResult.added.push(destination); + } else if (changes.modified.has(source)) { + incrementalResult.modified.push(destination); } else { continue; } diff --git a/packages/angular/build/src/builders/dev-server/vite-server.ts b/packages/angular/build/src/builders/dev-server/vite-server.ts index 89df08e7404f..439cebda2b03 100644 --- a/packages/angular/build/src/builders/dev-server/vite-server.ts +++ b/packages/angular/build/src/builders/dev-server/vite-server.ts @@ -49,6 +49,11 @@ interface OutputFileRecord { type: BuildOutputFileType; } +interface OutputAssetRecord { + source: string; + updated: boolean; +} + interface DevServerExternalResultMetadata extends Omit { explicitBrowser: string[]; explicitServer: string[]; @@ -168,7 +173,7 @@ export async function* serveWithVite( let serverUrl: URL | undefined; let hadError = false; const generatedFiles = new Map(); - const assetFiles = new Map(); + const assetFiles = new Map(); const externalMetadata: DevServerExternalResultMetadata = { implicitBrowser: [], implicitServer: [], @@ -229,19 +234,15 @@ export async function* serveWithVite( assetFiles.clear(); componentStyles.clear(); generatedFiles.clear(); - for (const entry of Object.entries(result.files)) { - const [outputPath, file] = entry; - if (file.origin === 'disk') { - assetFiles.set('/' + normalizePath(outputPath), normalizePath(file.inputPath)); - continue; - } + for (const [outputPath, file] of Object.entries(result.files)) { updateResultRecord( outputPath, file, normalizePath, htmlIndexPath, generatedFiles, + assetFiles, componentStyles, // The initial build will not yet have a server setup !server, @@ -265,6 +266,7 @@ export async function* serveWithVite( generatedFiles.delete(filePath); assetFiles.delete(filePath); } + for (const modified of result.modified) { updateResultRecord( modified, @@ -272,9 +274,11 @@ export async function* serveWithVite( normalizePath, htmlIndexPath, generatedFiles, + assetFiles, componentStyles, ); } + for (const added of result.added) { updateResultRecord( added, @@ -282,6 +286,7 @@ export async function* serveWithVite( normalizePath, htmlIndexPath, generatedFiles, + assetFiles, componentStyles, ); } @@ -352,12 +357,16 @@ export async function* serveWithVite( if (server) { // Update fs allow list to include any new assets from the build option. server.config.server.fs.allow = [ - ...new Set([...server.config.server.fs.allow, ...assetFiles.values()]), + ...new Set([ + ...server.config.server.fs.allow, + ...[...assetFiles.values()].map(({ source }) => source), + ]), ]; await handleUpdate( normalizePath, generatedFiles, + assetFiles, server, serverOptions, context.logger, @@ -471,15 +480,26 @@ export async function* serveWithVite( async function handleUpdate( normalizePath: (id: string) => string, generatedFiles: Map, + assetFiles: Map, server: ViteDevServer, serverOptions: NormalizedDevServerOptions, logger: BuilderContext['logger'], componentStyles: Map, ): Promise { const updatedFiles: string[] = []; - let destroyAngularServerAppCalled = false; + + // Invalidate any updated asset + for (const [file, record] of assetFiles) { + if (!record.updated) { + continue; + } + + record.updated = false; + updatedFiles.push(file); + } // Invalidate any updated files + let destroyAngularServerAppCalled = false; for (const [file, record] of generatedFiles) { if (!record.updated) { continue; @@ -584,10 +604,16 @@ function updateResultRecord( normalizePath: (id: string) => string, htmlIndexPath: string, generatedFiles: Map, + assetFiles: Map, componentStyles: Map, initial = false, ): void { if (file.origin === 'disk') { + assetFiles.set('/' + normalizePath(outputPath), { + source: normalizePath(file.inputPath), + updated: !initial, + }); + return; } @@ -644,7 +670,7 @@ function updateResultRecord( export async function setupServer( serverOptions: NormalizedDevServerOptions, outputFiles: Map, - assets: Map, + assets: Map, preserveSymlinks: boolean | undefined, externalMetadata: DevServerExternalResultMetadata, ssrMode: ServerSsrMode, @@ -743,7 +769,11 @@ export async function setupServer( // The first two are required for Vite to function in prebundling mode (the default) and to load // the Vite client-side code for browser reloading. These would be available by default but when // the `allow` option is explicitly configured, they must be included manually. - allow: [cacheDir, join(serverOptions.workspaceRoot, 'node_modules'), ...assets.values()], + allow: [ + cacheDir, + join(serverOptions.workspaceRoot, 'node_modules'), + ...[...assets.values()].map(({ source }) => source), + ], }, // This is needed when `externalDependencies` is used to prevent Vite load errors. // NOTE: If Vite adds direct support for externals, this can be removed. diff --git a/packages/angular/build/src/tools/vite/middlewares/assets-middleware.ts b/packages/angular/build/src/tools/vite/middlewares/assets-middleware.ts index b712744e8f33..963fca654d37 100644 --- a/packages/angular/build/src/tools/vite/middlewares/assets-middleware.ts +++ b/packages/angular/build/src/tools/vite/middlewares/assets-middleware.ts @@ -9,7 +9,7 @@ import { lookup as lookupMimeType } from 'mrmime'; import { extname } from 'node:path'; import type { Connect, ViteDevServer } from 'vite'; -import { AngularMemoryOutputFiles, pathnameWithoutBasePath } from '../utils'; +import { AngularMemoryOutputFiles, AngularOutputAssets, pathnameWithoutBasePath } from '../utils'; export interface ComponentStyleRecord { rawContent: Uint8Array; @@ -19,7 +19,7 @@ export interface ComponentStyleRecord { export function createAngularAssetsMiddleware( server: ViteDevServer, - assets: Map, + assets: AngularOutputAssets, outputFiles: AngularMemoryOutputFiles, componentStyles: Map, encapsulateStyle: (style: Uint8Array, componentId: string) => string, @@ -36,8 +36,8 @@ export function createAngularAssetsMiddleware( const pathnameHasTrailingSlash = pathname[pathname.length - 1] === '/'; // Rewrite all build assets to a vite raw fs URL - const assetSourcePath = assets.get(pathname); - if (assetSourcePath !== undefined) { + const asset = assets.get(pathname); + if (asset) { // Workaround to disable Vite transformer middleware. // See: https://github.com/vitejs/vite/blob/746a1daab0395f98f0afbdee8f364cb6cf2f3b3f/packages/vite/src/node/server/middlewares/transform.ts#L201 and // https://github.com/vitejs/vite/blob/746a1daab0395f98f0afbdee8f364cb6cf2f3b3f/packages/vite/src/node/server/transformRequest.ts#L204-L206 @@ -45,7 +45,7 @@ export function createAngularAssetsMiddleware( // The encoding needs to match what happens in the vite static middleware. // ref: https://github.com/vitejs/vite/blob/d4f13bd81468961c8c926438e815ab6b1c82735e/packages/vite/src/node/server/middlewares/static.ts#L163 - req.url = `${server.config.base}@fs/${encodeURI(assetSourcePath)}`; + req.url = `${server.config.base}@fs/${encodeURI(asset.source)}`; next(); return; @@ -61,7 +61,7 @@ export function createAngularAssetsMiddleware( assets.get(pathname + '.html'); if (htmlAssetSourcePath) { - req.url = `${server.config.base}@fs/${encodeURI(htmlAssetSourcePath)}`; + req.url = `${server.config.base}@fs/${encodeURI(htmlAssetSourcePath.source)}`; next(); return; diff --git a/packages/angular/build/src/tools/vite/plugins/setup-middlewares-plugin.ts b/packages/angular/build/src/tools/vite/plugins/setup-middlewares-plugin.ts index 1118cf3ac797..63407c56d3e5 100644 --- a/packages/angular/build/src/tools/vite/plugins/setup-middlewares-plugin.ts +++ b/packages/angular/build/src/tools/vite/plugins/setup-middlewares-plugin.ts @@ -18,7 +18,7 @@ import { createAngularSsrExternalMiddleware, createAngularSsrInternalMiddleware, } from '../middlewares'; -import { AngularMemoryOutputFiles } from '../utils'; +import { AngularMemoryOutputFiles, AngularOutputAssets } from '../utils'; export enum ServerSsrMode { /** @@ -47,7 +47,7 @@ export enum ServerSsrMode { interface AngularSetupMiddlewaresPluginOptions { outputFiles: AngularMemoryOutputFiles; - assets: Map; + assets: AngularOutputAssets; extensionMiddleware?: Connect.NextHandleFunction[]; indexHtmlTransformer?: (content: string) => Promise; componentStyles: Map; diff --git a/packages/angular/build/src/tools/vite/utils.ts b/packages/angular/build/src/tools/vite/utils.ts index fc0d82c2ce62..2b39235dc212 100644 --- a/packages/angular/build/src/tools/vite/utils.ts +++ b/packages/angular/build/src/tools/vite/utils.ts @@ -18,6 +18,8 @@ export type AngularMemoryOutputFiles = Map< { contents: Uint8Array; hash: string; servable: boolean } >; +export type AngularOutputAssets = Map; + export function pathnameWithoutBasePath(url: string, basePath: string): string { const parsedUrl = new URL(url, 'http://localhost'); const pathname = decodeURIComponent(parsedUrl.pathname);