fix(@angular/build): scope Sass package resolution caching to visible node_modules

Package specifiers were cached without any qualification, so the resolution made for one
stylesheet was reused for every other stylesheet in the build. A stylesheet with a nested
`node_modules` directory provided its resolution to stylesheets that cannot see it, a
stylesheet without one provided its resolution to those that can, and a failed resolution
was reused for a stylesheet that is able to resolve the package. Which of these occurred
depended on the order the stylesheets were compiled in.

Package resolutions and package roots are now keyed by the `node_modules` directories
visible from the importing stylesheet. Stylesheets that search the same directories still
share a single resolution, while stylesheets that search different directories no longer
share one.

Author: thekhegay

packages/angular/build/src/tools/esbuild/stylesheets/sass-language.ts

@@ -7,9 +7,10 @@
  */
 
 import type { OnLoadResult, PartialMessage, PartialNote, ResolveResult } from 'esbuild';
+import { existsSync } from 'node:fs';
 import { dirname, join } from 'node:path';
 import { fileURLToPath, pathToFileURL } from 'node:url';
-import type { CanonicalizeContext, CompileResult, Exception, Syntax } from 'sass-embedded';
+import type { CompileResult, Exception, Syntax } from 'sass-embedded';
 import type { SassCompiler } from '../../sass/sass-service';
 import { MemoryCache } from '../cache';
 import { StylesheetLanguage, StylesheetPluginOptions } from './stylesheet-plugin-factory';
@@ -18,6 +19,7 @@ let sassService: SassCompiler | undefined;
 let sassServicePromise: Promise<SassCompiler> | undefined;
 let resolutionCache: MemoryCache<URL | null> | undefined;
 let packageRootCache: MemoryCache<string | null> | undefined;
