Skip to content

Commit 45fa678

Browse files
authored
fix: account for :has(...) as part of :root (#14229)
We previously marked all `:root` selectors as global-like, which excempted them from further analysis. This causes problems: - things like `:not(...)` are never visited and therefore never marked as used -> we gotta do that directly when coming across this - `:has(...)` was never visited, too. Just marking it as used is not enough though, because we might need to scope its contents Therefore the logic is enhanced to account for these special cases. Fixes #14118 While fixing this I cleaned up some inconsistencies in what we mark as global. This simplified code and fixed some adjacent bugs, which conindicentally also fixes #14189
1 parent ac9b7de commit 45fa678

File tree

20 files changed

+484
-117
lines changed

20 files changed

+484
-117
lines changed

.changeset/beige-files-pull.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'svelte': patch
3+
---
4+
5+
fix: account for `:has(...)` as part of `:root`

.changeset/great-bulldogs-wonder.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'svelte': patch
3+
---
4+
5+
fix: prevent nested pseudo class from being marked as unused

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

Lines changed: 22 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import { walk } from 'zimmerframe';
55
import * as e from '../../../errors.js';
66
import { is_keyframes_node } from '../../css.js';
7+
import { is_global, is_unscoped_pseudo_class } from './utils.js';
78

89
/**
910
* @typedef {Visitors<
@@ -15,27 +16,6 @@ import { is_keyframes_node } from '../../css.js';
1516
* >} CssVisitors
1617
*/
1718

18-
/**
19-
* True if is `:global(...)` or `:global`
20-
* @param {Css.RelativeSelector} relative_selector
21-
* @returns {relative_selector is Css.RelativeSelector & { selectors: [Css.PseudoClassSelector, ...Array<Css.PseudoClassSelector | Css.PseudoElementSelector>] }}
22-
*/
23-
function is_global(relative_selector) {
24-
const first = relative_selector.selectors[0];
25-
26-
return (
27-
first.type === 'PseudoClassSelector' &&
28-
first.name === 'global' &&
29-
(first.args === null ||
30-
// Only these two selector types keep the whole selector global, because e.g.
31-
// :global(button).x means that the selector is still scoped because of the .x
32-
relative_selector.selectors.every(
33-
(selector) =>
34-
selector.type === 'PseudoClassSelector' || selector.type === 'PseudoElementSelector'
35-
))
36-
);
37-
}
38-
3919
/**
4020
* True if is `:global`
4121
* @param {Css.SimpleSelector} simple_selector
@@ -119,11 +99,14 @@ const css_visitors = {
11999
node.metadata.rule?.metadata.parent_rule &&
120100
node.children[0]?.selectors[0]?.type === 'NestingSelector'
121101
) {
102+
const first = node.children[0]?.selectors[1];
103+
const no_nesting_scope =
104+
first?.type !== 'PseudoClassSelector' || is_unscoped_pseudo_class(first);
122105
const parent_is_global = node.metadata.rule.metadata.parent_rule.prelude.children.some(
123106
(child) => child.children.length === 1 && child.children[0].metadata.is_global
124107
);
125108
// mark `&:hover` in `:global(.foo) { &:hover { color: green }}` as used
126-
if (parent_is_global) {
109+
if (no_nesting_scope && parent_is_global) {
127110
node.metadata.used = true;
128111
}
129112
}
@@ -156,9 +139,23 @@ const css_visitors = {
156139
].includes(first.name));
157140
}
158141

159-
node.metadata.is_global_like ||= !!node.selectors.find(
160-
(child) => child.type === 'PseudoClassSelector' && child.name === 'root'
161-
);
142+
node.metadata.is_global_like ||=
143+
node.selectors.some(
144+
(child) => child.type === 'PseudoClassSelector' && child.name === 'root'
145+
) &&
146+
// :root.y:has(.x) is not a global selector because while .y is unscoped, .x inside `:has(...)` should be scoped
147+
!node.selectors.some((child) => child.type === 'PseudoClassSelector' && child.name === 'has');
148+
149+
if (node.metadata.is_global_like || node.metadata.is_global) {
150+
// So that nested selectors like `:root:not(.x)` are not marked as unused
151+
for (const child of node.selectors) {
152+
walk(/** @type {Css.Node} */ (child), null, {
153+
ComplexSelector(node) {
154+
node.metadata.used = true;
155+
}
156+
});
157+
}
158+
}
162159

