Skip to content

Commit 2586ae8

Browse files
committed
perf(@angular/build): key Sass package resolutions without containing URL qualification
Previously, only pkg: URLs were treated as package specifiers when constructing the Sass resolutionCache key. Bare package specifiers such as @angular/material were qualified with the importing file's containing URL, resulting in cache misses across different component stylesheets. Additionally, packageRootCache was also unnecessarily qualified with the containing URL. Package specifiers are now detected and cached without containing URL qualification, allowing package resolutions and package roots to be shared across all stylesheets.
1 parent 0ad1409 commit 2586ae8

2 files changed

Lines changed: 70 additions & 5 deletions

File tree

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

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,20 @@ export const SassStylesheetLanguage = Object.freeze<StylesheetLanguage>({
6969
},
7070
});
7171

72+
export function isPackageUrl(url: string): boolean {
73+
if (url.startsWith('pkg:')) {
74+
return true;
75+
}
76+
77+
return (
78+
url.length > 0 &&
79+
!url.startsWith('.') &&
80+
!url.startsWith('/') &&
81+
!url.startsWith('\\') &&
82+
!url.includes(':')
83+
);
84+
}
85+
7286
function parsePackageName(url: string): { packageName: string; readonly pathSegments: string[] } {
7387
const parts = (url.startsWith('pkg:') ? url.slice(4) : url).split('/');
7488
const hasScope = parts.length >= 2 && parts[0][0] === '@';
@@ -131,9 +145,8 @@ async function compileString(
131145
importers: [
132146
{
133147
findFileUrl: (url, options) => {
134-
const cacheKey = url.startsWith('pkg:')
135-
? url
136-
: `${options.containingUrl?.href ?? ''}:${url}`;
148+
const isPackage = isPackageUrl(url);
149+
const cacheKey = isPackage ? url : `${options.containingUrl?.href ?? ''}:${url}`;
137150

138151
return currentResolutionCache.getOrCreate(cacheKey, async () => {
139152
const result = await resolveUrl(url, options);
@@ -142,13 +155,16 @@ async function compileString(
142155
}
143156

144157
// Check for package deep imports
158+
if (!isPackage) {
159+
return null;
160+
}
161+
145162
const { packageName, pathSegments } = parsePackageName(url);
146163

147164
// Caching package root locations is particularly beneficial for `@material/*` packages
148165
// which extensively use deep imports.
149-
const packageRootKey = `${options.containingUrl?.href ?? ''}:${packageName}`;
150166
const packageRoot = await currentPackageRootCache.getOrCreate(
151-
packageRootKey,
167+
packageName,
152168
async () => {
153169
// Use the required presence of a package root `package.json` file to resolve the location
154170
const packageResult = await resolveUrl(packageName + '/package.json', options);
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.dev/license
7+
*/
8+
9+
import { isPackageUrl } from './sass-language';
10+
11+
describe('sass-language', () => {
12+
describe('isPackageUrl', () => {
13+
it('should identify pkg: scheme URLs as package URLs', () => {
14+
expect(isPackageUrl('pkg:@angular/material')).toBeTrue();
15+
expect(isPackageUrl('pkg:bootstrap')).toBeTrue();
16+
expect(isPackageUrl('pkg:@material/button/button')).toBeTrue();
17+
});
18+
19+
it('should identify bare specifiers as package URLs', () => {
20+
expect(isPackageUrl('@angular/material')).toBeTrue();
21+
expect(isPackageUrl('@angular/material/button')).toBeTrue();
22+
expect(isPackageUrl('@material/button/button.scss')).toBeTrue();
23+
expect(isPackageUrl('bootstrap')).toBeTrue();
24+
expect(isPackageUrl('bootstrap/scss/bootstrap')).toBeTrue();
25+
});
26+
27+
it('should not identify relative paths as package URLs', () => {
28+
expect(isPackageUrl('./styles.scss')).toBeFalse();
29+
expect(isPackageUrl('../shared/variables')).toBeFalse();
30+
expect(isPackageUrl('.hidden')).toBeFalse();
31+
expect(isPackageUrl('.\\styles.scss')).toBeFalse();
32+
expect(isPackageUrl('..\\shared\\variables')).toBeFalse();
33+
});
34+
35+
it('should not identify absolute paths or non-pkg URLs as package URLs', () => {
36+
expect(isPackageUrl('/styles/theme.scss')).toBeFalse();
37+
expect(isPackageUrl('\\styles\\theme.scss')).toBeFalse();
38+
expect(isPackageUrl('file:///path/to/theme.scss')).toBeFalse();
39+
expect(isPackageUrl('http://example.com/styles.css')).toBeFalse();
40+
expect(isPackageUrl('https://example.com/styles.css')).toBeFalse();
41+
expect(isPackageUrl('C:\\path\\to\\theme.scss')).toBeFalse();
42+
expect(isPackageUrl('C:/path/to/theme.scss')).toBeFalse();
43+
});
44+
45+
it('should not identify empty string as a package URL', () => {
46+
expect(isPackageUrl('')).toBeFalse();
47+
});
48+
});
49+
});

0 commit comments

Comments
 (0)