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

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. We're also making sure that contents of `:not` that never match actually count as a used (because the result is negated), and as such the contents of `:not` that always match are actually marked as unused.

Fixes #13974
pull/14176/head
Simon H 10 months ago committed by GitHub
parent 57e27ae90e
commit 7dbe812fc9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: ensure non-matching elements are scoped for `:not(...)` selector

@ -10,6 +10,7 @@ import { get_attribute_chunks, is_text_attribute } from '../../../utils/ast.js';
* stylesheet: Compiler.Css.StyleSheet; * stylesheet: Compiler.Css.StyleSheet;
* element: Compiler.AST.RegularElement | Compiler.AST.SvelteElement; * element: Compiler.AST.RegularElement | Compiler.AST.SvelteElement;
* from_render_tag: boolean; * from_render_tag: boolean;
* inside_not: boolean;
* }} State * }} State
*/ */
/** @typedef {NODE_PROBABLY_EXISTS | NODE_DEFINITELY_EXISTS} NodeExistsValue */ /** @typedef {NODE_PROBABLY_EXISTS | NODE_DEFINITELY_EXISTS} NodeExistsValue */
@ -61,9 +62,13 @@ export function prune(stylesheet, element) {
const parent = get_element_parent(element); const parent = get_element_parent(element);
if (!parent) return; 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 { } else {
walk(stylesheet, { stylesheet, element, from_render_tag: false }, visitors); walk(stylesheet, { stylesheet, element, from_render_tag: false, inside_not: false }, visitors);
} }
} }
@ -127,7 +132,7 @@ const visitors = {
selectors_to_check, selectors_to_check,
/** @type {Compiler.Css.Rule} */ (node.metadata.rule), /** @type {Compiler.Css.Rule} */ (node.metadata.rule),
element, element,
context.state.stylesheet context.state
) )
) { ) {
mark(inner, element); mark(inner, element);
@ -143,7 +148,7 @@ const visitors = {
selectors, selectors,
/** @type {Compiler.Css.Rule} */ (node.metadata.rule), /** @type {Compiler.Css.Rule} */ (node.metadata.rule),
context.state.element, context.state.element,
context.state.stylesheet context.state
) )
) { ) {
mark(inner, context.state.element); mark(inner, context.state.element);
@ -188,10 +193,10 @@ function truncate(node) {
* @param {Compiler.Css.RelativeSelector[]} relative_selectors * @param {Compiler.Css.RelativeSelector[]} relative_selectors
* @param {Compiler.Css.Rule} rule * @param {Compiler.Css.Rule} rule
* @param {Compiler.AST.RegularElement | Compiler.AST.SvelteElement} element * @param {Compiler.AST.RegularElement | Compiler.AST.SvelteElement} element
* @param {Compiler.Css.StyleSheet} stylesheet * @param {State} state
* @returns {boolean} * @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 parent_selectors = relative_selectors.slice();
const relative_selector = parent_selectors.pop(); const relative_selector = parent_selectors.pop();
@ -201,7 +206,7 @@ function apply_selector(relative_selectors, rule, element, stylesheet) {
relative_selector, relative_selector,
rule, rule,
element, element,
stylesheet state
); );
if (!possible_match) { if (!possible_match) {
@ -215,14 +220,14 @@ function apply_selector(relative_selectors, rule, element, stylesheet) {
parent_selectors, parent_selectors,
rule, rule,
element, element,
stylesheet state
); );
} }
// if this is the left-most non-global selector, mark it — we want // if this is the left-most non-global selector, mark it — we want
// `x y z {...}` to become `x.blah y z.blah {...}` // `x y z {...}` to become `x.blah y z.blah {...}`
const parent = parent_selectors[parent_selectors.length - 1]; 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); mark(relative_selector, element);
} }
@ -235,17 +240,10 @@ function apply_selector(relative_selectors, rule, element, stylesheet) {
* @param {Compiler.Css.RelativeSelector[]} parent_selectors * @param {Compiler.Css.RelativeSelector[]} parent_selectors
* @param {Compiler.Css.Rule} rule * @param {Compiler.Css.Rule} rule
* @param {Compiler.AST.RegularElement | Compiler.AST.SvelteElement} element * @param {Compiler.AST.RegularElement | Compiler.AST.SvelteElement} element
* @param {Compiler.Css.StyleSheet} stylesheet * @param {State} state
* @returns {boolean} * @returns {boolean}
*/ */
function apply_combinator( function apply_combinator(combinator, relative_selector, parent_selectors, rule, element, state) {
combinator,
relative_selector,
parent_selectors,
rule,
element,
stylesheet
) {
const name = combinator.name; const name = combinator.name;
switch (name) { switch (name) {
@ -269,9 +267,9 @@ function apply_combinator(
} }
if (parent.type === 'RegularElement' || parent.type === 'SvelteElement') { 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... // 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); mark(parent_selectors[parent_selectors.length - 1], parent);
} }
@ -297,11 +295,15 @@ function apply_combinator(
if (possible_sibling.type === 'RenderTag' || possible_sibling.type === 'SlotElement') { if (possible_sibling.type === 'RenderTag' || possible_sibling.type === 'SlotElement') {
// `{@render foo()}<p>foo</p>` with `:global(.x) + p` is a match // `{@render foo()}<p>foo</p>` with `:global(.x) + p` is a match
if (parent_selectors.length === 1 && parent_selectors[0].metadata.is_global) { 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; sibling_matched = true;
} }
} else if (apply_selector(parent_selectors, rule, possible_sibling, stylesheet)) { } 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; sibling_matched = true;
} }
} }
@ -381,10 +383,10 @@ const regex_backslash_and_following_character = /\\(.)/g;
* @param {Compiler.Css.RelativeSelector} relative_selector * @param {Compiler.Css.RelativeSelector} relative_selector
* @param {Compiler.Css.Rule} rule * @param {Compiler.Css.Rule} rule
* @param {Compiler.AST.RegularElement | Compiler.AST.SvelteElement} element * @param {Compiler.AST.RegularElement | Compiler.AST.SvelteElement} element
* @param {Compiler.Css.StyleSheet} stylesheet * @param {State} state
* @returns {boolean } * @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 // Sort :has(...) selectors in one bucket and everything else into another
const has_selectors = []; const has_selectors = [];
const other_selectors = []; const other_selectors = [];
@ -458,7 +460,7 @@ function relative_selector_might_apply_to_node(relative_selector, rule, element,
if ( if (
selectors.length === 0 /* is :global(...) */ || selectors.length === 0 /* is :global(...) */ ||
(element.metadata.scoped && selector_matched) || (element.metadata.scoped && selector_matched) ||
apply_selector(selectors, rule, element, stylesheet) apply_selector(selectors, rule, element, state)
) { ) {
complex_selector.metadata.used = true; complex_selector.metadata.used = true;
selector_matched = matched = true; selector_matched = matched = true;
@ -504,7 +506,7 @@ function relative_selector_might_apply_to_node(relative_selector, rule, element,
) { ) {
const args = selector.args; const args = selector.args;
const complex_selector = args.children[0]; 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 // We came across a :global, everything beyond it is global and therefore a potential match
@ -515,12 +517,37 @@ function relative_selector_might_apply_to_node(relative_selector, rule, element,
for (const complex_selector of selector.args.children) { for (const complex_selector of selector.args.children) {
const relative = truncate(complex_selector); const relative = truncate(complex_selector);
if ( const is_global = relative.length === 0;
relative.length === 0 /* is :global(...) */ ||
apply_selector(relative, rule, element, stylesheet) 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; complex_selector.metadata.used = true;
matched = 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.
@ -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); const parent = /** @type {Compiler.Css.Rule} */ (rule.metadata.parent_rule);
for (const complex_selector of parent.prelude.children) { 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; complex_selector.metadata.used = true;
matched = true; matched = true;
} }

@ -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
}
}
]
});

@ -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;
}

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