163160
context.next();
164161
},

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

Lines changed: 48 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/** @import { Visitors } from 'zimmerframe' */
22
/** @import * as Compiler from '#compiler' */
33
import { walk } from 'zimmerframe';
4-
import { get_possible_values } from './utils.js';
4+
import { get_parent_rules, get_possible_values, is_outer_global } from './utils.js';
55
import { regex_ends_with_whitespace, regex_starts_with_whitespace } from '../../patterns.js';
66
import { get_attribute_chunks, is_text_attribute } from '../../../utils/ast.js';
77

@@ -172,7 +172,7 @@ function get_relative_selectors(node) {
172172
}
173173

174174
/**
175-
* Discard trailing `:global(...)` selectors without a `:has/is/where/not(...)` modifier, these are unused for scoping purposes
175+
* Discard trailing `:global(...)` selectors, these are unused for scoping purposes
176176
* @param {Compiler.Css.ComplexSelector} node
177177
*/
178178
function truncate(node) {
@@ -182,21 +182,22 @@ function truncate(node) {
182182
// not after a :global selector
183183
!metadata.is_global_like &&
184184
!(first.type === 'PseudoClassSelector' && first.name === 'global' && first.args === null) &&
185-
// not a :global(...) without a :has/is/where/not(...) modifier
186-
(!metadata.is_global ||
187-
selectors.some(
188-
(selector) =>
189-
selector.type === 'PseudoClassSelector' &&
190-
selector.args !== null &&
191-
(selector.name === 'has' ||
192-
selector.name === 'is' ||
193-
selector.name === 'where' ||
194-
selector.name === 'not')
195-
))
185+
// not a :global(...) without a :has/is/where(...) modifier that is scoped
186+
!metadata.is_global
196187
);
197188
});
198189

199-
return node.children.slice(0, i + 1);
190+
return node.children.slice(0, i + 1).map((child) => {
191+
// In case of `:root.y:has(...)`, `y` is unscoped, but everything in `:has(...)` should be scoped (if not global).
192+
// To properly accomplish that, we gotta filter out all selector types except `:has`.
193+
const root = child.selectors.find((s) => s.type === 'PseudoClassSelector' && s.name === 'root');
194+
if (!root || child.metadata.is_global_like) return child;
195+
196+
return {
197+
...child,
198+
selectors: child.selectors.filter((s) => s.type === 'PseudoClassSelector' && s.name === 'has')
199+
};
200+
});
200201
}
201202

202203
/**
@@ -334,7 +335,9 @@ function apply_combinator(combinator, relative_selector, parent_selectors, rule,
334335
* @param {Compiler.AST.RegularElement | Compiler.AST.SvelteElement} element
335336
*/
336337
function mark(relative_selector, element) {
337-
relative_selector.metadata.scoped = true;
338+
if (!is_outer_global(relative_selector)) {
339+
relative_selector.metadata.scoped = true;
340+
}
338341
element.metadata.scoped = true;
339342
}
340343

