Skip to content

Commit 7dbe812

Browse files
authored
fix: ensure non-matching elements are scoped for :not(...) selector (#13999)
If the contents of a `:not` selector don't match, then it's actually a match for `:not` because it's inverted. Therefore, we need to scope such elements. We're also making sure that contents of `:not` that never match actually count as a used (because the result is negated), and as such the contents of `:not` that always match are actually marked as unused. Fixes #13974
1 parent 57e27ae commit 7dbe812

File tree

14 files changed

+172
-90
lines changed

14 files changed

+172
-90
lines changed

.changeset/dirty-parrots-smell.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'svelte': patch
3+
---
4+
5+
fix: ensure non-matching elements are scoped for `:not(...)` selector

packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js

Lines changed: 58 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { get_attribute_chunks, is_text_attribute } from '../../../utils/ast.js';
1010
* stylesheet: Compiler.Css.StyleSheet;
1111
* element: Compiler.AST.RegularElement | Compiler.AST.SvelteElement;
1212
* from_render_tag: boolean;
13+
* inside_not: boolean;
1314
* }} State
1415
*/
1516
/** @typedef {NODE_PROBABLY_EXISTS | NODE_DEFINITELY_EXISTS} NodeExistsValue */
@@ -61,9 +62,13 @@ export function prune(stylesheet, element) {
6162
const parent = get_element_parent(element);
6263
if (!parent) return;
6364

64-
walk(stylesheet, { stylesheet, element: parent, from_render_tag: true }, visitors);
65+
walk(
66+
stylesheet,
67+
{ stylesheet, element: parent, from_render_tag: true, inside_not: false },
68+
visitors
69+
);
6570
} else {
66-
walk(stylesheet, { stylesheet, element, from_render_tag: false }, visitors);
71+
walk(stylesheet, { stylesheet, element, from_render_tag: false, inside_not: false }, visitors);
6772
}
6873
}
6974

