refactor(@angular-devkit/build-angular): use helper to setup esbuild plugin load caching

clydin · alan-agius4 · commit ffea33fc4575 · 2023-05-19T12:18:03.000+02:00
Within the esbuild-based browser application builder, a helper function has been introduced
to streamline the use of the load result cache within the internal plugins. This removes
repeat code that would otherwise be needed. The ability to use a load result cache with the
global script processing has also been added but has not yet been enabled.
diff --git a/packages/angular_devkit/build_angular/src/builders/browser-esbuild/global-scripts.ts b/packages/angular_devkit/build_angular/src/builders/browser-esbuild/global-scripts.ts
@@ -12,7 +12,8 @@ import assert from 'node:assert';
 import { readFile } from 'node:fs/promises';
 import path from 'node:path';
 import { assertIsError } from '../../utils/error';
-import { NormalizedBrowserOptions } from './options';
+import { LoadResultCache, createCachedLoad } from './load-result-cache';
+import type { NormalizedBrowserOptions } from './options';
 import { createSourcemapIngorelistPlugin } from './sourcemap-ignorelist-plugin';
 
 /**
@@ -24,6 +25,7 @@ import { createSourcemapIngorelistPlugin } from './sourcemap-ignorelist-plugin';
 export function createGlobalScriptsBundleOptions(
   options: NormalizedBrowserOptions,
   initial: boolean,
+  loadCache?: LoadResultCache,
 ): BuildOptions | undefined {
   const {
     globalScripts,
@@ -91,51 +93,60 @@ export function createGlobalScriptsBundleOptions(
               external: true,
             };
           });
-          build.onLoad({ filter: /./, namespace }, async (args) => {
-            const files = globalScripts.find(({ name }) => name === args.path.slice(0, -3))?.files;
-            assert(files, `Invalid operation: global scripts name not found [${args.path}]`);
+          build.onLoad(
+            { filter: /./, namespace },
+            createCachedLoad(loadCache, async (args) => {
+              const files = globalScripts.find(
+                ({ name }) => name === args.path.slice(0, -3),
+              )?.files;
+              assert(files, `Invalid operation: global scripts name not found [${args.path}]`);
 
-            // Global scripts are concatenated using magic-string instead of bundled via esbuild.
-            const bundleContent = new Bundle();
-            for (const filename of files) {
-              let fileContent;
-              try {
-                // Attempt to read as a relative path from the workspace root
-                fileContent = await readFile(path.join(workspaceRoot, filename), 'utf-8');
-              } catch (e) {
-                assertIsError(e);
-                if (e.code !== 'ENOENT') {
-                  throw e;
-                }
+              // Global scripts are concatenated using magic-string instead of bundled via esbuild.
+              const bundleContent = new Bundle();
+              const watchFiles = [];
+              for (const filename of files) {
+                let fileContent;
+                try {
+                  // Attempt to read as a relative path from the workspace root
+                  fileContent = await readFile(path.join(workspaceRoot, filename), 'utf-8');
+                  watchFiles.push(filename);
+                } catch (e) {
+                  assertIsError(e);
+                  if (e.code !== 'ENOENT') {
+                    throw e;
+                  }
+
+                  // If not found attempt to resolve as a module specifier
+                  const resolveResult = await build.resolve(filename, {
+                    kind: 'entry-point',
+                    resolveDir: workspaceRoot,
+                  });
 
-                // If not found attempt to resolve as a module specifier
-                const resolveResult = await build.resolve(filename, {
-                  kind: 'entry-point',
-                  resolveDir: workspaceRoot,
-                });
+                  if (resolveResult.errors.length) {
+                    // Remove resolution failure notes about marking as external since it doesn't apply
+                    // to global scripts.
+                    resolveResult.errors.forEach((error) => (error.notes = []));
 
-                if (resolveResult.errors.length) {
-                  // Remove resolution failure notes about marking as external since it doesn't apply
-                  // to global scripts.
-                  resolveResult.errors.forEach((error) => (error.notes = []));
+                    return {
+                      errors: resolveResult.errors,
+                      warnings: resolveResult.warnings,
+                    };
+                  }
 
-                  return {
-                    errors: resolveResult.errors,
-                    warnings: resolveResult.warnings,
-                  };
+                  watchFiles.push(path.relative(resolveResult.path, workspaceRoot));
+                  fileContent = await readFile(resolveResult.path, 'utf-8');
                 }
 
-                fileContent = await readFile(resolveResult.path, 'utf-8');
+                bundleContent.addSource(new MagicString(fileContent, { filename }));
               }
 
-              bundleContent.addSource(new MagicString(fileContent, { filename }));
-            }
-
-            return {
-              contents: bundleContent.toString(),
-              loader: 'js',
-            };
-          });
+              return {
+                contents: bundleContent.toString(),
+                loader: 'js',
+                watchFiles,
+              };
+            }),
+          );
         },
       },
     ],
diff --git a/packages/angular_devkit/build_angular/src/builders/browser-esbuild/load-result-cache.ts b/packages/angular_devkit/build_angular/src/builders/browser-esbuild/load-result-cache.ts
@@ -6,13 +6,38 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import type { OnLoadResult } from 'esbuild';
+import type { OnLoadResult, PluginBuild } from 'esbuild';
 
 export interface LoadResultCache {
   get(path: string): OnLoadResult | undefined;
   put(path: string, result: OnLoadResult): Promise<void>;
 }
 
+export function createCachedLoad(
+  cache: LoadResultCache | undefined,
+  callback: Parameters<PluginBuild['onLoad']>[1],
+): Parameters<PluginBuild['onLoad']>[1] {
+  if (cache === undefined) {
+    return callback;
+  }
+
+  return async (args) => {
+    const loadCacheKey = `${args.namespace}:${args.path}`;
+    let result: OnLoadResult | null | undefined = cache.get(loadCacheKey);
+
+    if (result === undefined) {
+      result = await callback(args);
+
+      // Do not cache null or undefined or results with errors
+      if (result && result.errors === undefined) {
+        await cache.put(loadCacheKey, result);
+      }
+    }
+
+    return result;
+  };
+}
+
 export class MemoryLoadResultCache implements LoadResultCache {
   #loadResults = new Map<string, OnLoadResult>();
   #fileDependencies = new Map<string, Set<string>>();
diff --git a/packages/angular_devkit/build_angular/src/builders/browser-esbuild/stylesheets/bundle-options.ts b/packages/angular_devkit/build_angular/src/builders/browser-esbuild/stylesheets/bundle-options.ts
@@ -7,6 +7,7 @@
  */
 
 import type { BuildOptions, OutputFile } from 'esbuild';