@@ -415,6 +418,21 @@ function relative_selector_might_apply_to_node(relative_selector, rule, element,
415418
/** @type {Array<Compiler.AST.RegularElement | Compiler.AST.SvelteElement>} */
416419
let sibling_elements; // do them lazy because it's rarely used and expensive to calculate
417420

421+
// If this is a :has inside a global selector, we gotta include the element itself, too,
422+
// because the global selector might be for an element that's outside the component (e.g. :root).
423+
const rules = [rule, ...get_parent_rules(rule)];
424+
const include_self =
425+
rules.some((r) => r.prelude.children.some((c) => c.children.some((s) => is_global(s, r)))) ||
426+
rules[rules.length - 1].prelude.children.some((c) =>
427+
c.children.some((r) =>
428+
r.selectors.some((s) => s.type === 'PseudoClassSelector' && s.name === 'root')
429+
)
430+
);
431+
if (include_self) {
432+
child_elements.push(element);
433+
descendant_elements.push(element);
434+
}
435+
418436
walk(
419437
/** @type {Compiler.SvelteNode} */ (element.fragment),
420438
{ is_child: true },
@@ -460,7 +478,7 @@ function relative_selector_might_apply_to_node(relative_selector, rule, element,
460478

461479
const descendants =
462480
left_most_combinator.name === '+' || left_most_combinator.name === '~'
463-
? (sibling_elements ??= get_following_sibling_elements(element))
481+
? (sibling_elements ??= get_following_sibling_elements(element, include_self))
464482
: left_most_combinator.name === '>'
465483
? child_elements
466484
: descendant_elements;
@@ -481,20 +499,6 @@ function relative_selector_might_apply_to_node(relative_selector, rule, element,
481499
}
482500

483501
if (!matched) {
484-
if (relative_selector.metadata.is_global && !relative_selector.metadata.is_global_like) {
485-
// Edge case: `:global(.x):has(.y)` where `.x` is global but `.y` doesn't match.
486-
// Since `used` is set to `true` for `:global(.x)` in css-analyze beforehand, and
487-
// we have no way of knowing if it's safe to set it back to `false`, we'll mark
488-
// the inner selector as used and scoped to prevent it from being pruned, which could
489-
// result in a invalid CSS output (e.g. `.x:has(/* unused .y */)`). The result
490-
// can't match a real element, so the only drawback is the missing prune.
491-
// TODO clean this up some day
492-
complex_selectors[0].metadata.used = true;
493-
complex_selectors[0].children.forEach((selector) => {
494-
selector.metadata.scoped = true;
495-
});
496-
}
497-
498502
return false;
499503
}
500504
}
@@ -507,9 +511,7 @@ function relative_selector_might_apply_to_node(relative_selector, rule, element,
507511

508512
switch (selector.type) {
509513
case 'PseudoClassSelector': {
510-
if (name === 'host' || name === 'root') {
511-
return false;
512-
}
514+
if (name === 'host' || name === 'root') return false;
513515

514516
if (
515517
name === 'global' &&
@@ -578,23 +580,6 @@ function relative_selector_might_apply_to_node(relative_selector, rule, element,
578580
}
579581

580582
if (!matched) {
581-
if (
582-
relative_selector.metadata.is_global &&
583-
!relative_selector.metadata.is_global_like
584-
) {
585-
// Edge case: `:global(.x):is(.y)` where `.x` is global but `.y` doesn't match.
586-
// Since `used` is set to `true` for `:global(.x)` in css-analyze beforehand, and
587-
// we have no way of knowing if it's safe to set it back to `false`, we'll mark
588-
// the inner selector as used and scoped to prevent it from being pruned, which could
589-
// result in a invalid CSS output (e.g. `.x:is(/* unused .y */)`). The result
590-
// can't match a real element, so the only drawback is the missing prune.
591-
// TODO clean this up some day
592-
selector.args.children[0].metadata.used = true;
593-
selector.args.children[0].children.forEach((selector) => {
594-
selector.metadata.scoped = true;
595-
});
596-
}
597-
598583
return false;
599584
}
600585
}
@@ -662,7 +647,10 @@ function relative_selector_might_apply_to_node(relative_selector, rule, element,
662647
const parent = /** @type {Compiler.Css.Rule} */ (rule.metadata.parent_rule);
663648

664649
for (const complex_selector of parent.prelude.children) {
665-
if (apply_selector(get_relative_selectors(complex_selector), parent, element, state)) {
650+
if (
651+
apply_selector(get_relative_selectors(complex_selector), parent, element, state) ||
652+
complex_selector.children.every((s) => is_global(s, parent))
653+
) {
666654
complex_selector.metadata.used = true;
667655
matched = true;
668656
}
@@ -681,8 +669,11 @@ function relative_selector_might_apply_to_node(relative_selector, rule, element,
681669
return true;
682670
}
683671

684-
/** @param {Compiler.AST.RegularElement | Compiler.AST.SvelteElement} element */
685-
function get_following_sibling_elements(element) {
672+
/**
673+
* @param {Compiler.AST.RegularElement | Compiler.AST.SvelteElement} element
674+
* @param {boolean} include_self
675+
*/
676+
function get_following_sibling_elements(element, include_self) {
686677
/** @type {Compiler.AST.RegularElement | Compiler.AST.SvelteElement | Compiler.AST.Root | null} */
687678
let parent = get_element_parent(element);
688679

@@ -723,6 +714,10 @@ function get_following_sibling_elements(element) {
723714
}
724715
}
725716

717+
if (include_self) {
718+
sibling_elements.push(element);
719+
}
720+
726721
return sibling_elements;
727722
}
728723

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,12 @@ const visitors = {
2424
}
2525
},
2626
ComplexSelector(node, context) {
27-
if (!node.metadata.used) {
27+
if (
28+
!node.metadata.used &&
29+
// prevent double-marking of `.unused:is(.unused)`
30+
(context.path.at(-2)?.type !== 'PseudoClassSelector' ||
31+
/** @type {Css.ComplexSelector} */ (context.path.at(-4))?.metadata.used)
32+
) {
2833
const content = context.state.stylesheet.content;
2934
const text = content.styles.substring(node.start - content.start, node.end - content.start);
3035
w.css_unused_selector(node, text);

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

Lines changed: 83 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/** @import { AST } from '#compiler' */
1+
/** @import { AST, Css } from '#compiler' */
22
/** @import { Node } from 'estree' */
33
const UNKNOWN = {};
44

@@ -33,3 +33,85 @@ export function get_possible_values(chunk) {
3333
if (values.has(UNKNOWN)) return null;
3434
return values;
3535
}
36+
37+
/**
38+
* Returns all parent rules; root is last
39+
* @param {Css.Rule | null} rule
40+
*/
41+
export function get_parent_rules(rule) {
42+
const parents = [];
43+
44+
let parent = rule?.metadata.parent_rule;
45+
while (parent) {
46+
parents.push(parent);
47+
parent = parent.metadata.parent_rule;
48+
}
49+
50+
return parents;
51+
}
52+
53+
/**
54+
* True if is `:global(...)` or `:global` and no pseudo class that is scoped.
55+
* @param {Css.RelativeSelector} relative_selector
56+
* @returns {relative_selector is Css.RelativeSelector & { selectors: [Css.PseudoClassSelector, ...Array<Css.PseudoClassSelector | Css.PseudoElementSelector>] }}
57+
*/
58+
export function is_global(relative_selector) {
59+
const first = relative_selector.selectors[0];
60+
61+
return (
62+
first.type === 'PseudoClassSelector' &&
63+
first.name === 'global' &&
64+
(first.args === null ||
65+
// Only these two selector types keep the whole selector global, because e.g.
66+
// :global(button).x means that the selector is still scoped because of the .x
67+
relative_selector.selectors.every(
68+
(selector) =>
69+
is_unscoped_pseudo_class(selector) || selector.type === 'PseudoElementSelector'
70+
))
71+
);
72+
}
73+
74+
/**
75+
* `true` if is a pseudo class that cannot be or is not scoped
76+
* @param {Css.SimpleSelector} selector
77+
*/
78+
export function is_unscoped_pseudo_class(selector) {
79+
return (
80+
selector.type === 'PseudoClassSelector' &&
81+
// These make the selector scoped
82+
((selector.name !== 'has' &&
83+
selector.name !== 'is' &&
84+
selector.name !== 'where' &&
85+
// Not is special because we want to scope as specific as possible, but because :not
86+
// inverses the result, we want to leave the unscoped, too. The exception is more than
87+
// one selector in the :not (.e.g :not(.x .y)), then .x and .y should be scoped
88+
(selector.name !== 'not' ||
89+
selector.args === null ||
90+
selector.args.children.every((c) => c.children.length === 1))) ||
91+
// selectors with has/is/where/not can also be global if all their children are global
92+
selector.args === null ||
93+
selector.args.children.every((c) => c.children.every((r) => is_global(r))))
94+
);
95+
}
96+
97+
/**
98+
* True if is `:global(...)` or `:global`, irrespective of whether or not there are any pseudo classes that are scoped.
99+
* Difference to `is_global`: `:global(x):has(y)` is `true` for `is_outer_global` but `false` for `is_global`.
100+
* @param {Css.RelativeSelector} relative_selector
101+
* @returns {relative_selector is Css.RelativeSelector & { selectors: [Css.PseudoClassSelector, ...Array<Css.PseudoClassSelector | Css.PseudoElementSelector>] }}
102+
*/
103+
export function is_outer_global(relative_selector) {
104+
const first = relative_selector.selectors[0];
105+
106+
return (
107+
first.type === 'PseudoClassSelector' &&
108+
first.name === 'global' &&
109+
(first.args === null ||
110+
// Only these two selector types can keep the whole selector global, because e.g.
111+
// :global(button).x means that the selector is still scoped because of the .x
112+
relative_selector.selectors.every(
113+
(selector) =>
114+
selector.type === 'PseudoClassSelector' || selector.type === 'PseudoElementSelector'
115+
))
116+
);
117+
}

0 commit comments

Comments
 (0)