From 03ce964858bcb5847012be73e47da9009fe9a0e4 Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Thu, 16 Nov 2023 17:06:59 -0500 Subject: [PATCH] refactor(@angular-devkit/build-angular): use native path normalization for angular compiler caching The path comparisons for the TypeScript emit file lookups now use the native path normalization functions. This removes the special handling previously used and better ensures that cache checks are valid. The separate babel cache Map has also been combined with the already present memory load cache within the Angular plugin. This removes the need for extra handling of another Map and allows it to use the common logic of the load cache. To support this usage, the load cache will also now automatically include the request path in the file dependencies if the request is from the `file` namespace. This removes the need to manually specify the request path file for each load result return value. --- .../tools/esbuild/angular/compiler-plugin.ts | 59 ++++++++++--------- .../esbuild/angular/source-file-cache.ts | 6 +- .../src/tools/esbuild/load-result-cache.ts | 9 ++- 3 files changed, 40 insertions(+), 34 deletions(-) diff --git a/packages/angular_devkit/build_angular/src/tools/esbuild/angular/compiler-plugin.ts b/packages/angular_devkit/build_angular/src/tools/esbuild/angular/compiler-plugin.ts index 102d674db1..9e6546c382 100644 --- a/packages/angular_devkit/build_angular/src/tools/esbuild/angular/compiler-plugin.ts +++ b/packages/angular_devkit/build_angular/src/tools/esbuild/angular/compiler-plugin.ts @@ -18,10 +18,9 @@ import type { import assert from 'node:assert'; import { realpath } from 'node:fs/promises'; import * as path from 'node:path'; -import { pathToFileURL } from 'node:url'; import { maxWorkers } from '../../../utils/environment-options'; import { JavaScriptTransformer } from '../javascript-transformer'; -import { LoadResultCache } from '../load-result-cache'; +import { LoadResultCache, createCachedLoad } from '../load-result-cache'; import { logCumulativeDurations, profileAsync, resetCumulativeDurations } from '../profiling'; import { BundleStylesheetOptions } from '../stylesheets/bundle-options'; import { AngularHostOptions } from './angular-host'; @@ -280,7 +279,7 @@ export function createCompilerPlugin( try { await profileAsync('NG_EMIT_TS', async () => { for (const { filename, contents } of await compilation.emitAffectedFiles()) { - typeScriptFileCache.set(pathToFileURL(filename).href, contents); + typeScriptFileCache.set(path.normalize(filename), contents); } }); } catch (error) { @@ -323,7 +322,9 @@ export function createCompilerPlugin( }); build.onLoad({ filter: /\.[cm]?[jt]sx?$/ }, async (args) => { - const request = pluginOptions.fileReplacements?.[args.path] ?? args.path; + const request = path.normalize( + pluginOptions.fileReplacements?.[path.normalize(args.path)] ?? args.path, + ); // Skip TS load attempt if JS TypeScript compilation not enabled and file is JS if (shouldTsIgnoreJs && /\.[cm]?js$/.test(request)) { @@ -334,7 +335,7 @@ export function createCompilerPlugin( // the options cannot change and do not need to be represented in the key. If the // cache is later stored to disk, then the options that affect transform output // would need to be added to the key as well as a check for any change of content. - let contents = typeScriptFileCache.get(pathToFileURL(request).href); + let contents = typeScriptFileCache.get(request); if (contents === undefined) { // If the Angular compilation had errors the file may not have been emitted. @@ -364,7 +365,7 @@ export function createCompilerPlugin( ); // Store as the returned Uint8Array to allow caching the fully transformed code - typeScriptFileCache.set(pathToFileURL(request).href, contents); + typeScriptFileCache.set(request, contents); } return { @@ -373,27 +374,25 @@ export function createCompilerPlugin( }; }); - build.onLoad({ filter: /\.[cm]?js$/ }, (args) => - profileAsync( - 'NG_EMIT_JS*', - async () => { - // The filename is currently used as a cache key. Since the cache is memory only, - // the options cannot change and do not need to be represented in the key. If the - // cache is later stored to disk, then the options that affect transform output - // would need to be added to the key as well as a check for any change of content. - let contents = pluginOptions.sourceFileCache?.babelFileCache.get(args.path); - if (contents === undefined) { - contents = await javascriptTransformer.transformFile(args.path, pluginOptions.jit); - pluginOptions.sourceFileCache?.babelFileCache.set(args.path, contents); - } + build.onLoad( + { filter: /\.[cm]?js$/ }, + createCachedLoad(pluginOptions.loadResultCache, async (args) => { + return profileAsync( + 'NG_EMIT_JS*', + async () => { + const contents = await javascriptTransformer.transformFile( + args.path, + pluginOptions.jit, + ); - return { - contents, - loader: 'js', - }; - }, - true, - ), + return { + contents, + loader: 'js', + }; + }, + true, + ); + }), ); // Setup bundling of component templates and stylesheets when in JIT mode @@ -531,8 +530,9 @@ function bundleWebWorker( } function createMissingFileError(request: string, original: string, root: string): PartialMessage { + const relativeRequest = path.relative(root, request); const error = { - text: `File '${path.relative(root, request)}' is missing from the TypeScript compilation.`, + text: `File '${relativeRequest}' is missing from the TypeScript compilation.`, notes: [ { text: `Ensure the file is part of the TypeScript program via the 'files' or 'include' property.`, @@ -540,9 +540,10 @@ function createMissingFileError(request: string, original: string, root: string) ], }; - if (request !== original) { + const relativeOriginal = path.relative(root, original); + if (relativeRequest !== relativeOriginal) { error.notes.push({ - text: `File is requested from a file replacement of '${path.relative(root, original)}'.`, + text: `File is requested from a file replacement of '${relativeOriginal}'.`, }); } diff --git a/packages/angular_devkit/build_angular/src/tools/esbuild/angular/source-file-cache.ts b/packages/angular_devkit/build_angular/src/tools/esbuild/angular/source-file-cache.ts index 379c2a6ad7..032ebc988c 100644 --- a/packages/angular_devkit/build_angular/src/tools/esbuild/angular/source-file-cache.ts +++ b/packages/angular_devkit/build_angular/src/tools/esbuild/angular/source-file-cache.ts @@ -8,7 +8,6 @@ import { platform } from 'node:os'; import * as path from 'node:path'; -import { pathToFileURL } from 'node:url'; import type ts from 'typescript'; import { MemoryLoadResultCache } from '../load-result-cache'; @@ -17,7 +16,6 @@ const WINDOWS_SEP_REGEXP = new RegExp(`\\${path.win32.sep}`, 'g'); export class SourceFileCache extends Map { readonly modifiedFiles = new Set(); - readonly babelFileCache = new Map(); readonly typeScriptFileCache = new Map(); readonly loadResultCache = new MemoryLoadResultCache(); @@ -32,8 +30,8 @@ export class SourceFileCache extends Map { this.modifiedFiles.clear(); } for (let file of files) { - this.babelFileCache.delete(file); - this.typeScriptFileCache.delete(pathToFileURL(file).href); + file = path.normalize(file); + this.typeScriptFileCache.delete(file); this.loadResultCache.invalidate(file); // Normalize separators to allow matching TypeScript Host paths diff --git a/packages/angular_devkit/build_angular/src/tools/esbuild/load-result-cache.ts b/packages/angular_devkit/build_angular/src/tools/esbuild/load-result-cache.ts index 78553d3148..2649532429 100644 --- a/packages/angular_devkit/build_angular/src/tools/esbuild/load-result-cache.ts +++ b/packages/angular_devkit/build_angular/src/tools/esbuild/load-result-cache.ts @@ -32,6 +32,11 @@ export function createCachedLoad( // Do not cache null or undefined if (result) { + // Ensure requested path is included if it was a resolved file + if (args.namespace === 'file') { + result.watchFiles ??= []; + result.watchFiles.push(args.path); + } await cache.put(loadCacheKey, result); } } @@ -79,6 +84,8 @@ export class MemoryLoadResultCache implements LoadResultCache { } get watchFiles(): string[] { - return [...this.#loadResults.keys(), ...this.#fileDependencies.keys()]; + // this.#loadResults.keys() is not included here because the keys + // are namespaced request paths and not disk-based file paths. + return [...this.#fileDependencies.keys()]; } }