Skip to content
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

fix: ensure non-matching elements are scoped for :not(...) selector #13999

Merged
merged 3 commits into from
Nov 4, 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/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
89 changes: 58 additions & 31 deletions packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ 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 */
Expand Down Expand Up @@ -61,9 +62,13 @@ export function prune(stylesheet, element) {
const parent = get_element_parent(element);
if (!parent) return;

walk(stylesheet, { stylesheet, element: parent, from_render_tag: true }, visitors);
walk(
stylesheet,
{ stylesheet, element: parent, from_render_tag: true, inside_not: false },
visitors
);
} else {
walk(stylesheet, { stylesheet, element, from_render_tag: false }, visitors);
walk(stylesheet, { stylesheet, element, from_render_tag: false, inside_not: false }, visitors);
}
}

Expand Down Expand Up @@ -127,7 +132,7 @@ const visitors = {
selectors_to_check,
/** @type {Compiler.Css.Rule} */ (node.metadata.rule),
element,
context.state.stylesheet
context.state
)
) {
mark(inner, element);
Expand All @@ -143,7 +148,7 @@ const visitors = {
selectors,
/** @type {Compiler.Css.Rule} */ (node.metadata.rule),
context.state.element,
context.state.stylesheet
context.state
)
) {
mark(inner, context.state.element);
Expand Down Expand Up @@ -188,10 +193,10 @@ function truncate(node) {
* @param {Compiler.Css.RelativeSelector[]} relative_selectors
* @param {Compiler.Css.Rule} rule
* @param {Compiler.AST.RegularElement | Compiler.AST.SvelteElement} element
* @param {Compiler.Css.StyleSheet} stylesheet
* @param {State} state
* @returns {boolean}
*/
function apply_selector(relative_selectors, rule, element, stylesheet) {
function apply_selector(relative_selectors, rule, element, state) {
const parent_selectors = relative_selectors.slice();
const relative_selector = parent_selectors.pop();

Expand All @@ -201,7 +206,7 @@ function apply_selector(relative_selectors, rule, element, stylesheet) {
relative_selector,
rule,
element,
stylesheet
state
);

if (!possible_match) {
Expand All @@ -215,14 +220,14 @@ function apply_selector(relative_selectors, rule, element, stylesheet) {
parent_selectors,
rule,
element,
stylesheet
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 (!parent || is_global(parent, rule)) {
if (!state.inside_not && (!parent || is_global(parent, rule))) {
mark(relative_selector, element);
}

Expand All @@ -235,17 +240,10 @@ function apply_selector(relative_selectors, rule, element, stylesheet) {
* @param {Compiler.Css.RelativeSelector[]} parent_selectors
* @param {Compiler.Css.Rule} rule
* @param {Compiler.AST.RegularElement | Compiler.AST.SvelteElement} element
* @param {Compiler.Css.StyleSheet} stylesheet
* @param {State} state
* @returns {boolean}
*/
function apply_combinator(
combinator,
relative_selector,
parent_selectors,
rule,
element,
stylesheet
) {
function apply_combinator(combinator, relative_selector, parent_selectors, rule, element, state) {
const name = combinator.name;

switch (name) {
Expand All @@ -269,9 +267,9 @@ function apply_combinator(
}

if (parent.type === 'RegularElement' || parent.type === 'SvelteElement') {
if (apply_selector(parent_selectors, rule, parent, stylesheet)) {
if (apply_selector(parent_selectors, rule, parent, state)) {
// TODO the `name === ' '` causes false positives, but removing it causes false negatives...
if (name === ' ' || crossed_component_boundary) {
if (!state.inside_not && (name === ' ' || crossed_component_boundary)) {
mark(parent_selectors[parent_selectors.length - 1], parent);
}

Expand All @@ -297,11 +295,15 @@ function apply_combinator(
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) {
mark(relative_selector, element);
if (!state.inside_not) {
mark(relative_selector, element);
}
sibling_matched = true;
}
} else if (apply_selector(parent_selectors, rule, possible_sibling, stylesheet)) {
mark(relative_selector, element);
} else if (apply_selector(parent_selectors, rule, possible_sibling, state)) {
if (!state.inside_not) {
mark(relative_selector, element);
}
sibling_matched = true;
}
}
Expand Down Expand Up @@ -381,10 +383,10 @@ const regex_backslash_and_following_character = /\\(.)/g;
* @param {Compiler.Css.RelativeSelector} relative_selector
* @param {Compiler.Css.Rule} rule
* @param {Compiler.AST.RegularElement | Compiler.AST.SvelteElement} element
* @param {Compiler.Css.StyleSheet} stylesheet
* @param {State} state
* @returns {boolean }
*/
function relative_selector_might_apply_to_node(relative_selector, rule, element, stylesheet) {
function relative_selector_might_apply_to_node(relative_selector, rule, element, state) {
// Sort :has(...) selectors in one bucket and everything else into another
const has_selectors = [];
const other_selectors = [];
Expand Down Expand Up @@ -458,7 +460,7 @@ function relative_selector_might_apply_to_node(relative_selector, rule, element,
if (
selectors.length === 0 /* is :global(...) */ ||
(element.metadata.scoped && selector_matched) ||
apply_selector(selectors, rule, element, stylesheet)
apply_selector(selectors, rule, element, state)
) {
complex_selector.metadata.used = true;
selector_matched = matched = true;
Expand Down Expand Up @@ -504,7 +506,7 @@ function relative_selector_might_apply_to_node(relative_selector, rule, element,
) {
const args = selector.args;
const complex_selector = args.children[0];
return apply_selector(complex_selector.children, rule, element, stylesheet);
return apply_selector(complex_selector.children, rule, element, state);
}

// We came across a :global, everything beyond it is global and therefore a potential match
Expand All @@ -515,12 +517,37 @@ 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) {
complex_selector.metadata.used = true;
matched = true;
} else if (name !== 'not' && 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.
Expand Down Expand Up @@ -618,7 +645,7 @@ function relative_selector_might_apply_to_node(relative_selector, rule, element,
const parent = /** @type {Compiler.Css.Rule} */ (rule.metadata.parent_rule);

for (const complex_selector of parent.prelude.children) {
if (apply_selector(truncate(complex_selector), parent, element, stylesheet)) {
if (apply_selector(truncate(complex_selector), parent, element, state)) {
complex_selector.metadata.used = true;
matched = true;
}
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(p)"',
start: {
line: 14,
column: 1,
character: 197
},
end: {
line: 14,
column: 20,
character: 216
}
}
]
});
19 changes: 19 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,19 @@

.svelte-xyz:not(.foo) {
color: green;
}
.svelte-xyz:not(.foo:where(.svelte-xyz)):not(.unused) {
color: green;
}
.x:not(.foo.svelte-xyz) {
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(.unused:where(.svelte-xyz)) {
color: green;
}
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>
23 changes: 23 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,23 @@
<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(p) {
color: red;
}
:global(.x):not(p) {
color: red; /* TODO would be nice to prune this one day */
}
:global(.x) :not(.unused) {
color: green;
}
</style>
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { test } from '../../test';

export default test({
warnings: []
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@

.svelte-xyz:not(.bar:where(.svelte-xyz)) {
color: green;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<p class="foo svelte-xyz">foo</p> <p class="bar">bar</p>
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<p class="foo">foo</p>
<p class="bar">bar</p>

<style>
:not(.bar) {
color: green;
}
</style>
46 changes: 16 additions & 30 deletions packages/svelte/tests/css/samples/not-selector/_config.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,58 +4,44 @@ export default test({
warnings: [
{
code: 'css_unused_selector',
message: 'Unused CSS selector ":not(.unused)"',
message: 'Unused CSS selector ":not(p)"',
start: {
line: 8,
line: 11,
column: 1,
character: 89
character: 125
},
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
line: 11,
column: 8,
character: 132
}
},
{
code: 'css_unused_selector',
message: 'Unused CSS selector "p :not(.foo)"',
start: {
line: 25,
line: 22,
column: 1,
character: 300
character: 235
},
end: {
line: 25,
line: 22,
column: 13,
character: 312
character: 247
}
},
{
code: 'css_unused_selector',
message: 'Unused CSS selector ":global(.x) :not(.unused)"',
message: 'Unused CSS selector "p :not(.unused)"',
start: {
line: 34,
line: 25,
column: 1,
character: 469
character: 268
},
end: {
line: 34,
column: 26,
character: 494
line: 25,
column: 16,
character: 283
}
}
]
Expand Down
Loading