+let nodeModulesChainCache: Map<string, string> | undefined;
 
 function isSassException(error: unknown): error is Exception {
   return !!error && typeof error === 'object' && 'sassMessage' in error;
@@ -26,6 +28,7 @@ function isSassException(error: unknown): error is Exception {
 export function resetSassWorkerPoolCaches(): void {
   resolutionCache?.clear();
   packageRootCache?.clear();
+  nodeModulesChainCache?.clear();
   if (sassService) {
     sassService.clearCache();
   } else if (sassServicePromise) {
@@ -50,12 +53,7 @@ export const SassStylesheetLanguage = Object.freeze<StylesheetLanguage>({
   fileFilter: /\.s[ac]ss$/,
   process(data, file, format, options, build) {
     const syntax = format === 'sass' ? 'indented' : 'scss';
-    const resolveUrl = async (url: string, options: CanonicalizeContext) => {
-      let resolveDir = build.initialOptions.absWorkingDir;
-      if (options.containingUrl) {
-        resolveDir = dirname(fileURLToPath(options.containingUrl));
-      }
-
+    const resolveUrl = async (url: string, resolveDir: string | undefined) => {
       const path = url.startsWith('pkg:') ? url.slice(4) : url;
       const result = await build.resolve(path, {
         kind: 'import-rule',
@@ -65,7 +63,14 @@ export const SassStylesheetLanguage = Object.freeze<StylesheetLanguage>({
       return result;
     };
 
-    return compileString(data, file, syntax, options, resolveUrl);
+    return compileString(
+      data,
+      file,
+      syntax,
+      options,
+      resolveUrl,
+      build.initialOptions.absWorkingDir,
+    );
   },
 });
 
@@ -97,12 +102,42 @@ function parsePackageName(url: string): { packageName: string; readonly pathSegm
   };
 }
 
+/**
+ * Builds a key describing the `node_modules` directories visible from a directory, in the order
+ * that package resolution searches them. Directories with differing keys search different
+ * locations and must not share a cached package resolution. An identical key does not by itself
+ * guarantee an identical resolution, since esbuild also applies the path mappings of the nearest
+ * `tsconfig.json` and the browser field of the nearest `package.json`, neither of which is
+ * modeled here.
+ *
+ * The result is cached for the directory and for each of its parents.
+ *
+ * @param directory An absolute path of the directory to analyze.
+ * @returns A key representing the `node_modules` directories visible from the directory.
+ */
+function nodeModulesChainKey(directory: string): string {
+  nodeModulesChainCache ??= new Map<string, string>();
+  const cached = nodeModulesChainCache.get(directory);
+  if (cached !== undefined) {
+    return cached;
+  }
+
+  const parent = dirname(directory);
+  const parentKey = parent === directory ? '' : nodeModulesChainKey(parent);
+  const candidate = join(directory, 'node_modules');
+  const key = existsSync(candidate) ? `${candidate}\n${parentKey}` : parentKey;
+  nodeModulesChainCache.set(directory, key);
+
+  return key;
+}
+
 async function compileString(
   data: string,
   filePath: string,
   syntax: Syntax,
   options: StylesheetPluginOptions,
-  resolveUrl: (url: string, options: CanonicalizeContext) => Promise<ResolveResult>,
+  resolveUrl: (url: string, resolveDir: string | undefined) => Promise<ResolveResult>,
+  workingDirectory: string | undefined,
 ): Promise<OnLoadResult> {
   // Lazily load Sass when a Sass file is found
   if (sassService === undefined) {
@@ -119,7 +154,9 @@ async function compileString(
   }
 
   // Caching follows Sass behavior where a given package url will always resolve to the same value
-  // regardless of its importer's path. Relative paths are qualified with the containing URL.
+  // for importers that search the same `node_modules` directories. Package urls are therefore
+  // qualified with the visible `node_modules` directories of the importer instead of its path,
+  // while relative paths are qualified with the containing URL.
   // A null value indicates that the cached resolution attempt failed to find a location and
   // later stage resolution should be attempted. This avoids potentially expensive repeat
   // failing resolution attempts.
@@ -145,11 +182,18 @@ async function compileString(
       importers: [
         {
           findFileUrl: (url, options) => {
+            const resolveDir = options.containingUrl
+              ? dirname(fileURLToPath(options.containingUrl))
+              : workingDirectory;
             const isPackage = isPackageUrl(url);
-            const cacheKey = isPackage ? url : `${options.containingUrl?.href ?? ''}:${url}`;
+            const chainKey =
+              isPackage && resolveDir !== undefined ? nodeModulesChainKey(resolveDir) : '';
+            const cacheKey = isPackage
+              ? `${chainKey}:${url}`
+              : `${options.containingUrl?.href ?? ''}:${url}`;
 
             return currentResolutionCache.getOrCreate(cacheKey, async () => {
-              const result = await resolveUrl(url, options);
+              const result = await resolveUrl(url, resolveDir);
               if (result.path) {
                 return pathToFileURL(result.path);
               }
@@ -164,10 +208,10 @@ async function compileString(
               // Caching package root locations is particularly beneficial for `@material/*` packages
               // which extensively use deep imports.
               const packageRoot = await currentPackageRootCache.getOrCreate(
-                packageName,
+                `${chainKey}:${packageName}`,
                 async () => {
                   // Use the required presence of a package root `package.json` file to resolve the location
-                  const packageResult = await resolveUrl(packageName + '/package.json', options);
+                  const packageResult = await resolveUrl(packageName + '/package.json', resolveDir);
 
                   return packageResult.path ? dirname(packageResult.path) : null;
                 },

packages/angular/build/src/tools/esbuild/stylesheets/sass-language_spec.ts

@@ -6,7 +6,17 @@
  * found in the LICENSE file at https://angular.dev/license
  */
 
-import { isPackageUrl } from './sass-language';
+import type { PluginBuild } from 'esbuild';
+import { statSync } from 'node:fs';
+import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises';
+import { tmpdir } from 'node:os';
+import { dirname, join } from 'node:path';
+import {
+  SassStylesheetLanguage,
+  isPackageUrl,
+  resetSassWorkerPoolCaches,
+  shutdownSassWorkerPool,
+} from './sass-language';
 
 describe('sass-language', () => {
   describe('isPackageUrl', () => {
@@ -46,4 +56,142 @@ describe('sass-language', () => {
       expect(isPackageUrl('')).toBeFalse();
     });
   });
+
+  describe('package resolution caching', () => {
+    let temporaryRoot: string;
+    let projectRoot: string;
+    let resolveRequests: string[];
+
+    /**
+     * Creates a build stub that resolves a package specifier by searching the `node_modules`
+     * directories visible from the resolve directory, which is how esbuild resolves the
+     * package specifiers of a stylesheet.
+     */
+    function createBuildStub(): PluginBuild {
+      return {
+        initialOptions: { absWorkingDir: projectRoot },
+        resolve: async (path: string, options: { resolveDir: string }) => {
+          resolveRequests.push(`${options.resolveDir}:${path}`);
+
+          for (let directory = options.resolveDir; ; directory = dirname(directory)) {
+            // A package specifier resolves to the index file of the package, and an explicit file
+            // within it to that file. A deeper subpath is left unresolved, as esbuild leaves one
+            // that the `exports` of the package does not name; the Sass importer then resolves it
+            // against the package root instead.
+            for (const candidate of [
+              join(directory, 'node_modules', path, '_index.scss'),
+              join(directory, 'node_modules', path),
+            ]) {
+              if (statSync(candidate, { throwIfNoEntry: false })?.isFile()) {
+                return { path: candidate, errors: [], warnings: [] };
+              }
+            }
+
+            if (dirname(directory) === directory) {
+              return { path: undefined, errors: [], warnings: [] };
+            }
+          }
+        },
+      } as unknown as PluginBuild;
+    }
+
+    async function compile(stylesheet: string, source = "@use 'theme';"): Promise<string> {
+      const result = await SassStylesheetLanguage.process?.(
+        source,
+        stylesheet,
+        'scss',
+        { sourcemap: false },
+        createBuildStub(),
+      );
+      if (!result) {
+        throw new Error('The Sass stylesheet language has no process function.');
+      }
+
+      if (result.errors?.length) {
+        return `error: ${result.errors[0].text}`;
+      }
+
+      return (result.contents as string).trim();
+    }
+
+    beforeAll(async () => {
+      temporaryRoot = await mkdtemp(join(tmpdir(), 'angular-cli-sass-language-'));
+      projectRoot = join(temporaryRoot, 'project');
+
+      // A project with a `theme` package, plus a component directory with its own copy of it.
+      for (const [directory, marker] of [
+        [join(projectRoot, 'node_modules', 'theme'), 'project'],
+        [join(projectRoot, 'nested', 'node_modules', 'theme'), 'nested'],
+      ]) {
+        await mkdir(join(directory, 'sub'), { recursive: true });
+        await writeFile(join(directory, 'package.json'), '{ "name": "theme" }');
+        await writeFile(join(directory, '_index.scss'), `.marker { content: "${marker}"; }`);
+        await writeFile(
+          join(directory, 'sub', '_other.scss'),
+          `.deep { content: "${marker} deep"; }`,
+        );
+      }
+      for (const directory of ['component', 'sibling', 'nested']) {
+        await mkdir(join(projectRoot, directory), { recursive: true });
+      }
+      // A directory outside the project with no `theme` package visible to it.
+      await mkdir(join(temporaryRoot, 'isolated'), { recursive: true });
+    });
+
+    afterAll(async () => {
+      shutdownSassWorkerPool();
+      await rm(temporaryRoot, { force: true, recursive: true });
+    });
+
+    beforeEach(() => {
+      resetSassWorkerPoolCaches();
+      resolveRequests = [];
+    });
+
+    it('should not use a nested package resolution for a stylesheet that cannot see it', async () => {
+      const nested = await compile(join(projectRoot, 'nested', 'styles.scss'));
+      const component = await compile(join(projectRoot, 'component', 'styles.scss'));
+
+      expect(nested).toContain('content: "nested";');
+      expect(component).toContain('content: "project";');
+    });
+
+    it('should not use a package resolution from a stylesheet with additional nested packages', async () => {
+      // The reverse order of the above. The resolution of the first stylesheet must not be
+      // reused for the second, which has an additional `node_modules` directory to search.
+      const component = await compile(join(projectRoot, 'component', 'styles.scss'));
+      const nested = await compile(join(projectRoot, 'nested', 'styles.scss'));
+
+      expect(component).toContain('content: "project";');
+      expect(nested).toContain('content: "nested";');
+    });
+
+    it('should reuse a package resolution for stylesheets that search the same directories', async () => {
+      const component = await compile(join(projectRoot, 'component', 'styles.scss'));
+      const sibling = await compile(join(projectRoot, 'sibling', 'styles.scss'));
+
+      expect(component).toContain('content: "project";');
+      expect(sibling).toContain('content: "project";');
+      expect(resolveRequests.length).toBe(1);
+    });
+
+    it('should not use a nested package root for a deep import that cannot see it', async () => {
+      // A subpath that resolves to no file of its own is located through the root of the package,
+      // which is cached separately from the resolution of the specifier.
+      const deep = "@use 'theme/sub/other';";
+      const nested = await compile(join(projectRoot, 'nested', 'styles.scss'), deep);
+      const component = await compile(join(projectRoot, 'component', 'styles.scss'), deep);
+
+      expect(nested).toContain('content: "nested deep";');
+      expect(component).toContain('content: "project deep";');
+    });
+
+    it('should not reuse a failed package resolution for a stylesheet that can resolve it', async () => {
+      const isolated = await compile(join(temporaryRoot, 'isolated', 'styles.scss'));
+      const component = await compile(join(projectRoot, 'component', 'styles.scss'));
+
+      expect(isolated).toContain("Can't find stylesheet to import.");
+      expect(component).toContain('content: "project";');
+    });
+  });
 });