Skip to content

breaking: scope :not(...) selectors #13568

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Oct 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/nervous-squids-drop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

breaking: scope `:not(...)` selectors
48 changes: 44 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 @@ -121,16 +121,26 @@ const visitors = {
};

/**
* Discard trailing `:global(...)` selectors without a `:has(...)` modifier, these are unused for scoping purposes
* Discard trailing `:global(...)` selectors without a `:has/is/where/not(...)` modifier, these are unused for scoping purposes
* @param {Compiler.Css.ComplexSelector} node
*/
function truncate(node) {
const i = node.children.findLastIndex(({ metadata, selectors }) => {
const first = selectors[0];
return (
// not after a :global selector
!metadata.is_global_like &&
!(first.type === 'PseudoClassSelector' && first.name === 'global' && first.args === null) &&
// not a :global(...) without a :has/is/where/not(...) modifier
(!metadata.is_global ||
selectors.some(
(selector) => selector.type === 'PseudoClassSelector' && selector.name === 'has'
(selector) =>
selector.type === 'PseudoClassSelector' &&
selector.args !== null &&
(selector.name === 'has' ||
selector.name === 'not' ||
selector.name === 'is' ||
selector.name === 'where')
))
);
});
Expand Down Expand Up @@ -440,17 +450,47 @@ function relative_selector_might_apply_to_node(
// 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') && selector.args) {
if ((name === 'is' || name === 'where' || name === 'not') && selector.args) {
let matched = false;

for (const complex_selector of selector.args.children) {
if (apply_selector(truncate(complex_selector), rule, element, stylesheet, check_has)) {
const relative = truncate(complex_selector);
if (
relative.length === 0 /* is :global(...) */ ||
apply_selector(relative, rule, element, stylesheet, check_has)
) {
complex_selector.metadata.used = true;
matched = true;
} 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.
// The result may not match a real element, so the only drawback is the missing prune.
complex_selector.metadata.used = true;
matched = true;
for (const selector of relative) {
selector.metadata.scoped = true;
}
}
}

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

return false;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ const visitors = {
context.state.specificity.bumped = before_bumped;
},
PseudoClassSelector(node, context) {
if (node.name === 'is' || node.name === 'where' || node.name === 'has') {
if (node.name === 'is' || node.name === 'where' || node.name === 'has' || node.name === 'not') {
context.next();
}
}
Expand Down
2 changes: 2 additions & 0 deletions packages/svelte/src/compiler/types/css.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ export namespace Css {
/**
* `true` if the whole selector is unscoped, e.g. `:global(...)` or `:global` or `:global.x`.
* Selectors like `:global(...).x` are not considered global, because they still need scoping.
* Selectors like `:global(...):is/where/not/has(...)` are considered global even if they aren't
* strictly speaking (we should consolidate the logic around this at some point).
*/
is_global: boolean;
/** `:root`, `:host`, `::view-transition`, or selectors after a `:global` */
Expand Down
70 changes: 63 additions & 7 deletions packages/svelte/tests/css/samples/is/_config.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,72 @@ export default test({
warnings: [
{
code: 'css_unused_selector',
message: 'Unused CSS selector ".unused"',
start: {
line: 11,
column: 10,
character: 80
},
end: {
character: 38,
column: 11,
line: 6
line: 11,
column: 17,
character: 87
}
},
{
code: 'css_unused_selector',
message: 'Unused CSS selector "x :is(.unused)"',
start: {
line: 14,
column: 1,
character: 111
},
message: 'Unused CSS selector "z"',
end: {
line: 14,
column: 15,
character: 125
}
},
{
code: 'css_unused_selector',
message: 'Unused CSS selector ".unused"',
start: {
character: 37,
column: 10,
line: 6
line: 14,
column: 7,
character: 117
},
end: {
line: 14,
column: 14,
character: 124
}
},
{
code: 'css_unused_selector',
message: 'Unused CSS selector ":global(.foo) :is(.unused)"',
start: {
line: 28,
column: 1,
character: 274
},
end: {
line: 28,
column: 27,
character: 300
}
},
{
code: 'css_unused_selector',
message: 'Unused CSS selector ".unused"',
start: {
line: 28,
column: 19,
character: 292
},
end: {
line: 28,
column: 26,
character: 299
}
}
]
Expand Down
34 changes: 32 additions & 2 deletions packages/svelte/tests/css/samples/is/expected.css
Original file line number Diff line number Diff line change
@@ -1,4 +1,34 @@

x.svelte-xyz :is(y:where(.svelte-xyz) /* (unused) z*/) {
color: purple;
x.svelte-xyz :is(y:where(.svelte-xyz)) {
color: green;
}
x.svelte-xyz :is(y:where(.svelte-xyz) /* (unused) .unused*/) {
color: green;
}
/* (unused) x :is(.unused) {
color: red;
}*/

x.svelte-xyz :is(y) {
color: green;
}
x.svelte-xyz :is(.foo) {
color: green;
}

.foo :is(x.svelte-xyz) {
color: green;
}
/* (unused) :global(.foo) :is(.unused) {
color: red;
}*/

x.svelte-xyz :is(html *) {
color: green;
}
x.svelte-xyz :is(html:where(.svelte-xyz) :where(.svelte-xyz)) {
color: red; /* TODO would be nice to prune this one day */
}
y.svelte-xyz :is(x:where(.svelte-xyz) :where(.svelte-xyz)) {
color: green; /* matches z */
}
38 changes: 35 additions & 3 deletions packages/svelte/tests/css/samples/is/input.svelte
Original file line number Diff line number Diff line change
@@ -1,9 +1,41 @@
<x>
<y></y>
<y>
<z></z>
</y>
</x>

<style>
x :is(y, z) {
color: purple;
x :is(y) {
color: green;
}
x :is(y, .unused) {
color: green;
}
x :is(.unused) {
color: red;
}

x :is(:global(y)) {
color: green;
}
x :is(:global(.foo)) {
color: green;
}

:global(.foo) :is(x) {
color: green;
}
:global(.foo) :is(.unused) {
color: red;
}

x :is(:global(html *)) {
color: green;
}
x :is(html *) {
color: red; /* TODO would be nice to prune this one day */
}
y :is(x *) {
color: green; /* matches z */
}
</style>
62 changes: 62 additions & 0 deletions packages/svelte/tests/css/samples/not-selector/_config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import { test } from '../../test';

export default test({
warnings: [
{
code: 'css_unused_selector',
message: 'Unused CSS selector ":not(.unused)"',
start: {
line: 8,
column: 1,
character: 89
},
end: {
line: 8,
column: 14,
character: 102
}
},
{
code: 'css_unused_selector',
message: 'Unused CSS selector ":not(.foo):not(.unused)"',
start: {
line: 18,
column: 1,
character: 221
},
end: {
line: 18,
column: 24,
character: 244
}
},
{
code: 'css_unused_selector',
message: 'Unused CSS selector "p :not(.foo)"',
start: {
line: 25,
column: 1,
character: 300
},
end: {
line: 25,
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
}
}
]
});
30 changes: 30 additions & 0 deletions packages/svelte/tests/css/samples/not-selector/expected.css
Original file line number Diff line number Diff line change
@@ -1,3 +1,33 @@

.svelte-xyz:not(.foo:where(.svelte-xyz)) {
color: green;
}
/* (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;
}*/

p.svelte-xyz:not(.foo:where(.svelte-xyz)) {
color: green;
}
/* (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;
}*/
29 changes: 29 additions & 0 deletions packages/svelte/tests/css/samples/not-selector/input.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,35 @@

<style>
:not(.foo) {
color: green;
}
:not(.unused) {
color: red;
}
:not(:global(.foo)) {
color: green;
}

:not(.foo):not(:global(.unused)) {
color: green;
}
:not(.foo):not(.unused) {
color: red;
}

p:not(.foo) {
color: green;
}
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>
Loading