+import { createHash } from 'node:crypto';
 import path from 'node:path';
 import { BundlerContext } from '../esbuild';
 import { LoadResultCache } from '../load-result-cache';
@@ -108,7 +109,11 @@ export async function bundleComponentStylesheet(
   cache?: LoadResultCache,
 ) {
   const namespace = 'angular:styles/component';
-  const entry = [language, componentStyleCounter++, filename].join(';');
+  // Use a hash of the inline stylesheet content to ensure a consistent identifier. External stylesheets will resolve
+  // to the actual stylesheet file path.
+  // TODO: Consider xxhash instead for hashing
+  const id = inline ? createHash('sha256').update(data).digest('hex') : componentStyleCounter++;
+  const entry = [language, id, filename].join(';');
 
   const buildOptions = createStylesheetBundleOptions(options, cache, { [entry]: data });
   buildOptions.entryPoints = [`${namespace};${entry}`];
diff --git a/packages/angular_devkit/build_angular/src/builders/browser-esbuild/stylesheets/sass-plugin.ts b/packages/angular_devkit/build_angular/src/builders/browser-esbuild/stylesheets/sass-plugin.ts
@@ -16,7 +16,7 @@ import type {
   FileImporterWithRequestContextOptions,
   SassWorkerImplementation,
 } from '../../../sass/sass-service';
-import type { LoadResultCache } from '../load-result-cache';
+import { LoadResultCache, createCachedLoad } from '../load-result-cache';
 
 export interface SassPluginOptions {
   sourcemap: boolean;
@@ -63,43 +63,33 @@ export function createSassPlugin(options: SassPluginOptions, cache?: LoadResultC
         return result;
       };
 
-      build.onLoad({ filter: /^s[ac]ss;/, namespace: 'angular:styles/component' }, async (args) => {
-        const data = options.inlineComponentData?.[args.path];
-        assert(
-          typeof data === 'string',
-          `component style name should always be found [${args.path}]`,
-        );
+      // Load inline component stylesheets
+      build.onLoad(
+        { filter: /^s[ac]ss;/, namespace: 'angular:styles/component' },
+        createCachedLoad(cache, async (args) => {
+          const data = options.inlineComponentData?.[args.path];
+          assert(
+            typeof data === 'string',
+            `component style name should always be found [${args.path}]`,
+          );
 
-        let result = cache?.get(data);
-        if (result === undefined) {
           const [language, , filePath] = args.path.split(';', 3);
           const syntax = language === 'sass' ? 'indented' : 'scss';
 
-          result = await compileString(data, filePath, syntax, options, resolveUrl);
-          if (result.errors === undefined) {
-            // Cache the result if there were no errors
-            await cache?.put(data, result);
-          }
-        }
-
-        return result;
-      });
+          return compileString(data, filePath, syntax, options, resolveUrl);
+        }),
+      );
 
-      build.onLoad({ filter: /\.s[ac]ss$/ }, async (args) => {
-        let result = cache?.get(args.path);
-        if (result === undefined) {
+      // Load file stylesheets
+      build.onLoad(
+        { filter: /\.s[ac]ss$/ },
+        createCachedLoad(cache, async (args) => {
           const data = await readFile(args.path, 'utf-8');
           const syntax = extname(args.path).toLowerCase() === '.sass' ? 'indented' : 'scss';
 
-          result = await compileString(data, args.path, syntax, options, resolveUrl);
-          if (result.errors === undefined) {
-            // Cache the result if there were no errors
-            await cache?.put(args.path, result);
-          }
-        }
-
-        return result;
-      });
+          return compileString(data, args.path, syntax, options, resolveUrl);
+        }),
+      );
     },
   };
 }
