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.
pull/14177/head
Simon Holthausen 2 years ago
parent d03337707f
commit 1089c7b157

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: remove scoping for `:not` selectors

@ -50,9 +50,9 @@ function migrate_css(state) {
while (code) { while (code) {
if ( if (
code.startsWith(':has') || code.startsWith(':has') ||
code.startsWith(':not') ||
code.startsWith(':is') || code.startsWith(':is') ||
code.startsWith(':where') code.startsWith(':where') ||
code.startsWith(':not')
) { ) {
let start = code.indexOf('(') + 1; let start = code.indexOf('(') + 1;
let is_global = false; let is_global = false;
@ -74,7 +74,7 @@ function migrate_css(state) {
char = code[end]; char = code[end];
} }
if (start && end) { if (start && end) {
if (!is_global) { if (!is_global && !code.startsWith(':not')) {
str.prependLeft(starting + start, ':global('); str.prependLeft(starting + start, ':global(');
str.appendRight(starting + end - 1, ')'); str.appendRight(starting + end - 1, ')');
} }

@ -162,7 +162,9 @@ const visitors = {
}; };
/** /**
* Discard trailing `:global(...)` selectors without a `:has/is/where/not(...)` modifier, these are unused for scoping purposes * Discard trailing `:global(...)` selectors without a `:has/is/where(...)` modifier, these are unused for scoping purposes
* `:not(...)` modifiers are not considered, because they should stay unscoped, because scoping them would achieve the
* opposite of what we want, because they are then _more_ likely to bleed out of the component.
* @param {Compiler.Css.ComplexSelector} node * @param {Compiler.Css.ComplexSelector} node
*/ */
function truncate(node) { function truncate(node) {
@ -172,16 +174,13 @@ function truncate(node) {
// not after a :global selector // not after a :global selector
!metadata.is_global_like && !metadata.is_global_like &&
!(first.type === 'PseudoClassSelector' && first.name === 'global' && first.args === null) && !(first.type === 'PseudoClassSelector' && first.name === 'global' && first.args === null) &&
// not a :global(...) without a :has/is/where/not(...) modifier // not a :global(...) without a :has/is/where(...) modifier
(!metadata.is_global || (!metadata.is_global ||
selectors.some( selectors.some(
(selector) => (selector) =>
selector.type === 'PseudoClassSelector' && selector.type === 'PseudoClassSelector' &&
selector.args !== null && selector.args !== null &&
(selector.name === 'has' || (selector.name === 'has' || selector.name === 'is' || selector.name === 'where')
selector.name === 'not' ||
selector.name === 'is' ||
selector.name === 'where')
)) ))
); );
}); });
@ -512,7 +511,10 @@ function relative_selector_might_apply_to_node(relative_selector, rule, element,
// We came across a :global, everything beyond it is global and therefore a potential match // We came across a :global, everything beyond it is global and therefore a potential match
if (name === 'global' && selector.args === null) return true; if (name === 'global' && selector.args === null) return true;
if ((name === 'is' || name === 'where' || name === 'not') && selector.args) { // We ignore :not(...) because its contents should stay unscoped. Scoping them would achieve the
// opposite of what we want, because they are then _more_ likely to bleed out of the component,
// because there would be more chances of the inner selector not matching, which means `:not` matches.
if ((name === 'is' || name === 'where') && selector.args) {
let matched = false; let matched = false;
for (const complex_selector of selector.args.children) { for (const complex_selector of selector.args.children) {
@ -522,32 +524,9 @@ function relative_selector_might_apply_to_node(relative_selector, rule, element,
if (is_global) { if (is_global) {
complex_selector.metadata.used = true; complex_selector.metadata.used = true;
matched = true; matched = true;
} else if (name !== 'not' && apply_selector(relative, rule, element, state)) { } else if (apply_selector(relative, rule, element, state)) {
complex_selector.metadata.used = true; complex_selector.metadata.used = true;
matched = true; matched = true;
} else if (
name === 'not' &&
!apply_selector(relative, rule, element, { ...state, inside_not: true })
) {
// For `:not(...)` we gotta do the inverse: If it did not match, mark the element and possibly
// everything above (if the selector is written is a such) as scoped (because they matched by not matching).
element.metadata.scoped = true;
complex_selector.metadata.used = true;
matched = true;
for (const r of relative) {
r.metadata.scoped = true;
}
// bar:not(foo bar) means that foo is an ancestor of bar
if (complex_selector.children.length > 1) {
/** @type {Compiler.AST.RegularElement | Compiler.AST.SvelteElement | null} */
let el = get_element_parent(element);
while (el) {
el.metadata.scoped = true;
el = get_element_parent(el);
}
}
} else if (complex_selector.children.length > 1 && (name == 'is' || name == 'where')) { } else if (complex_selector.children.length > 1 && (name == 'is' || name == 'where')) {
// foo :is(bar baz) can also mean that bar is an ancestor of foo, and baz a descendant. // foo :is(bar baz) can also mean that bar is an ancestor of foo, and baz a descendant.
// We can't fully check if that actually matches with our current algorithm, so we just assume it does. // We can't fully check if that actually matches with our current algorithm, so we just assume it does.

@ -278,29 +278,10 @@ const visitors = {
ComplexSelector(node, context) { ComplexSelector(node, context) {
const before_bumped = context.state.specificity.bumped; const before_bumped = context.state.specificity.bumped;
/**
* @param {Css.PseudoClassSelector} selector
* @param {Css.Combinator | null} combinator
*/
function remove_global_pseudo_class(selector, combinator) {
if (selector.args === null) {
let start = selector.start;
if (combinator?.name === ' ') {
// div :global.x becomes div.x
while (/\s/.test(context.state.code.original[start - 1])) start--;
}
context.state.code.remove(start, selector.start + ':global'.length);
} else {
context.state.code
.remove(selector.start, selector.start + ':global('.length)
.remove(selector.end - 1, selector.end);
}
}
for (const relative_selector of node.children) { for (const relative_selector of node.children) {
if (relative_selector.metadata.is_global) { if (relative_selector.metadata.is_global) {
const global = /** @type {Css.PseudoClassSelector} */ (relative_selector.selectors[0]); const global = /** @type {Css.PseudoClassSelector} */ (relative_selector.selectors[0]);
remove_global_pseudo_class(global, relative_selector.combinator); remove_global_pseudo_class(global, relative_selector.combinator, context.state);
if ( if (
node.metadata.rule?.metadata.parent_rule && node.metadata.rule?.metadata.parent_rule &&
@ -328,7 +309,7 @@ const visitors = {
// for any :global() or :global at the middle of compound selector // for any :global() or :global at the middle of compound selector
for (const selector of relative_selector.selectors) { for (const selector of relative_selector.selectors) {
if (selector.type === 'PseudoClassSelector' && selector.name === 'global') { if (selector.type === 'PseudoClassSelector' && selector.name === 'global') {
remove_global_pseudo_class(selector, null); remove_global_pseudo_class(selector, null, context.state);
} }
} }
@ -374,12 +355,42 @@ const visitors = {
context.state.specificity.bumped = before_bumped; context.state.specificity.bumped = before_bumped;
}, },
PseudoClassSelector(node, context) { PseudoClassSelector(node, context) {
if (node.name === 'is' || node.name === 'where' || node.name === 'has' || node.name === 'not') { if (node.name === 'is' || node.name === 'where' || node.name === 'has') {
context.next(); context.next();
} }
if (node.name === 'not' && node.args) {
for (const complex_selector of node.args.children) {
for (const relative_selector of complex_selector.children) {
if (relative_selector.metadata.is_global) {
const global = /** @type {Css.PseudoClassSelector} */ (relative_selector.selectors[0]);
remove_global_pseudo_class(global, relative_selector.combinator, context.state);
}
}
}
}
} }
}; };
/**
* @param {Css.PseudoClassSelector} selector
* @param {Css.Combinator | null} combinator
* @param {State} state
*/
function remove_global_pseudo_class(selector, combinator, state) {
if (selector.args === null) {
let start = selector.start;
if (combinator?.name === ' ') {
// div :global.x becomes div.x
while (/\s/.test(state.code.original[start - 1])) start--;
}
state.code.remove(start, selector.start + ':global'.length);
} else {
state.code
.remove(selector.start, selector.start + ':global('.length)
.remove(selector.end - 1, selector.end);
}
}
/** /**
* Walk backwards until we find a non-whitespace character * Walk backwards until we find a non-whitespace character
* @param {number} end * @param {number} end

@ -1,20 +1,5 @@
import { test } from '../../test'; import { test } from '../../test';
export default test({ export default test({
warnings: [ warnings: []
{
code: 'css_unused_selector',
message: 'Unused CSS selector ":global(.x) :not(p)"',
start: {
line: 14,
column: 1,
character: 197
},
end: {
line: 14,
column: 20,
character: 216
}
}
]
}); });

@ -2,18 +2,18 @@
.svelte-xyz:not(.foo) { .svelte-xyz:not(.foo) {
color: green; color: green;
} }
.svelte-xyz:not(.foo:where(.svelte-xyz)):not(.unused) { .svelte-xyz:not(.foo):not(.unused) {
color: green; color: green;
} }
.x:not(.foo.svelte-xyz) { .x:not(.foo) {
color: green; color: green;
} }
/* (unused) :global(.x) :not(p) { .x .svelte-xyz:not(p) {
color: red;
}*/
.x:not(p.svelte-xyz) {
color: red; /* TODO would be nice to prune this one day */ color: red; /* TODO would be nice to prune this one day */
} }
.x .svelte-xyz:not(.unused:where(.svelte-xyz)) { .x:not(p) {
color: red; /* TODO would be nice to prune this one day */
}
.x .svelte-xyz:not(.unused) {
color: green; color: green;
} }

@ -12,7 +12,7 @@
color: green; color: green;
} }
:global(.x) :not(p) { :global(.x) :not(p) {
color: red; color: red; /* TODO would be nice to prune this one day */
} }
:global(.x):not(p) { :global(.x):not(p) {
color: red; /* TODO would be nice to prune this one day */ color: red; /* TODO would be nice to prune this one day */

@ -1,5 +0,0 @@
import { test } from '../../test';
export default test({
warnings: []
});

@ -1,4 +0,0 @@
.svelte-xyz:not(.bar:where(.svelte-xyz)) {
color: green;
}

@ -1 +0,0 @@
<p class="foo svelte-xyz">foo</p> <p class="bar">bar</p>

@ -1,8 +0,0 @@
<p class="foo">foo</p>
<p class="bar">bar</p>
<style>
:not(.bar) {
color: green;
}
</style>

@ -2,32 +2,18 @@ import { test } from '../../test';
export default test({ export default test({
warnings: [ warnings: [
{
code: 'css_unused_selector',
message: 'Unused CSS selector ":not(p)"',
start: {
line: 11,
column: 1,
character: 125
},
end: {
line: 11,
column: 8,
character: 132
}
},
{ {
code: 'css_unused_selector', code: 'css_unused_selector',
message: 'Unused CSS selector "p :not(.foo)"', message: 'Unused CSS selector "p :not(.foo)"',
start: { start: {
line: 22, line: 22,
column: 1, column: 1,
character: 235 character: 291
}, },
end: { end: {
line: 22, line: 22,
column: 13, column: 13,
character: 247 character: 303
} }
}, },
{ {
@ -36,12 +22,12 @@ export default test({
start: { start: {
line: 25, line: 25,
column: 1, column: 1,
character: 268 character: 324
}, },
end: { end: {
line: 25, line: 25,
column: 16, column: 16,
character: 283 character: 339
} }
} }
] ]

@ -1,19 +1,19 @@
.svelte-xyz:not(.foo:where(.svelte-xyz)) { .svelte-xyz:not(.foo) {
color: green; color: green;
} }
.svelte-xyz:not(.unused:where(.svelte-xyz)) { .svelte-xyz:not(.unused) {
color: green; color: green;
} }
/* (unused) :not(p) { .svelte-xyz:not(p) {
color: red; color: red; /* TODO would be nice to mark this as unused someday */
}*/ }
.svelte-xyz:not(.foo:where(.svelte-xyz)):not(.unused:where(.svelte-xyz)) { .svelte-xyz:not(.foo):not(.unused) {
color: green; color: green;
} }
p.svelte-xyz:not(.foo:where(.svelte-xyz)) { p.svelte-xyz:not(.foo) {
color: green; color: green;
} }
/* (unused) p :not(.foo) { /* (unused) p :not(.foo) {

@ -9,7 +9,7 @@
color: green; color: green;
} }
:not(p) { :not(p) {
color: red; color: red; /* TODO would be nice to mark this as unused someday */
} }
:not(.foo):not(.unused) { :not(.foo):not(.unused) {

@ -21,22 +21,22 @@ what if i'm talking about `:has()` in my blog?
<style lang="postcss"> <style lang="postcss">
div:has(:global(span)){} div:has(:global(span)){}
div > :not(:global(span)){} div > :not(span){}
div > :is(:global(span)){} div > :is(:global(span)){}
div > :where(:global(span)){} div > :where(:global(span)){}
div:has(:global(:is(span))){} div:has(:global(:is(span))){}
div > :not(:global(:is(span))){} div > :not(:is(span)){}
div > :is(:global(:is(span))){} div > :is(:global(:is(span))){}
div > :where(:global(:is(span))){} div > :where(:global(:is(span))){}
div:has(:global(.class:is(span))){} div:has(:global(.class:is(span))){}
div > :not(:global(.class:is(span))){} div > :not(.class:is(span)){}
div > :is(:global(.class:is(span))){} div > :is(:global(.class:is(span))){}
div > :where(:global(.class:is(span))){} div > :where(:global(.class:is(span))){}
div :has(:global(.class:is(span:where(:focus)))){} div :has(:global(.class:is(span:where(:focus)))){}
div :not(:global(.class:is(span:where(:focus-within)))){} div :not(.class:is(span:where(:focus-within))){}
div :is(:global(.class:is(span:is(:hover)))){} div :is(:global(.class:is(span:is(:hover)))){}
div :where(:global(.class:is(span:has(* > *)))){} div :where(:global(.class:is(span:has(* > *)))){}
div :is(:global(.class:is(span:is(:hover)), .x)){} div :is(:global(.class:is(span:is(:hover)), .x)){}
@ -51,7 +51,7 @@ what if i'm talking about `:has()` in my blog?
p:has(:global(&)){ p:has(:global(&)){
} }
:not(:global(span > *)){ :not(span > *){
:where(:global(form)){} :where(:global(form)){}
} }
} }

Loading…
Cancel
Save