Skip to content

Commit

Permalink
fix: ensure non-matching elements are scoped for :not(...) selector
Browse files Browse the repository at this point in the history
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.

Fixes #13974
  • Loading branch information
dummdidumm committed Oct 28, 2024
1 parent e99e865 commit f704196
Show file tree
Hide file tree
Showing 10 changed files with 90 additions and 56 deletions.
5 changes: 5 additions & 0 deletions .changeset/dirty-parrots-smell.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: ensure non-matching elements are scoped for `:not(...)` selector
23 changes: 19 additions & 4 deletions packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js
Original file line number Diff line number Diff line change
Expand Up @@ -515,12 +515,27 @@ function relative_selector_might_apply_to_node(relative_selector, rule, element,

for (const complex_selector of selector.args.children) {
const relative = truncate(complex_selector);
if (
relative.length === 0 /* is :global(...) */ ||
apply_selector(relative, rule, element, stylesheet)
) {
const is_global = relative.length === 0;

if (is_global || apply_selector(relative, rule, element, stylesheet)) {
complex_selector.metadata.used = true;
matched = true;
} else if (name === 'not') {
// 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).
// The above if condition will ensure the selector itself will be marked as used if it doesn't match at least once,
// and therefore having actual usefulness in the CSS output.
element.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.
Expand Down
20 changes: 20 additions & 0 deletions packages/svelte/tests/css/samples/not-selector-global/_config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { test } from '../../test';

export default test({
warnings: [
{
code: 'css_unused_selector',
message: 'Unused CSS selector ":global(.x) :not(.unused)"',
start: {
line: 17,
column: 1,
character: 289
},
end: {
line: 17,
column: 26,
character: 314
}
}
]
});
16 changes: 16 additions & 0 deletions packages/svelte/tests/css/samples/not-selector-global/expected.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@

.svelte-xyz:not(.foo) {
color: green;
}
.svelte-xyz:not(.foo:where(.svelte-xyz)):not(.unused) {
color: green;
}
.x:not(.foo.svelte-xyz) {
color: green;
}
.x:not(.unused.svelte-xyz) {
color: red; /* TODO would be nice to prune this one day */
}
/* (unused) :global(.x) :not(.unused) {
color: red;
}*/
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<p class="foo svelte-xyz">foo</p> <p class="bar svelte-xyz">bar</p>
20 changes: 20 additions & 0 deletions packages/svelte/tests/css/samples/not-selector-global/input.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<p class="foo">foo</p>
<p class="bar">bar</p>

<style>
:not(:global(.foo)) {
color: green;
}
:not(.foo):not(:global(.unused)) {
color: green;
}
:global(.x):not(.foo) {
color: green;
}
:global(.x):not(.unused) {
color: red; /* TODO would be nice to prune this one day */
}
:global(.x) :not(.unused) {
color: red;
}
</style>
30 changes: 8 additions & 22 deletions packages/svelte/tests/css/samples/not-selector/_config.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,42 +20,28 @@ export default test({
code: 'css_unused_selector',
message: 'Unused CSS selector ":not(.foo):not(.unused)"',
start: {
line: 18,
line: 12,
column: 1,
character: 221
character: 124
},
end: {
line: 18,
line: 12,
column: 24,
character: 244
character: 147
}
},
{
code: 'css_unused_selector',
message: 'Unused CSS selector "p :not(.foo)"',
start: {
line: 25,
line: 19,
column: 1,
character: 300
character: 203
},
end: {
line: 25,
line: 19,
column: 13,
character: 312
}
},
{
code: 'css_unused_selector',
message: 'Unused CSS selector ":global(.x) :not(.unused)"',
start: {
line: 34,
column: 1,
character: 469
},
end: {
line: 34,
column: 26,
character: 494
character: 215
}
}
]
Expand Down
15 changes: 0 additions & 15 deletions packages/svelte/tests/css/samples/not-selector/expected.css
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,7 @@
/* (unused) :not(.unused) {
color: red;
}*/
.svelte-xyz:not(.foo) {
color: green;
}

.svelte-xyz:not(.foo:where(.svelte-xyz)):not(.unused) {
color: green;
}
/* (unused) :not(.foo):not(.unused) {
color: red;
}*/
Expand All @@ -22,12 +16,3 @@
/* (unused) p :not(.foo) {
color: red;
}*/
.x:not(.foo.svelte-xyz) {
color: green;
}
.x:not(.unused.svelte-xyz) {
color: red; /* TODO would be nice to prune this one day */
}
/* (unused) :global(.x) :not(.unused) {
color: red;
}*/
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<p class="foo svelte-xyz">foo</p> <p class="bar svelte-xyz">bar</p>
15 changes: 0 additions & 15 deletions packages/svelte/tests/css/samples/not-selector/input.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,7 @@
:not(.unused) {
color: red;
}
:not(:global(.foo)) {
color: green;
}
:not(.foo):not(:global(.unused)) {
color: green;
}
:not(.foo):not(.unused) {
color: red;
}
Expand All @@ -25,13 +19,4 @@
p :not(.foo) {
color: red;
}
:global(.x):not(.foo) {
color: green;
}
:global(.x):not(.unused) {
color: red; /* TODO would be nice to prune this one day */
}
:global(.x) :not(.unused) {
color: red;
}
</style>

0 comments on commit f704196

Please sign in to comment.