Skip to content

Commit 6f03561

Browse files
authored
breaking: scope :not(...) selectors (#13568)
The other part of #13395 This implements scoping for selectors inside `:not(...)`. The approach is almot the same as for `:is/where(...)`. This is a breaking change because people could've used `:not(.unknown)` with `.unknown` not appearing in the HTML, and so they need to do `:not(:global(.unknown))` instead. While implementing it I also discovered a few bugs, which are fixed in this PR: - `foo :is(bar baz)` wasn't properly handled. This selector can mean `foo bar baz` but it can also mean `bar foo baz` (super weird, but it is what it is). Since our current algorithm isn't suited for handling this, we just assume it matches and scope it. Worst case is we missed a prune - `bar` in `:global(foo):is(bar)` was always marked as unused, even if it matched
1 parent 94aea0f commit 6f03561

File tree

11 files changed

+305
-19
lines changed

11 files changed

+305
-19
lines changed

.changeset/nervous-squids-drop.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+
breaking: scope `:not(...)` selectors

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

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -121,16 +121,26 @@ const visitors = {
121121
};
122122

123123
/**
124-
* Discard trailing `:global(...)` selectors without a `:has(...)` modifier, these are unused for scoping purposes
124+
* Discard trailing `:global(...)` selectors without a `:has/is/where/not(...)` modifier, these are unused for scoping purposes
125125
* @param {Compiler.Css.ComplexSelector} node
126126
*/
127127
function truncate(node) {
128128
const i = node.children.findLastIndex(({ metadata, selectors }) => {
129+
const first = selectors[0];
129130
return (
131+
// not after a :global selector
130132
!metadata.is_global_like &&
133+
!(first.type === 'PseudoClassSelector' && first.name === 'global' && first.args === null) &&
134+
// not a :global(...) without a :has/is/where/not(...) modifier
131135
(!metadata.is_global ||
132136
selectors.some(
133-
(selector) => selector.type === 'PseudoClassSelector' && selector.name === 'has'
137+
(selector) =>
138+
selector.type === 'PseudoClassSelector' &&
139+
selector.args !== null &&
140+
(selector.name === 'has' ||
141+
selector.name === 'not' ||
142+
selector.name === 'is' ||
143+
selector.name === 'where')
134144
))
135145
);
136146
});
@@ -450,17 +460,47 @@ function relative_selector_might_apply_to_node(
450460
// We came across a :global, everything beyond it is global and therefore a potential match
451461
if (name === 'global' && selector.args === null) return true;
452462

453-
if ((name === 'is' || name === 'where') && selector.args) {
463+
if ((name === 'is' || name === 'where' || name === 'not') && selector.args) {
454464
let matched = false;
455465

456466
for (const complex_selector of selector.args.children) {
457-
if (apply_selector(truncate(complex_selector), rule, element, stylesheet, check_has)) {
467+
const relative = truncate(complex_selector);
468+
if (
469+
relative.length === 0 /* is :global(...) */ ||
470+
apply_selector(relative, rule, element, stylesheet, check_has)
471+
) {
458472
complex_selector.metadata.used = true;
459473
matched = true;
474+
} else if (complex_selector.children.length > 1 && (name == 'is' || name == 'where')) {
475+
// foo :is(bar baz) can also mean that bar is an ancestor of foo, and baz a descendant.
476+
// We can't fully check if that actually matches with our current algorithm, so we just assume it does.
477+
// The result may not match a real element, so the only drawback is the missing prune.
478+
complex_selector.metadata.used = true;
479+
matched = true;
480+
for (const selector of relative) {
481+
selector.metadata.scoped = true;
482+
}
460483
}
461484
}
462485

463486
if (!matched) {
487+
if (
488+
relative_selector.metadata.is_global &&
489+
!relative_selector.metadata.is_global_like
490+
) {
491+
// Edge case: `:global(.x):is(.y)` where `.x` is global but `.y` doesn't match.
492+
// Since `used` is set to `true` for `:global(.x)` in css-analyze beforehand, and
493+
// we have no way of knowing if it's safe to set it back to `false`, we'll mark
494+
// the inner selector as used and scoped to prevent it from being pruned, which could
495+
// result in a invalid CSS output (e.g. `.x:is(/* unused .y */)`). The result
496+
// can't match a real element, so the only drawback is the missing prune.
497+
// TODO clean this up some day
498+
selector.args.children[0].metadata.used = true;
499+
selector.args.children[0].children.forEach((selector) => {
500+
selector.metadata.scoped = true;
501+
});
502+
}
503+
464504
return false;
465505
}
466506
}

packages/svelte/src/compiler/phases/3-transform/css/index.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ const visitors = {
311311
context.state.specificity.bumped = before_bumped;
312312
},
313313
PseudoClassSelector(node, context) {
314-
if (node.name === 'is' || node.name === 'where' || node.name === 'has') {
314+
if (node.name === 'is' || node.name === 'where' || node.name === 'has' || node.name === 'not') {
315315
context.next();
316316
}
317317
}

packages/svelte/src/compiler/types/css.d.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,8 @@ export namespace Css {
8787
/**
8888
* `true` if the whole selector is unscoped, e.g. `:global(...)` or `:global` or `:global.x`.
8989
* Selectors like `:global(...).x` are not considered global, because they still need scoping.
90+
* Selectors like `:global(...):is/where/not/has(...)` are considered global even if they aren't
91+
* strictly speaking (we should consolidate the logic around this at some point).
9092
*/
9193
is_global: boolean;
9294
/** `:root`, `:host`, `::view-transition`, or selectors after a `:global` */

packages/svelte/tests/css/samples/is/_config.js

Lines changed: 63 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,72 @@ export default test({
44
warnings: [
55
{
66
code: 'css_unused_selector',
7+
message: 'Unused CSS selector ".unused"',
8+
start: {
9+
line: 11,
10+
column: 10,
11+
character: 80
12+
},
713
end: {
8-
character: 38,
9-
column: 11,
10-
line: 6
14+
line: 11,
15+
column: 17,
16+
character: 87
17+
}
18+
},
19+
{
20+
code: 'css_unused_selector',
21+
message: 'Unused CSS selector "x :is(.unused)"',
22+
start: {
23+
line: 14,
24+
column: 1,
25+
character: 111
1126
},
12-
message: 'Unused CSS selector "z"',
27+
end: {
28+
line: 14,
29+
column: 15,
30+
character: 125
31+
}
32+
},
33+
{
34+
code: 'css_unused_selector',
35+
message: 'Unused CSS selector ".unused"',
1336
start: {
14-
character: 37,
15-
column: 10,
16-
line: 6
37+
line: 14,
38+
column: 7,
39+
character: 117
40+
},
41+
end: {
42+
line: 14,
43+
column: 14,
44+
character: 124
45+
}
46+
},
47+
{
48+
code: 'css_unused_selector',
49+
message: 'Unused CSS selector ":global(.foo) :is(.unused)"',
50+
start: {
51+
line: 28,
52+
column: 1,
53+
character: 274
54+
},
55+
end: {
56+
line: 28,
57+
column: 27,
58+
character: 300
59+
}
60+
},
61+
{
62+
code: 'css_unused_selector',
63+
message: 'Unused CSS selector ".unused"',
64+
start: {
65+
line: 28,
66+
column: 19,
67+
character: 292
68+
},
69+
end: {
70+
line: 28,
71+
column: 26,
72+
character: 299
1773
}
1874
}
1975
]
Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,34 @@
11

2-
x.svelte-xyz :is(y:where(.svelte-xyz) /* (unused) z*/) {
3-
color: purple;
2+
x.svelte-xyz :is(y:where(.svelte-xyz)) {
3+
color: green;
4+
}
5+
x.svelte-xyz :is(y:where(.svelte-xyz) /* (unused) .unused*/) {
6+
color: green;
7+
}
8+
/* (unused) x :is(.unused) {
9+
color: red;
10+
}*/
11+
12+
x.svelte-xyz :is(y) {
13+
color: green;
14+
}
15+
x.svelte-xyz :is(.foo) {
16+
color: green;
17+
}
18+
19+
.foo :is(x.svelte-xyz) {
20+
color: green;
21+
}
22+
/* (unused) :global(.foo) :is(.unused) {
23+
color: red;
24+
}*/
25+
26+
x.svelte-xyz :is(html *) {
27+
color: green;
28+
}
29+
x.svelte-xyz :is(html:where(.svelte-xyz) :where(.svelte-xyz)) {
30+
color: red; /* TODO would be nice to prune this one day */
31+
}
32+
y.svelte-xyz :is(x:where(.svelte-xyz) :where(.svelte-xyz)) {
33+
color: green; /* matches z */
434
}
Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,41 @@
11
<x>
2-
<y></y>
2+
<y>
3+
<z></z>
4+
</y>
35
</x>
46

57
<style>
6-
x :is(y, z) {
7-
color: purple;
8+
x :is(y) {
9+
color: green;
10+
}
11+
x :is(y, .unused) {
12+
color: green;
13+
}
14+
x :is(.unused) {
15+
color: red;
16+
}
17+
18+
x :is(:global(y)) {
19+
color: green;
20+
}
21+
x :is(:global(.foo)) {
22+
color: green;
23+
}
24+
25+
:global(.foo) :is(x) {
26+
color: green;
27+
}
28+
:global(.foo) :is(.unused) {
29+
color: red;
30+
}
31+
32+
x :is(:global(html *)) {
33+
color: green;
34+
}
35+
x :is(html *) {
36+
color: red; /* TODO would be nice to prune this one day */
37+
}
38+
y :is(x *) {
39+
color: green; /* matches z */
840
}
941
</style>
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
import { test } from '../../test';
2+
3+
export default test({
4+
warnings: [
5+
{
6+
code: 'css_unused_selector',
7+
message: 'Unused CSS selector ":not(.unused)"',
8+
start: {
9+
line: 8,
10+
column: 1,
11+
character: 89
12+
},
13+
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
31+
}
32+
},
33+
{
34+
code: 'css_unused_selector',
35+
message: 'Unused CSS selector "p :not(.foo)"',
36+
start: {
37+
line: 25,
38+
column: 1,
39+
character: 300
40+
},
41+
end: {
42+
line: 25,
43+
column: 13,
44+
character: 312
45+
}
46+
},
47+
{
48+
code: 'css_unused_selector',
49+
message: 'Unused CSS selector ":global(.x) :not(.unused)"',
50+
start: {
51+
line: 34,
52+
column: 1,
53+
character: 469
54+
},
55+
end: {
56+
line: 34,
57+
column: 26,
58+
character: 494
59+
}
60+
}
61+
]
62+
});
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,33 @@
1+
2+
.svelte-xyz:not(.foo:where(.svelte-xyz)) {
3+
color: green;
4+
}
5+
/* (unused) :not(.unused) {
6+
color: red;
7+
}*/
18
.svelte-xyz:not(.foo) {
9+
color: green;
10+
}
11+
12+
.svelte-xyz:not(.foo:where(.svelte-xyz)):not(.unused) {
13+
color: green;
14+
}
15+
/* (unused) :not(.foo):not(.unused) {
216
color: red;
17+
}*/
18+
19+
p.svelte-xyz:not(.foo:where(.svelte-xyz)) {
20+
color: green;
321
}
22+
/* (unused) p :not(.foo) {
23+
color: red;
24+
}*/
25+
.x:not(.foo.svelte-xyz) {
26+
color: green;
27+
}
28+
.x:not(.unused.svelte-xyz) {
29+
color: red; /* TODO would be nice to prune this one day */
30+
}
31+
/* (unused) :global(.x) :not(.unused) {
32+
color: red;
33+
}*/

packages/svelte/tests/css/samples/not-selector/input.svelte

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,35 @@
33

44
<style>
55
:not(.foo) {
6+
color: green;
7+
}
8+
:not(.unused) {
9+
color: red;
10+
}
11+
:not(:global(.foo)) {
12+
color: green;
13+
}
14+
15+
:not(.foo):not(:global(.unused)) {
16+
color: green;
17+
}
18+
:not(.foo):not(.unused) {
19+
color: red;
20+
}
21+
22+
p:not(.foo) {
23+
color: green;
24+
}
25+
p :not(.foo) {
26+
color: red;
27+
}
28+
:global(.x):not(.foo) {
29+
color: green;
30+
}
31+
:global(.x):not(.unused) {
32+
color: red; /* TODO would be nice to prune this one day */
33+
}
34+
:global(.x) :not(.unused) {
635
color: red;
736
}
837
</style>

0 commit comments

Comments
 (0)