Skip to content

Commit

Permalink
fix(@angular-devkit/build-angular): ensure css url() prefix warnings …
Browse files Browse the repository at this point in the history
…support Sass rebasing

The stylesheet url() resource plugin will now correctly issue warnings for the usage of Webpack
specific prefixes such as the tilde when used in an imported Sass file. Previously, these URLs
would be rebased by the Sass processing step which would cause the tilde prefix to no longer
be a prefix. This would then no longer be considered a warning due to the tilde no longer being
the first character of the URL value. Additionally, a warning is also now issued for the previously
unsupported but available caret prefix. Removing the caret prefix and adding the path to the
`externalDependencies` build option should provide equivalent behavior.

(cherry picked from commit df1aba1)
  • Loading branch information
clydin committed Oct 23, 2023
1 parent 5d09de6 commit 1a6aa43
Show file tree
Hide file tree
Showing 4 changed files with 160 additions and 12 deletions.
2 changes: 1 addition & 1 deletion packages/angular_devkit/build_angular/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ ts_library(

LARGE_SPECS = {
"application": {
"shards": 16,
"shards": 12,
"size": "large",
"flaky": True,
"extra_deps": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { APPLICATION_BUILDER_INFO, BASE_OPTIONS, describeBuilder } from '../setu

describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => {
describe('Behavior: "Stylesheet url() Resolution"', () => {
it('should show a note when using tilde prefix', async () => {
it('should show a note when using tilde prefix in a directly referenced stylesheet', async () => {
await harness.writeFile(
'src/styles.css',
`
Expand All @@ -26,14 +26,142 @@ describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => {
styles: ['src/styles.css'],
});

const { result, logs } = await harness.executeOnce();
const { result, logs } = await harness.executeOnce({ outputLogsOnFailure: false });
expect(result?.success).toBe(false);

expect(logs).toContain(
jasmine.objectContaining({
message: jasmine.stringMatching('You can remove the tilde and'),
}),
);
expect(logs).not.toContain(
jasmine.objectContaining({
message: jasmine.stringMatching('Preprocessor stylesheets may not show the exact'),
}),
);
});

it('should show a note when using tilde prefix in an imported CSS stylesheet', async () => {
await harness.writeFile(
'src/styles.css',
`
@import "a.css";
`,
);
await harness.writeFile(
'src/a.css',
`
.a {
background-image: url("~/image.jpg")
}
`,
);

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

const { result, logs } = await harness.executeOnce({ outputLogsOnFailure: false });
expect(result?.success).toBe(false);

expect(logs).toContain(
jasmine.objectContaining({
message: jasmine.stringMatching('You can remove the tilde and'),
}),
);
});

it('should show a note when using tilde prefix in an imported Sass stylesheet', async () => {
await harness.writeFile(
'src/styles.scss',
`
@import "a";
`,
);
await harness.writeFile(
'src/a.scss',
`
.a {
background-image: url("~/image.jpg")
}
`,
);

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

const { result, logs } = await harness.executeOnce({ outputLogsOnFailure: false });
expect(result?.success).toBe(false);

expect(logs).toContain(
jasmine.objectContaining({
message: jasmine.stringMatching('You can remove the tilde and'),
}),
);
expect(logs).toContain(
jasmine.objectContaining({
message: jasmine.stringMatching('Preprocessor stylesheets may not show the exact'),
}),
);
});

it('should show a note when using caret prefix in a directly referenced stylesheet', async () => {
await harness.writeFile(
'src/styles.css',
`
.a {
background-image: url("^image.jpg")
}
`,
);

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

const { result, logs } = await harness.executeOnce({ outputLogsOnFailure: false });
expect(result?.success).toBe(false);

expect(logs).toContain(
jasmine.objectContaining({
message: jasmine.stringMatching('You can remove the caret and'),
}),
);
});

it('should show a note when using caret prefix in an imported Sass stylesheet', async () => {
await harness.writeFile(
'src/styles.scss',
`
@import "a";
`,
);
await harness.writeFile(
'src/a.scss',
`
.a {
background-image: url("^image.jpg")
}
`,
);

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

const { result, logs } = await harness.executeOnce({ outputLogsOnFailure: false });
expect(result?.success).toBe(false);

expect(logs).toContain(
jasmine.objectContaining({
message: jasmine.stringMatching('You can remove the caret and'),
}),
);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import type { Plugin, PluginBuild } from 'esbuild';
import { readFile } from 'node:fs/promises';
import { join, relative } from 'node:path';
import { extname, join, relative } from 'node:path';
import { LoadResultCache, createCachedLoad } from '../load-result-cache';

/**
Expand Down Expand Up @@ -56,13 +56,33 @@ export function createCssResourcePlugin(cache?: LoadResultCache): Plugin {
resolveDir,
});

if (result.errors.length && args.path[0] === '~') {
result.errors[0].notes = [
{
if (result.errors.length) {
const error = result.errors[0];
if (args.path[0] === '~') {
error.notes = [
{
location: null,
text: 'You can remove the tilde and use a relative path to reference it, which should remove this error.',
},
];
} else if (args.path[0] === '^') {
error.notes = [
{
location: null,
text:
'You can remove the caret and add the path to the `externalDependencies` build option,' +
' which should remove this error.',
},
];
}

const extension = importer && extname(importer);
if (extension !== '.css') {
error.notes.push({
location: null,
text: 'You can remove the tilde and use a relative path to reference it, which should remove this error.',
},
];
text: 'Preprocessor stylesheets may not show the exact file location of the error.',
});
}
}

// Return results that are not files since these are most likely specific to another plugin
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ 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 or a Sass variable
if (value.length === 0 || value.startsWith('$')) {
// Skip if value is empty, a Sass variable, or Webpack-specific prefix
if (value.length === 0 || value[0] === '$' || value[0] === '~' || value[0] === '^') {
continue;
}

Expand Down

0 comments on commit 1a6aa43

Please sign in to comment.