fix(@angular/build): scope Sass package resolution caching for stylesheets in 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 dependency within
`node_modules` that has its own nested version of a package received the version resolved for
the application, the application received the nested version when the dependency was compiled
first, and a failed resolution was reused for a dependency 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 qualified with a scope. A stylesheet within
`node_modules` uses its own directory as the scope, which keeps nested dependency versions
isolated. All other stylesheets use the working directory of the build, so the component
stylesheets of an application continue to share a single resolution. The scope is derived from
the path of the stylesheet alone and requires no file system access. A containing URL that does
not use the `file:` scheme is resolved from the working directory instead of causing an error.

Author: thekhegay

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

@@ -9,7 +9,7 @@
 import type { OnLoadResult, PartialMessage, PartialNote, ResolveResult } from 'esbuild';
 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';
@@ -50,12 +50,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 +60,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,
+    );
   },
 });
 
@@ -102,7 +104,8 @@ async function compileString(
   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 +122,8 @@ 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.
+  // regardless of its importer's path, except for importers within `node_modules`, which are
+  // scoped to their own directory. 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 +149,24 @@ async function compileString(
       importers: [
         {
           findFileUrl: (url, options) => {
+            const containingPath =
+              options.containingUrl?.protocol === 'file:'
+                ? fileURLToPath(options.containingUrl)
+                : undefined;
+            const resolveDir = containingPath ? dirname(containingPath) : workingDirectory;
             const isPackage = isPackageUrl(url);
-            const cacheKey = isPackage ? url : `${options.containingUrl?.href ?? ''}:${url}`;
+
+            // Package urls from files within `node_modules` are scoped to the directory of the
+            // importer to isolate nested dependency versions. All other files share the working
+            // directory, allowing component stylesheets to share package resolutions.
+            const isNodeModules = /[\\/]node_modules[\\/]/.test(containingPath ?? '');
+            const scope = isNodeModules ? (resolveDir ?? '') : (workingDirectory ?? '');
+            const cacheKey = isPackage
+              ? `${scope}:${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 +181,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,
+                `${scope}:${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,20 @@
  * 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 { pathToFileURL } from 'node:url';
+import type { FileImporter } from 'sass-embedded';
+import { SassCompiler } from '../../sass/sass-service';
+import {
+  SassStylesheetLanguage,
+  isPackageUrl,
+  resetSassWorkerPoolCaches,
+  shutdownSassWorkerPool,
+} from './sass-language';
 
 describe('sass-language', () => {
   describe('isPackageUrl', () => {
@@ -46,4 +59,168 @@ describe('sass-language', () => {
       expect(isPackageUrl('')).toBeFalse();
     });
   });
+
+  describe('package resolution caching', () => {
+    let temporaryRoot: string;
+    let projectRoot: string;
+    let buttonStylesheet: string;
+    let cardStylesheet: string;
+    let dependencyStylesheet: 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();
+    }
+
+    async function writePackage(directory: string, marker: string): Promise<void> {
+      await mkdir(join(directory, 'sub'), { recursive: true });
+      await writeFile(join(directory, 'package.json'), '{}');
+      await writeFile(join(directory, '_index.scss'), `.marker { content: "${marker}"; }`);
+      await writeFile(
+        join(directory, 'sub', '_other.scss'),
+        `.deep { content: "${marker} deep"; }`,
+      );
+    }
+
+    beforeAll(async () => {
+      temporaryRoot = await mkdtemp(join(tmpdir(), 'angular-cli-sass-language-'));
+      projectRoot = join(temporaryRoot, 'project');
+      const dependencyRoot = join(projectRoot, 'node_modules', 'dependency');
+
+      // An application using a `theme` package, and a dependency with its own nested version of
+      // `theme` plus an `extra` package that only the dependency can see.
+      await writePackage(join(projectRoot, 'node_modules', 'theme'), 'project');
+      await writePackage(join(dependencyRoot, 'node_modules', 'theme'), 'dependency');
+      await writePackage(join(dependencyRoot, 'node_modules', 'extra'), 'extra');
+
+      buttonStylesheet = join(projectRoot, 'src', 'app', 'button', 'button.scss');
+      cardStylesheet = join(projectRoot, 'src', 'app', 'card', 'card.scss');
+      dependencyStylesheet = join(dependencyRoot, 'styles.scss');
+      for (const stylesheet of [buttonStylesheet, cardStylesheet]) {
+        await mkdir(dirname(stylesheet), { recursive: true });
+      }
+    });
+
+    afterAll(async () => {
+      shutdownSassWorkerPool();
+      await rm(temporaryRoot, { force: true, recursive: true });
+    });
+
+    beforeEach(() => {
+      resetSassWorkerPoolCaches();
+      resolveRequests = [];
+    });
+
+    it('should not use the package resolution of a dependency for the application', async () => {
+      const dependency = await compile(dependencyStylesheet);
+      const application = await compile(buttonStylesheet);
+
+      expect(dependency).toContain('content: "dependency";');
+      expect(application).toContain('content: "project";');
+    });
+
+    it('should not use the package resolution of the application for a dependency', async () => {
+      const application = await compile(buttonStylesheet);
+      const dependency = await compile(dependencyStylesheet);
+
+      expect(application).toContain('content: "project";');
+      expect(dependency).toContain('content: "dependency";');
+    });
+
+    it('should not reuse a failed package resolution of the application for a dependency', async () => {
+      const source = "@use 'extra';";
+      const application = await compile(buttonStylesheet, source);
+      const dependency = await compile(dependencyStylesheet, source);
+
+      expect(application).toContain("Can't find stylesheet to import.");
+      expect(dependency).toContain('content: "extra";');
+    });
+
+    it('should not use the package root of a dependency for a deep import of the application', 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 source = "@use 'theme/sub/other';";
+      const dependency = await compile(dependencyStylesheet, source);
+      const application = await compile(buttonStylesheet, source);
+
+      expect(dependency).toContain('content: "dependency deep";');
+      expect(application).toContain('content: "project deep";');
+    });
+
+    it('should share a package resolution between the stylesheets of different components', async () => {
+      const button = await compile(buttonStylesheet);
+      const card = await compile(cardStylesheet);
+
+      expect(button).toContain('content: "project";');
+      expect(card).toContain('content: "project";');
+      expect(resolveRequests.length).toBe(1);
+    });
+
+    it('should resolve a package url of a non-file containing URL from the working directory', async () => {
+      // The stylesheets of a build have file URLs, but Sass does not limit a containing URL to them.
+      spyOn(SassCompiler.prototype, 'compileStringAsync').and.callFake(async (_, options) => {
+        const importer = options.importers?.[0] as FileImporter<'async'>;
+        const url = await importer.findFileUrl('theme', {
+          containingUrl: new URL('custom:styles.scss'),
+          fromImport: false,
+        });
+
+        return { css: url?.href ?? '', loadedUrls: [] };
+      });
+
+      const result = await compile(buttonStylesheet);
+
+      expect(result).toBe(
+        pathToFileURL(join(projectRoot, 'node_modules', 'theme', '_index.scss')).href,
+      );
+    });
+  });
 });