fix: remove scoping for most :not selectors (#14177)

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.

The exception are `:not` selectors with descendant selectors, as that means "look up the tree" and we need to scope all ancestor elements in that case.

Author: dummdidumm

.changeset/quick-eels-occur.md

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

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

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

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

@@ -10,7 +10,6 @@ import { get_attribute_chunks, is_text_attribute } from '../../../utils/ast.js';
  *   stylesheet: Compiler.Css.StyleSheet;
  *   element: Compiler.AST.RegularElement | Compiler.AST.SvelteElement;
  *   from_render_tag: boolean;
- *   inside_not: boolean;
  * }} State
  */
 /** @typedef {NODE_PROBABLY_EXISTS | NODE_DEFINITELY_EXISTS} NodeExistsValue */
@@ -62,13 +61,9 @@ export function prune(stylesheet, element) {
 		const parent = get_element_parent(element);
 		if (!parent) return;
 
-		walk(
-			stylesheet,
-			{ stylesheet, element: parent, from_render_tag: true, inside_not: false },
-			visitors
-		);
+		walk(stylesheet, { stylesheet, element: parent, from_render_tag: true }, visitors);
 	} else {
-		walk(stylesheet, { stylesheet, element, from_render_tag: false, inside_not: false }, visitors);
+		walk(stylesheet, { stylesheet, element, from_render_tag: false }, visitors);
 	}
 }
 
@@ -179,9 +174,9 @@ function truncate(node) {
 						selector.type === 'PseudoClassSelector' &&
 						selector.args !== null &&
 						(selector.name === 'has' ||
-							selector.name === 'not' ||
 							selector.name === 'is' ||
-							selector.name === 'where')
+							selector.name === 'where' ||
+							selector.name === 'not')
 				))
 		);
 	});
@@ -227,7 +222,7 @@ function apply_selector(relative_selectors, rule, element, state) {
 	// if this is the left-most non-global selector, mark it — we want
 	// `x y z {...}` to become `x.blah y z.blah {...}`
 	const parent = parent_selectors[parent_selectors.length - 1];
-	if (!state.inside_not && (!parent || is_global(parent, rule))) {
+	if (!parent || is_global(parent, rule)) {
 		mark(relative_selector, element);
 	}
 
@@ -269,7 +264,7 @@ function apply_combinator(combinator, relative_selector, parent_selectors, rule,
 				if (parent.type === 'RegularElement' || parent.type === 'SvelteElement') {
 					if (apply_selector(parent_selectors, rule, parent, state)) {
 						// TODO the `name === ' '` causes false positives, but removing it causes false negatives...
-						if (!state.inside_not && (name === ' ' || crossed_component_boundary)) {
+						if (name === ' ' || crossed_component_boundary) {
 							mark(parent_selectors[parent_selectors.length - 1], parent);
 						}
 
@@ -295,15 +290,11 @@ function apply_combinator(combinator, relative_selector, parent_selectors, rule,
 				if (possible_sibling.type === 'RenderTag' || possible_sibling.type === 'SlotElement') {
 					// `{@render foo()}<p>foo</p>` with `:global(.x) + p` is a match
 					if (parent_selectors.length === 1 && parent_selectors[0].metadata.is_global) {
-						if (!state.inside_not) {
-							mark(relative_selector, element);
-						}
+						mark(relative_selector, element);
 						sibling_matched = true;
 					}
 				} else if (apply_selector(parent_selectors, rule, possible_sibling, state)) {
-					if (!state.inside_not) {
-						mark(relative_selector, element);
-					}
+					mark(relative_selector, element);
 					sibling_matched = true;
 				}
 			}
@@ -512,7 +503,35 @@ 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
 				if (name === 'global' && selector.args === null) return true;
 
-				if ((name === 'is' || name === 'where' || name === 'not') && selector.args) {
+				// :not(...) 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. The exception is complex selectors
+				// with descendants, in which case we scope them all.
+				if (name === 'not' && selector.args) {
+					for (const complex_selector of selector.args.children) {
+						complex_selector.metadata.used = true;
+						const relative = truncate(complex_selector);
+
+						if (complex_selector.children.length > 1) {
+							// foo:not(bar foo) means that bar is an ancestor of foo (side note: ending with foo is the only way the selector make sense).
+							// We can't fully check if that actually matches with our current algorithm, so we just assume it does.
+							// The result may not match a real element, so the only drawback is the missing prune.
+							for (const selector of relative) {
+								selector.metadata.scoped = true;
+							}
+
+							/** @type {Compiler.AST.RegularElement | Compiler.AST.SvelteElement | null} */
+							let el = element;
+							while (el) {
+								el.metadata.scoped = true;
+								el = get_element_parent(el);
+							}
+						}
+					}
+
+					break;
+				}
+
+				if ((name === 'is' || name === 'where') && selector.args) {
 					let matched = false;
 
 					for (const complex_selector of selector.args.children) {
@@ -522,32 +541,9 @@ function relative_selector_might_apply_to_node(relative_selector, rule, element,
 						if (is_global) {
 							complex_selector.metadata.used = 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;
 							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')) {
 							// 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.

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

@@ -278,29 +278,10 @@ const visitors = {
 	ComplexSelector(node, context) {
 		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) {
 			if (relative_selector.metadata.is_global) {
 				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 (
 					node.metadata.rule?.metadata.parent_rule &&
@@ -328,7 +309,7 @@ const visitors = {
 				// for any :global() or :global at the middle of compound selector
 				for (const selector of relative_selector.selectors) {
 					if (selector.type === 'PseudoClassSelector' && selector.name === 'global') {
-						remove_global_pseudo_class(selector, null);
+						remove_global_pseudo_class(selector, null, context.state);
 					}
 				}
 
@@ -380,6 +361,26 @@ const visitors = {
 	}
 };
 
+/**
+ * @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
  * @param {number} end

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

@@ -1,20 +1,5 @@
 import { test } from '../../test';
 
 export default test({
-	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
-			}
-		}
-	]
+	warnings: []
 });

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

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

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

@@ -1 +1,3 @@
-<p class="foo svelte-xyz">foo</p> <p class="bar svelte-xyz">bar</p>
+<p class="foo svelte-xyz">foo</p>
+<p class="bar svelte-xyz">bar <span class="svelte-xyz">baz</span></p>
+<span class="svelte-xyz">buzz</span>

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

@@ -1,5 +1,9 @@
 <p class="foo">foo</p>
-<p class="bar">bar</p>
+<p class="bar">
+	bar
+	<span>baz</span>
+</p>
+<span>buzz</span>
 
 <style>
 	:not(:global(.foo)) {
@@ -12,12 +16,22 @@
 		color: green;
 	}
 	:global(.x) :not(p) {
-		color: red;
+		color: green;
 	}
 	:global(.x):not(p) {
-		color: red; /* TODO would be nice to prune this one day */
+		color: green;
 	}
 	:global(.x) :not(.unused) {
 		color: green;
 	}
+
+	:global(span):not(p span) {
+		color: green;
+	}
+	span:not(:global(p span)) {
+		color: green;
+	}
+	:global(span:not(p span)) {
+		color: green;
+	}
 </style>