Skip to content

Commit 1089c7b

Browse files
committed
fix: remove scoping for :not selectors
fixes #14168 This reverts the whole "selectors inside `:not` are scoped" logic. Scoping is done so that styles don't bleed. But within `:not`,everything is reversed, which means scoping the selectors now means they are more likely to bleed. That is the opposite of what we want to achieve, therefore we should just leave those selectors alone.
1 parent d033377 commit 1089c7b

File tree

15 files changed

+77
-129
lines changed

15 files changed

+77
-129
lines changed

.changeset/quick-eels-occur.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: remove scoping for `:not` selectors

packages/svelte/src/compiler/migrate/index.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,9 @@ function migrate_css(state) {
5050
while (code) {
5151
if (
5252
code.startsWith(':has') ||
53-
code.startsWith(':not') ||
5453
code.startsWith(':is') ||
55-
code.startsWith(':where')
54+
code.startsWith(':where') ||
55+
code.startsWith(':not')
5656
) {
5757
let start = code.indexOf('(') + 1;
5858
let is_global = false;
@@ -74,7 +74,7 @@ function migrate_css(state) {
7474
char = code[end];
7575
}
7676
if (start && end) {
77-
if (!is_global) {
77+
if (!is_global && !code.startsWith(':not')) {
7878
str.prependLeft(starting + start, ':global(');
7979
str.appendRight(starting + end - 1, ')');
8080
}

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

Lines changed: 10 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,9 @@ const visitors = {
162162
};
163163

164164
/**
165-
* Discard trailing `:global(...)` selectors without a `:has/is/where/not(...)` modifier, these are unused for scoping purposes
165+
* Discard trailing `:global(...)` selectors without a `:has/is/where(...)` modifier, these are unused for scoping purposes
166+
* `:not(...)` modifiers are not considered, because they should stay unscoped, because scoping them would achieve the
167+
* opposite of what we want, because they are then _more_ likely to bleed out of the component.
166168
* @param {Compiler.Css.ComplexSelector} node
167169
*/
168170
function truncate(node) {
@@ -172,16 +174,13 @@ function truncate(node) {
172174
// not after a :global selector
173175
!metadata.is_global_like &&
174176
!(first.type === 'PseudoClassSelector' && first.name === 'global' && first.args === null) &&
175-
// not a :global(...) without a :has/is/where/not(...) modifier
177+
// not a :global(...) without a :has/is/where(...) modifier
176178
(!metadata.is_global ||
177179
selectors.some(
178180
(selector) =>
179181
selector.type === 'PseudoClassSelector' &&
180182
selector.args !== null &&
181-
(selector.name === 'has' ||
182-
selector.name === 'not' ||
183-
selector.name === 'is' ||
184-
selector.name === 'where')
183+
(selector.name === 'has' || selector.name === 'is' || selector.name === 'where')
185184
))
186185
);
187186
});
@@ -512,7 +511,10 @@ function relative_selector_might_apply_to_node(relative_selector, rule, element,
512511
// We came across a :global, everything beyond it is global and therefore a potential match
513512
if (name === 'global' && selector.args === null) return true;
514513

515-
if ((name === 'is' || name === 'where' || name === 'not') && selector.args) {
514+
// We ignore :not(...) because its contents should stay unscoped. Scoping them would achieve the
515+
// opposite of what we want, because they are then _more_ likely to bleed out of the component,
516+
// because there would be more chances of the inner selector not matching, which means `:not` matches.
517+
if ((name === 'is' || name === 'where') && selector.args) {
516518
let matched = false;
517519

518520
for (const complex_selector of selector.args.children) {
@@ -522,32 +524,9 @@ function relative_selector_might_apply_to_node(relative_selector, rule, element,
522524
if (is_global) {
523525
complex_selector.metadata.used = true;
524526
matched = true;
525-
} else if (name !== 'not' && apply_selector(relative, rule, element, state)) {
527+
} else if (apply_selector(relative, rule, element, state)) {
526528
complex_selector.metadata.used = true;
527529
matched = true;
528-
} else if (
529-
name === 'not' &&
530-
!apply_selector(relative, rule, element, { ...state, inside_not: true })
531-
) {
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;
535-
complex_selector.metadata.used = true;
536-
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-
}
551530
} else if (complex_selector.children.length > 1 && (name == 'is' || name == 'where')) {
552531
// foo :is(bar baz) can also mean that bar is an ancestor of foo, and baz a descendant.
553532
// We can't fully check if that actually matches with our current algorithm, so we just assume it does.

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

Lines changed: 33 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -278,29 +278,10 @@ const visitors = {
278278
ComplexSelector(node, context) {
279279
const before_bumped = context.state.specificity.bumped;
280280

281-
/**
282-
* @param {Css.PseudoClassSelector} selector
283-
* @param {Css.Combinator | null} combinator
284-
*/
285-
function remove_global_pseudo_class(selector, combinator) {
286-
if (selector.args === null) {
287-
let start = selector.start;
288-
if (combinator?.name === ' ') {
289-
// div :global.x becomes div.x
290-
while (/\s/.test(context.state.code.original[start - 1])) start--;
291-
}
292-
context.state.code.remove(start, selector.start + ':global'.length);
293-
} else {
294-
context.state.code
295-
.remove(selector.start, selector.start + ':global('.length)
296-
.remove(selector.end - 1, selector.end);
297-
}
298-
}
299-
300281
for (const relative_selector of node.children) {
301282
if (relative_selector.metadata.is_global) {
302283
const global = /** @type {Css.PseudoClassSelector} */ (relative_selector.selectors[0]);
303-
remove_global_pseudo_class(global, relative_selector.combinator);
284+
remove_global_pseudo_class(global, relative_selector.combinator, context.state);
304285

305286
if (
306287
node.metadata.rule?.metadata.parent_rule &&
@@ -328,7 +309,7 @@ const visitors = {
328309
// for any :global() or :global at the middle of compound selector
329310
for (const selector of relative_selector.selectors) {
330311
if (selector.type === 'PseudoClassSelector' && selector.name === 'global') {
331-
remove_global_pseudo_class(selector, null);
312+
remove_global_pseudo_class(selector, null, context.state);
332313
}
333314
}
334315

@@ -374,12 +355,42 @@ const visitors = {
374355
context.state.specificity.bumped = before_bumped;
375356
},
376357
PseudoClassSelector(node, context) {
377-
if (node.name === 'is' || node.name === 'where' || node.name === 'has' || node.name === 'not') {
358+
if (node.name === 'is' || node.name === 'where' || node.name === 'has') {
378359
context.next();
379360
}
361+
if (node.name === 'not' && node.args) {
362+
for (const complex_selector of node.args.children) {
363+
for (const relative_selector of complex_selector.children) {
364+
if (relative_selector.metadata.is_global) {
365+
const global = /** @type {Css.PseudoClassSelector} */ (relative_selector.selectors[0]);
366+
remove_global_pseudo_class(global, relative_selector.combinator, context.state);
367+
}
368+
}
369+
}
370+
}
380371
}
381372
};
382373

374+
/**
375+
* @param {Css.PseudoClassSelector} selector
376+
* @param {Css.Combinator | null} combinator
377+
* @param {State} state
378+
*/
379+
function remove_global_pseudo_class(selector, combinator, state) {
380+
if (selector.args === null) {
381+
let start = selector.start;
382+
if (combinator?.name === ' ') {
383+
// div :global.x becomes div.x
384+
while (/\s/.test(state.code.original[start - 1])) start--;
385+
}
386+
state.code.remove(start, selector.start + ':global'.length);
387+
} else {
388+
state.code
389+
.remove(selector.start, selector.start + ':global('.length)
390+
.remove(selector.end - 1, selector.end);
391+
}
392+
}
393+
383394
/**
384395
* Walk backwards until we find a non-whitespace character
385396
* @param {number} end
Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,5 @@
11
import { test } from '../../test';
22

33
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-
]
4+
warnings: []
205
});

packages/svelte/tests/css/samples/not-selector-global/expected.css

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,18 @@
22
.svelte-xyz:not(.foo) {
33
color: green;
44
}
5-
.svelte-xyz:not(.foo:where(.svelte-xyz)):not(.unused) {
5+
.svelte-xyz:not(.foo):not(.unused) {
66
color: green;
77
}
8-
.x:not(.foo.svelte-xyz) {
8+
.x:not(.foo) {
99
color: green;
1010
}
11-
/* (unused) :global(.x) :not(p) {
12-
color: red;
13-
}*/
14-
.x:not(p.svelte-xyz) {
11+
.x .svelte-xyz:not(p) {
1512
color: red; /* TODO would be nice to prune this one day */
1613
}
17-
.x .svelte-xyz:not(.unused:where(.svelte-xyz)) {
14+
.x:not(p) {
15+
color: red; /* TODO would be nice to prune this one day */
16+
}
17+
.x .svelte-xyz:not(.unused) {
1818
color: green;
1919
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
color: green;
1313
}
1414
:global(.x) :not(p) {
15-
color: red;
15+
color: red; /* TODO would be nice to prune this one day */
1616
}
1717
:global(.x):not(p) {
1818
color: red; /* TODO would be nice to prune this one day */

packages/svelte/tests/css/samples/not-selector-hash-on-right-elements/_config.js

Lines changed: 0 additions & 5 deletions
This file was deleted.

packages/svelte/tests/css/samples/not-selector-hash-on-right-elements/expected.css

Lines changed: 0 additions & 4 deletions
This file was deleted.

packages/svelte/tests/css/samples/not-selector-hash-on-right-elements/expected.html

Lines changed: 0 additions & 1 deletion
This file was deleted.

packages/svelte/tests/css/samples/not-selector-hash-on-right-elements/input.svelte

Lines changed: 0 additions & 8 deletions
This file was deleted.

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

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2,32 +2,18 @@ import { test } from '../../test';
22

33
export default test({
44
warnings: [
5-
{
6-
code: 'css_unused_selector',
7-
message: 'Unused CSS selector ":not(p)"',
8-
start: {
9-
line: 11,
10-
column: 1,
11-
character: 125
12-
},
13-
end: {
14-
line: 11,
15-
column: 8,
16-
character: 132
17-
}
18-
},
195
{
206
code: 'css_unused_selector',
217
message: 'Unused CSS selector "p :not(.foo)"',
228
start: {
239
line: 22,
2410
column: 1,
25-
character: 235
11+
character: 291
2612
},
2713
end: {
2814
line: 22,
2915
column: 13,
30-
character: 247
16+
character: 303
3117
}
3218
},
3319
{
@@ -36,12 +22,12 @@ export default test({
3622
start: {
3723
line: 25,
3824
column: 1,
39-
character: 268
25+
character: 324
4026
},
4127
end: {
4228
line: 25,
4329
column: 16,
44-
character: 283
30+
character: 339
4531
}
4632
}
4733
]

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,19 @@
11

2-
.svelte-xyz:not(.foo:where(.svelte-xyz)) {
2+
.svelte-xyz:not(.foo) {
33
color: green;
44
}
5-
.svelte-xyz:not(.unused:where(.svelte-xyz)) {
5+
.svelte-xyz:not(.unused) {
66
color: green;
77
}
8-
/* (unused) :not(p) {
9-
color: red;
10-
}*/
8+
.svelte-xyz:not(p) {
9+
color: red; /* TODO would be nice to mark this as unused someday */
10+
}
1111

12-
.svelte-xyz:not(.foo:where(.svelte-xyz)):not(.unused:where(.svelte-xyz)) {
12+
.svelte-xyz:not(.foo):not(.unused) {
1313
color: green;
1414
}
1515

16-
p.svelte-xyz:not(.foo:where(.svelte-xyz)) {
16+
p.svelte-xyz:not(.foo) {
1717
color: green;
1818
}
1919
/* (unused) p :not(.foo) {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
color: green;
1010
}
1111
:not(p) {
12-
color: red;
12+
color: red; /* TODO would be nice to mark this as unused someday */
1313
}
1414
1515
:not(.foo):not(.unused) {

packages/svelte/tests/migrate/samples/is-not-where-has/output.svelte

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,22 +21,22 @@ what if i'm talking about `:has()` in my blog?
2121

2222
<style lang="postcss">
2323
div:has(:global(span)){}
24-
div > :not(:global(span)){}
24+
div > :not(span){}
2525
div > :is(:global(span)){}
2626
div > :where(:global(span)){}
2727
2828
div:has(:global(:is(span))){}
29-
div > :not(:global(:is(span))){}
29+
div > :not(:is(span)){}
3030
div > :is(:global(:is(span))){}
3131
div > :where(:global(:is(span))){}
3232
3333
div:has(:global(.class:is(span))){}
34-
div > :not(:global(.class:is(span))){}
34+
div > :not(.class:is(span)){}
3535
div > :is(:global(.class:is(span))){}
3636
div > :where(:global(.class:is(span))){}
3737
3838
div :has(:global(.class:is(span:where(:focus)))){}
39-
div :not(:global(.class:is(span:where(:focus-within)))){}
39+
div :not(.class:is(span:where(:focus-within))){}
4040
div :is(:global(.class:is(span:is(:hover)))){}
4141
div :where(:global(.class:is(span:has(* > *)))){}
4242
div :is(:global(.class:is(span:is(:hover)), .x)){}
@@ -51,7 +51,7 @@ what if i'm talking about `:has()` in my blog?
5151
p:has(:global(&)){
5252
5353
}
54-
:not(:global(span > *)){
54+
:not(span > *){
5555
:where(:global(form)){}
5656
}
5757
}

0 commit comments

Comments
 (0)