Skip to content

Commit f9e982c

Browse files
committed
fix(@angular-devkit/build-angular): check namespaced Sass variables when rebasing URLs
The `@use` Sass directive allows Sass variables to be accessed via a namespace. This was previously not checked when performing URL path rebasing on imported Sass stylesheets. These type of Sass variable references are now also ignored when attempting to rebase URL paths during loading. The final rebased URL now also does not add a leading relative prefix indicator (`./`) unless not already present. (cherry picked from commit 8e9b675)
1 parent e1eeb36 commit f9e982c

File tree

2 files changed

+77
-4
lines changed

2 files changed

+77
-4
lines changed

packages/angular_devkit/build_angular/src/builders/application/tests/behavior/stylesheet-url-resolution_spec.ts

+42
Original file line numberDiff line numberDiff line change
@@ -163,5 +163,47 @@ describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => {
163163
}),
164164
);
165165
});
166+
167+
it('should not rebase a URL with a namespaced Sass variable reference', async () => {
168+
await harness.writeFile(
169+
'src/styles.scss',
170+
`
171+
@import "a";
172+
`,
173+
);
174+
await harness.writeFile(
175+
'src/a.scss',
176+
`
177+
@use './b' as named;
178+
.a {
179+
background-image: url(named.$my-var)
180+
}
181+
`,
182+
);
183+
await harness.writeFile(
184+
'src/b.scss',
185+
`
186+
@forward './c.scss' show $my-var;
187+
`,
188+
);
189+
await harness.writeFile(
190+
'src/c.scss',
191+
`
192+
$my-var: "https://example.com/example.png";
193+
`,
194+
);
195+
196+
harness.useTarget('build', {
197+
...BASE_OPTIONS,
198+
styles: ['src/styles.scss'],
199+
});
200+
201+
const { result } = await harness.executeOnce();
202+
expect(result?.success).toBe(true);
203+
204+
harness
205+
.expectFile('dist/browser/styles.css')
206+
.content.toContain('url(https://example.com/example.png)');
207+
});
166208
});
167209
});

packages/angular_devkit/build_angular/src/tools/sass/rebasing-importer.ts

+35-4
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,28 @@ export interface DirectoryEntry {
2323
directories: Set<string>;
2424
}
2525

26+
/**
27+
* Ensures that a bare specifier URL path that is intended to be treated as
28+
* a relative path has a leading `./` or `../` prefix.
29+
*
30+
* @param url A bare specifier URL path that should be considered relative.
31+
* @returns
32+
*/
33+
function ensureRelative(url: string): string {
34+
// Empty
35+
if (!url) {
36+
return url;
37+
}
38+
39+
// Already relative
40+
if (url[0] === '.' && (url[1] === '/' || (url[1] === '.' && url[2] === '/'))) {
41+
return url;
42+
}
43+
44+
// Needs prefix
45+
return './' + url;
46+
}
47+
2648
/**
2749
* A Sass Importer base class that provides the load logic to rebase all `url()` functions
2850
* within a stylesheet. The rebasing will ensure that the URLs in the output of the Sass compiler
@@ -55,8 +77,14 @@ abstract class UrlRebasingImporter implements Importer<'sync'> {
5577
// Rebase any URLs that are found
5678
let updatedContents;
5779
for (const { start, end, value } of findUrls(contents)) {
58-
// Skip if value is empty, a Sass variable, or Webpack-specific prefix
59-
if (value.length === 0 || value[0] === '$' || value[0] === '~' || value[0] === '^') {
80+
// Skip if value is empty or Webpack-specific prefix
81+
if (value.length === 0 || value[0] === '~' || value[0] === '^') {
82+
continue;
83+
}
84+
85+
// Skip if value is a Sass variable.
86+
// Sass variable usage either starts with a `$` or contains a namespace and a `.$`
87+
if (value[0] === '$' || /^\w+\.\$/.test(value)) {
6088
continue;
6189
}
6290

@@ -69,10 +97,13 @@ abstract class UrlRebasingImporter implements Importer<'sync'> {
6997

7098
// Normalize path separators and escape characters
7199
// https://developer.mozilla.org/en-US/docs/Web/CSS/url#syntax
72-
const rebasedUrl = './' + rebasedPath.replace(/\\/g, '/').replace(/[()\s'"]/g, '\\$&');
100+
const rebasedUrl = ensureRelative(
101+
rebasedPath.replace(/\\/g, '/').replace(/[()\s'"]/g, '\\$&'),
102+
);
73103

74104
updatedContents ??= new MagicString(contents);
75-
updatedContents.update(start, end, rebasedUrl);
105+
// Always quote the URL to avoid potential downstream parsing problems
106+
updatedContents.update(start, end, `"${rebasedUrl}"`);
76107
}
77108

78109
if (updatedContents) {

0 commit comments

Comments
 (0)