@@ -127,7 +132,7 @@ const visitors = {
127132
selectors_to_check,
128133
/** @type {Compiler.Css.Rule} */ (node.metadata.rule),
129134
element,
130-
context.state.stylesheet
135+
context.state
131136
)
132137
) {
133138
mark(inner, element);
@@ -143,7 +148,7 @@ const visitors = {
143148
selectors,
144149
/** @type {Compiler.Css.Rule} */ (node.metadata.rule),
145150
context.state.element,
146-
context.state.stylesheet
151+
context.state
147152
)
148153
) {
149154
mark(inner, context.state.element);
@@ -188,10 +193,10 @@ function truncate(node) {
188193
* @param {Compiler.Css.RelativeSelector[]} relative_selectors
189194
* @param {Compiler.Css.Rule} rule
190195
* @param {Compiler.AST.RegularElement | Compiler.AST.SvelteElement} element
191-
* @param {Compiler.Css.StyleSheet} stylesheet
196+
* @param {State} state
192197
* @returns {boolean}
193198
*/
194-
function apply_selector(relative_selectors, rule, element, stylesheet) {
199+
function apply_selector(relative_selectors, rule, element, state) {
195200
const parent_selectors = relative_selectors.slice();
196201
const relative_selector = parent_selectors.pop();
197202

@@ -201,7 +206,7 @@ function apply_selector(relative_selectors, rule, element, stylesheet) {
201206
relative_selector,
202207
rule,
203208
element,
204-
stylesheet
209+
state
205210
);
206211

207212
if (!possible_match) {
@@ -215,14 +220,14 @@ function apply_selector(relative_selectors, rule, element, stylesheet) {
215220
parent_selectors,
216221
rule,
217222
element,
218-
stylesheet
223+
state
219224
);
220225
}
221226

222227
// if this is the left-most non-global selector, mark it — we want
223228
// `x y z {...}` to become `x.blah y z.blah {...}`
224229
const parent = parent_selectors[parent_selectors.length - 1];
225-
if (!parent || is_global(parent, rule)) {
230+
if (!state.inside_not && (!parent || is_global(parent, rule))) {
226231
mark(relative_selector, element);
227232
}
228233

@@ -235,17 +240,10 @@ function apply_selector(relative_selectors, rule, element, stylesheet) {
235240
* @param {Compiler.Css.RelativeSelector[]} parent_selectors
236241
* @param {Compiler.Css.Rule} rule
237242
* @param {Compiler.AST.RegularElement | Compiler.AST.SvelteElement} element
238-
* @param {Compiler.Css.StyleSheet} stylesheet
243+
* @param {State} state
239244
* @returns {boolean}
240245
*/
241-
function apply_combinator(
242-
combinator,
243-
relative_selector,
244-
parent_selectors,
245-
rule,
246-
element,
247-
stylesheet
248-
) {
246+
function apply_combinator(combinator, relative_selector, parent_selectors, rule, element, state) {
249247
const name = combinator.name;
250248

251249
switch (name) {
@@ -269,9 +267,9 @@ function apply_combinator(
269267
}
270268

271269
if (parent.type === 'RegularElement' || parent.type === 'SvelteElement') {
272-
if (apply_selector(parent_selectors, rule, parent, stylesheet)) {
270+
if (apply_selector(parent_selectors, rule, parent, state)) {
273271
// TODO the `name === ' '` causes false positives, but removing it causes false negatives...
274-
if (name === ' ' || crossed_component_boundary) {
272+
if (!state.inside_not && (name === ' ' || crossed_component_boundary)) {
275273
mark(parent_selectors[parent_selectors.length - 1], parent);
276274
}
277275

@@ -297,11 +295,15 @@ function apply_combinator(
297295
if (possible_sibling.type === 'RenderTag' || possible_sibling.type === 'SlotElement') {
298296
// `{@render foo()}<p>foo</p>` with `:global(.x) + p` is a match
299297
if (parent_selectors.length === 1 && parent_selectors[0].metadata.is_global) {
300-
mark(relative_selector, element);
298+
if (!state.inside_not) {
299+
mark(relative_selector, element);
300+
}
301301
sibling_matched = true;
302302
}
303-
} else if (apply_selector(parent_selectors, rule, possible_sibling, stylesheet)) {
304-
mark(relative_selector, element);
303+
} else if (apply_selector(parent_selectors, rule, possible_sibling, state)) {
304+
if (!state.inside_not) {
305+
mark(relative_selector, element);
306+
}
305307
sibling_matched = true;
306308
}
307309
}
@@ -381,10 +383,10 @@ const regex_backslash_and_following_character = /\\(.)/g;
381383
* @param {Compiler.Css.RelativeSelector} relative_selector
382384
* @param {Compiler.Css.Rule} rule
383385
* @param {Compiler.AST.RegularElement | Compiler.AST.SvelteElement} element
384-
* @param {Compiler.Css.StyleSheet} stylesheet
386+
* @param {State} state
385387
* @returns {boolean }
386388
*/
387-
function relative_selector_might_apply_to_node(relative_selector, rule, element, stylesheet) {
389+
function relative_selector_might_apply_to_node(relative_selector, rule, element, state) {
388390
// Sort :has(...) selectors in one bucket and everything else into another
389391
const has_selectors = [];
390392
const other_selectors = [];
@@ -458,7 +460,7 @@ function relative_selector_might_apply_to_node(relative_selector, rule, element,
458460
if (
459461
selectors.length === 0 /* is :global(...) */ ||
460462
(element.metadata.scoped && selector_matched) ||
461-
apply_selector(selectors, rule, element, stylesheet)
463+
apply_selector(selectors, rule, element, state)
462464
) {
463465
complex_selector.metadata.used = true;
464466
selector_matched = matched = true;
@@ -504,7 +506,7 @@ function relative_selector_might_apply_to_node(relative_selector, rule, element,
504506
) {
505507
const args = selector.args;
506508
const complex_selector = args.children[0];
507-
return apply_selector(complex_selector.children, rule, element, stylesheet);
509+
return apply_selector(complex_selector.children, rule, element, state);
508510
}
509511

510512
// We came across a :global, everything beyond it is global and therefore a potential match
@@ -515,12 +517,37 @@ function relative_selector_might_apply_to_node(relative_selector, rule, element,
515517

516518
for (const complex_selector of selector.args.children) {
517519
const relative = truncate(complex_selector);
518-
if (
519-
relative.length === 0 /* is :global(...) */ ||
520-
apply_selector(relative, rule, element, stylesheet)
520+
const is_global = relative.length === 0;
521+
522+
if (is_global) {
523+
complex_selector.metadata.used = true;
524+
matched = true;
525+
} else if (name !== 'not' && apply_selector(relative, rule, element, state)) {
526+
complex_selector.metadata.used = true;
527+
matched = true;
528+
} else if (
529+
name === 'not' &&
530+
!apply_selector(relative, rule, element, { ...state, inside_not: true })
521531
) {
532+
// For `:not(...)` we gotta do the inverse: If it did not match, mark the element and possibly
533+
// everything above (if the selector is written is a such) as scoped (because they matched by not matching).
534+
element.metadata.scoped = true;
522535
complex_selector.metadata.used = true;
523536
matched = true;
537+
538+
for (const r of relative) {
539+
r.metadata.scoped = true;
540+
}
541+
542+
// bar:not(foo bar) means that foo is an ancestor of bar
543+
if (complex_selector.children.length > 1) {
544+
/** @type {Compiler.AST.RegularElement | Compiler.AST.SvelteElement | null} */
545+
let el = get_element_parent(element);
546+
while (el) {
547+
el.metadata.scoped = true;
548+
el = get_element_parent(el);
549+
}
550+
}
524551
} else if (complex_selector.children.length > 1 && (name == 'is' || name == 'where')) {
525552
// foo :is(bar baz) can also mean that bar is an ancestor of foo, and baz a descendant.
526553
// We can't fully check if that actually matches with our current algorithm, so we just assume it does.
@@ -618,7 +645,7 @@ function relative_selector_might_apply_to_node(relative_selector, rule, element,
618645
const parent = /** @type {Compiler.Css.Rule} */ (rule.metadata.parent_rule);
619646

620647
for (const complex_selector of parent.prelude.children) {
621-
if (apply_selector(truncate(complex_selector), parent, element, stylesheet)) {
648+
if (apply_selector(truncate(complex_selector), parent, element, state)) {
622649
complex_selector.metadata.used = true;
623650
matched = true;
624651
}
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import { test } from '../../test';
2+
3+
export default test({
4+
warnings: [
5+
{
6+
code: 'css_unused_selector',
7+
message: 'Unused CSS selector ":global(.x) :not(p)"',
8+
start: {
9+
line: 14,
10+
column: 1,
11+
character: 197
12+
},
13+
end: {
14+
line: 14,
15+
column: 20,
16+
character: 216
17+
}
18+
}
19+
]
20+
});
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
2+
.svelte-xyz:not(.foo) {
3+
color: green;
4+
}
5+
.svelte-xyz:not(.foo:where(.svelte-xyz)):not(.unused) {
6+
color: green;
7+
}
8+
.x:not(.foo.svelte-xyz) {
9+
color: green;
10+
}
11+
/* (unused) :global(.x) :not(p) {
12+
color: red;
13+
}*/
14+
.x:not(p.svelte-xyz) {
15+
color: red; /* TODO would be nice to prune this one day */
16+
}
17+
.x .svelte-xyz:not(.unused:where(.svelte-xyz)) {
18+
color: green;
19+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
<p class="foo svelte-xyz">foo</p> <p class="bar svelte-xyz">bar</p>
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<p class="foo">foo</p>
2+
<p class="bar">bar</p>
3+
4+
<style>
5+
:not(:global(.foo)) {
6+
color: green;
7+
}
8+
:not(.foo):not(:global(.unused)) {
9+
color: green;
10+
}
11+
:global(.x):not(.foo) {
12+
color: green;
13+
}
14+
:global(.x) :not(p) {
15+
color: red;
16+
}
17+
:global(.x):not(p) {
18+
color: red; /* TODO would be nice to prune this one day */
19+
}
20+
:global(.x) :not(.unused) {
21+
color: green;
22+
}
23+
</style>
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import { test } from '../../test';
2+
3+
export default test({
4+
warnings: []
5+
});
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
2+
.svelte-xyz:not(.bar:where(.svelte-xyz)) {
3+
color: green;
4+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
<p class="foo svelte-xyz">foo</p> <p class="bar">bar</p>
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<p class="foo">foo</p>
2+
<p class="bar">bar</p>
3+
4+
<style>
5+
:not(.bar) {
6+
color: green;
7+
}
8+
</style>

packages/svelte/tests/css/samples/not-selector/_config.js

Lines changed: 16 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -4,58 +4,44 @@ export default test({
44
warnings: [
55
{
66
code: 'css_unused_selector',
7-
message: 'Unused CSS selector ":not(.unused)"',
7+
message: 'Unused CSS selector ":not(p)"',
88
start: {
9-
line: 8,
9+
line: 11,
1010
column: 1,
11-
character: 89
11+
character: 125
1212
},
1313
end: {
14-
line: 8,
15-
column: 14,
16-
character: 102
17-
}
18-
},
19-
{
20-
code: 'css_unused_selector',
21-
message: 'Unused CSS selector ":not(.foo):not(.unused)"',
22-
start: {
23-
line: 18,
24-
column: 1,
25-
character: 221
26-
},
27-
end: {
28-
line: 18,
29-
column: 24,
30-
character: 244
14+
line: 11,
15+
column: 8,
16+
character: 132
3117
}
3218
},
3319
{
3420
code: 'css_unused_selector',
3521
message: 'Unused CSS selector "p :not(.foo)"',
3622
start: {
37-
line: 25,
23+
line: 22,
3824
column: 1,
39-
character: 300
25+
character: 235
4026
},
4127
end: {
42-
line: 25,
28+
line: 22,
4329
column: 13,
44-
character: 312
30+
character: 247
4531
}
4632
},
4733
{
4834
code: 'css_unused_selector',
49-
message: 'Unused CSS selector ":global(.x) :not(.unused)"',
35+
message: 'Unused CSS selector "p :not(.unused)"',
5036
start: {
51-
line: 34,
37+
line: 25,
5238
column: 1,
53-
character: 469
39+
character: 268
5440
},
5541
end: {
56-
line: 34,
57-
column: 26,
58-
character: 494
42+
line: 25,
43+
column: 16,
44+
character: 283
5945
}
6046
}
6147
]

0 commit comments

Comments
 (0)