Skip to content

Commit 1a6aa43

Browse files
clydinclydin
committed
fix(@angular-devkit/build-angular): ensure css url() prefix warnings 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)
1 parent 5d09de6 commit 1a6aa43

File tree

4 files changed

+160
-12
lines changed

4 files changed

+160
-12
lines changed

‎packages/angular_devkit/build_angular/BUILD.bazel‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,7 @@ ts_library(
310310

311311
LARGE_SPECS = {
312312
"application": {
313-
"shards": 16,
313+
"shards": 12,
314314
"size": "large",
315315
"flaky": True,
316316
"extra_deps": [

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

Lines changed: 130 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { APPLICATION_BUILDER_INFO, BASE_OPTIONS, describeBuilder } from '../setu
1111

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

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

3232
expect(logs).toContain(
3333
jasmine.objectContaining({
3434
message: jasmine.stringMatching('You can remove the tilde and'),
3535
}),
3636
);
37+
expect(logs).not.toContain(
38+
jasmine.objectContaining({
39+
message: jasmine.stringMatching('Preprocessor stylesheets may not show the exact'),
40+
}),
41+
);
42+
});
43+
44+
it('should show a note when using tilde prefix in an imported CSS stylesheet', async () => {
45+
await harness.writeFile(
46+
'src/styles.css',
47+
`
48+
@import "a.css";
49+
`,
50+
);
51+
await harness.writeFile(
52+
'src/a.css',
53+
`
54+
.a {
55+
background-image: url("~/image.jpg")
56+
}
57+
`,
58+
);
59+
60+
harness.useTarget('build', {
61+
...BASE_OPTIONS,
62+
styles: ['src/styles.css'],
63+
});
64+
65+
const { result, logs } = await harness.executeOnce({ outputLogsOnFailure: false });
66+
expect(result?.success).toBe(false);
67+
68+
expect(logs).toContain(
69+
jasmine.objectContaining({
70+
message: jasmine.stringMatching('You can remove the tilde and'),
71+
}),
72+
);
73+
});
74+
75+
it('should show a note when using tilde prefix in an imported Sass stylesheet', async () => {
76+
await harness.writeFile(
77+
'src/styles.scss',
78+
`
79+
@import "a";
80+
`,
81+
);
82+
await harness.writeFile(
83+
'src/a.scss',
84+
`
85+
.a {
86+
background-image: url("~/image.jpg")
87+
}
88+
`,
89+
);
90+
91+
harness.useTarget('build', {
92+
...BASE_OPTIONS,
93+
styles: ['src/styles.scss'],
94+
});
95+
96+
const { result, logs } = await harness.executeOnce({ outputLogsOnFailure: false });
97+
expect(result?.success).toBe(false);
98+
99+
expect(logs).toContain(
100+
jasmine.objectContaining({
101+
message: jasmine.stringMatching('You can remove the tilde and'),
102+
}),
103+
);
104+
expect(logs).toContain(
105+
jasmine.objectContaining({
106+
message: jasmine.stringMatching('Preprocessor stylesheets may not show the exact'),
107+
}),
108+
);
109+
});
110+
111+
it('should show a note when using caret prefix in a directly referenced stylesheet', async () => {
112+
await harness.writeFile(
113+
'src/styles.css',
114+
`
115+
.a {
116+
background-image: url("^image.jpg")
117+
}
118+
`,
119+
);
120+
121+
harness.useTarget('build', {
122+
...BASE_OPTIONS,
123+
styles: ['src/styles.css'],
124+
});
125+
126+
const { result, logs } = await harness.executeOnce({ outputLogsOnFailure: false });
127+
expect(result?.success).toBe(false);
128+
129+
expect(logs).toContain(
130+
jasmine.objectContaining({
131+
message: jasmine.stringMatching('You can remove the caret and'),
132+
}),
133+
);
134+
});
135+
136+
it('should show a note when using caret prefix in an imported Sass stylesheet', async () => {
137+
await harness.writeFile(
138+
'src/styles.scss',
139+
`
140+
@import "a";
141+
`,
142+
);
143+
await harness.writeFile(
144+
'src/a.scss',
145+
`
146+
.a {
147+
background-image: url("^image.jpg")
148+
}
149+
`,
150+
);
151+
152+
harness.useTarget('build', {
153+
...BASE_OPTIONS,
154+
styles: ['src/styles.scss'],
155+
});
156+
157+
const { result, logs } = await harness.executeOnce({ outputLogsOnFailure: false });
158+
expect(result?.success).toBe(false);
159+
160+
expect(logs).toContain(
161+
jasmine.objectContaining({
162+
message: jasmine.stringMatching('You can remove the caret and'),
163+
}),
164+
);
37165
});
38166
});
39167
});

‎packages/angular_devkit/build_angular/src/tools/esbuild/stylesheets/css-resource-plugin.ts‎

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

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

1414
/**
@@ -56,13 +56,33 @@ export function createCssResourcePlugin(cache?: LoadResultCache): Plugin {
5656
resolveDir,
5757
});
5858

59-
if (result.errors.length && args.path[0] === '~') {
60-
result.errors[0].notes = [
61-
{
59+
if (result.errors.length) {
60+
const error = result.errors[0];
61+
if (args.path[0] === '~') {
62+
error.notes = [
63+
{
64+
location: null,
65+
text: 'You can remove the tilde and use a relative path to reference it, which should remove this error.',
66+
},
67+
];
68+
} else if (args.path[0] === '^') {
69+
error.notes = [
70+
{
71+
location: null,
72+
text:
73+
'You can remove the caret and add the path to the `externalDependencies` build option,' +
74+
' which should remove this error.',
75+
},
76+
];
77+
}
78+
79+
const extension = importer && extname(importer);
80+
if (extension !== '.css') {
81+
error.notes.push({
6282
location: null,
63-
text: 'You can remove the tilde and use a relative path to reference it, which should remove this error.',
64-
},
65-
];
83+
text: 'Preprocessor stylesheets may not show the exact file location of the error.',
84+
});
85+
}
6686
}
6787

6888
// Return results that are not files since these are most likely specific to another plugin

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,8 @@ abstract class UrlRebasingImporter implements Importer<'sync'> {
5555
// Rebase any URLs that are found
5656
let updatedContents;
5757
for (const { start, end, value } of findUrls(contents)) {
58-
// Skip if value is empty or a Sass variable
59-
if (value.length === 0 || value.startsWith('$')) {
58+
// Skip if value is empty, a Sass variable, or Webpack-specific prefix
59+
if (value.length === 0 || value[0] === '$' || value[0] === '~' || value[0] === '^') {
6060
continue;
6161
}
6262

0 commit comments

Comments
 (0)