@ -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>

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

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

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

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

@ -4,58 +4,44 @@ export default test({
warnings: [ warnings: [
{ {
code: 'css_unused_selector', code: 'css_unused_selector',
message: 'Unused CSS selector ":not(.unused)"', message: 'Unused CSS selector ":not(p)"',
start: { start: {
line: 8, line: 11,
column: 1, column: 1,
character: 89 character: 125
}, },
end: { end: {
line: 8, line: 11,
column: 14, column: 8,
character: 102 character: 132
}
},
{
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', code: 'css_unused_selector',
message: 'Unused CSS selector "p :not(.foo)"', message: 'Unused CSS selector "p :not(.foo)"',
start: { start: {
line: 25, line: 22,
column: 1, column: 1,
character: 300 character: 235
}, },
end: { end: {
line: 25, line: 22,
column: 13, column: 13,
character: 312 character: 247
} }
}, },
{ {
code: 'css_unused_selector', code: 'css_unused_selector',
message: 'Unused CSS selector ":global(.x) :not(.unused)"', message: 'Unused CSS selector "p :not(.unused)"',
start: { start: {
line: 34, line: 25,
column: 1, column: 1,
character: 469 character: 268
}, },
end: { end: {
line: 34, line: 25,
column: 26, column: 16,
character: 494 character: 283
} }
} }
] ]

@ -2,19 +2,16 @@
.svelte-xyz:not(.foo:where(.svelte-xyz)) { .svelte-xyz:not(.foo:where(.svelte-xyz)) {
color: green; color: green;
} }
/* (unused) :not(.unused) { .svelte-xyz:not(.unused:where(.svelte-xyz)) {
color: red;
}*/
.svelte-xyz:not(.foo) {
color: green; color: green;
} }
/* (unused) :not(p) {
color: red;
}*/
.svelte-xyz:not(.foo:where(.svelte-xyz)):not(.unused) { .svelte-xyz:not(.foo:where(.svelte-xyz)):not(.unused:where(.svelte-xyz)) {
color: green; color: green;
} }
/* (unused) :not(.foo):not(.unused) {
color: red;
}*/
p.svelte-xyz:not(.foo:where(.svelte-xyz)) { p.svelte-xyz:not(.foo:where(.svelte-xyz)) {
color: green; color: green;
@ -22,12 +19,6 @@
/* (unused) p :not(.foo) { /* (unused) p :not(.foo) {
color: red; color: red;
}*/ }*/
.x:not(.foo.svelte-xyz) { /* (unused) p :not(.unused) {
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; color: red;
}*/ }*/

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

@ -6,17 +6,14 @@
color: green; color: green;
} }
:not(.unused) { :not(.unused) {
color: red;
}
:not(:global(.foo)) {
color: green; color: green;
} }
:not(p) {
:not(.foo):not(:global(.unused)) { color: red;
color: green;
} }
:not(.foo):not(.unused) { :not(.foo):not(.unused) {
color: red; color: green;
} }
p:not(.foo) { p:not(.foo) {
@ -25,13 +22,7 @@
p :not(.foo) { p :not(.foo) {
color: red; color: red;
} }
:global(.x):not(.foo) { p :not(.unused) {
color: green;
}
:global(.x):not(.unused) {
color: red; /* TODO would be nice to prune this one day */
}
:global(.x) :not(.unused) {
color: red; color: red;
} }
</style> </style>
Loading…
Cancel
Save