Skip to content

Commit

Permalink
fix(@angular-devkit/build-angular): check namespaced Sass variables w…
Browse files Browse the repository at this point in the history
…hen 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)
  • Loading branch information
clydin committed Dec 5, 2023
1 parent e1eeb36 commit f9e982c
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -163,5 +163,47 @@ describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => {
}),
);
});

it('should not rebase a URL with a namespaced Sass variable reference', async () => {
await harness.writeFile(
'src/styles.scss',
`
@import "a";
`,
);
await harness.writeFile(
'src/a.scss',
`
@use './b' as named;
.a {
background-image: url(named.$my-var)
}
`,
);
await harness.writeFile(
'src/b.scss',
`
@forward './c.scss' show $my-var;
`,
);
await harness.writeFile(
'src/c.scss',
`
$my-var: "https://example.com/example.png";
`,
);

harness.useTarget('build', {
...BASE_OPTIONS,
styles: ['src/styles.scss'],
});

const { result } = await harness.executeOnce();
expect(result?.success).toBe(true);

harness
.expectFile('dist/browser/styles.css')
.content.toContain('url(https://example.com/example.png)');
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,28 @@ export interface DirectoryEntry {
directories: Set<string>;
}

/**
* Ensures that a bare specifier URL path that is intended to be treated as
* a relative path has a leading `./` or `../` prefix.
*
* @param url A bare specifier URL path that should be considered relative.
* @returns
*/
function ensureRelative(url: string): string {
// Empty
if (!url) {
return url;
}

// Already relative
if (url[0] === '.' && (url[1] === '/' || (url[1] === '.' && url[2] === '/'))) {
return url;
}

// Needs prefix
return './' + url;
}

/**
* A Sass Importer base class that provides the load logic to rebase all `url()` functions
* within a stylesheet. The rebasing will ensure that the URLs in the output of the Sass compiler
Expand Down Expand Up @@ -55,8 +77,14 @@ abstract class UrlRebasingImporter implements Importer<'sync'> {
// Rebase any URLs that are found
let updatedContents;
for (const { start, end, value } of findUrls(contents)) {
// Skip if value is empty, a Sass variable, or Webpack-specific prefix
if (value.length === 0 || value[0] === '$' || value[0] === '~' || value[0] === '^') {
// Skip if value is empty or Webpack-specific prefix
if (value.length === 0 || value[0] === '~' || value[0] === '^') {
continue;
}

// Skip if value is a Sass variable.
// Sass variable usage either starts with a `$` or contains a namespace and a `.$`
if (value[0] === '$' || /^\w+\.\$/.test(value)) {
continue;
}

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

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

updatedContents ??= new MagicString(contents);
updatedContents.update(start, end, rebasedUrl);
// Always quote the URL to avoid potential downstream parsing problems
updatedContents.update(start, end, `"${rebasedUrl}"`);
}

if (updatedContents) {
Expand Down

0 comments on commit f9e982c

Please sign in to comment.