Skip to content

Commit

Permalink
fix(@angular/build): avoid escaping rebased Sass URL values
Browse files Browse the repository at this point in the history
Remove escaping of the raw existing values present within a URL
when rebasing Sass files. These values are present in input code
and should retain their respective errors and behavior as written.
The escaping of the prefixed path is retained to prevent parsing errors
for the internally injected values.

(cherry picked from commit 721d50b)
  • Loading branch information
clydin authored and alan-agius4 committed May 29, 2024
1 parent 346df49 commit 43a2a7d
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,36 @@ describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => {
harness.expectFile('dist/browser/media/logo.svg').toExist();
});

it('should rebase a URL with interpolation using concatenation referencing a local resource', async () => {
await harness.writeFiles({
'src/styles.scss': `@use 'theme/a';`,
'src/theme/a.scss': `
@import './b';
$extra-var: "2";
$postfix-var: "xyz";
.a {
background-image: url("#{$my-var}logo#{$extra-var+ "-" + $postfix-var}.svg")
}
`,
'src/theme/b.scss': `$my-var: "./images/";`,
'src/theme/images/logo2-xyz.svg': `<svg></svg>`,
});

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

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

harness
.expectFile('dist/browser/styles.css')
.content.toContain(`url("./media/logo2-xyz.svg")`);
harness.expectFile('dist/browser/media/logo2-xyz.svg').toExist();
});

it('should rebase a URL with an non-leading interpolation referencing a local resource', async () => {
await harness.writeFiles({
'src/styles.scss': `@use 'theme/a';`,
Expand Down
31 changes: 3 additions & 28 deletions packages/angular/build/src/tools/sass/rebasing-importer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,28 +24,6 @@ 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 @@ -100,18 +78,15 @@ abstract class UrlRebasingImporter implements Importer<'sync'> {

// Sass variable usage either starts with a `$` or contains a namespace and a `.$`
const valueNormalized = value[0] === '$' || /^\w+\.\$/.test(value) ? `#{${value}}` : value;
const rebasedPath =
relative(this.entryDirectory, stylesheetDirectory) + '||file:' + valueNormalized;
const rebasedPath = relative(this.entryDirectory, stylesheetDirectory);

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

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

if (updatedContents) {
Expand Down

0 comments on commit 43a2a7d

Please sign in to comment.