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.

Sign up for GitHub

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: correctly visit elements that may match :has() during pruning #15207

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

fix: correctly visit elements that may match :has() during pruning #15207

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d2cf4f6
fix: correctly visit elements that may match :has() during pruning
Feb 4, 2025
7fc1514
format
Feb 4, 2025
68b4715
put type narrowing to the correct place
Feb 4, 2025
63a32c8
drop unused logic
Feb 4, 2025
b41004a
lint
Feb 4, 2025
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/cuddly-turtles-work.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: correctly visit elements that may match :has() during pruning
173 changes: 108 additions & 65 deletions packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import { regex_ends_with_whitespace, regex_starts_with_whitespace } from '../../
import { get_attribute_chunks, is_text_attribute } from '../../../utils/ast.js';

/** @typedef {NODE_PROBABLY_EXISTS | NODE_DEFINITELY_EXISTS} NodeExistsValue */
/** @typedef {Compiler.AST.CSS.BaseNode & { type: 'ElementSelector', element: Compiler.AST.RegularElement | Compiler.AST.SvelteElement }} ElementSelector */
/** @typedef {Omit<Compiler.AST.CSS.RelativeSelector, 'selectors'> & { selectors: Array<Compiler.AST.CSS.SimpleSelector | ElementSelector> }} ExtendedRelativeSelector */

const NODE_PROBABLY_EXISTS = 0;
const NODE_DEFINITELY_EXISTS = 1;
Expand Down Expand Up @@ -156,7 +158,7 @@ function truncate(node) {
}

/**
* @param {Compiler.AST.CSS.RelativeSelector[]} relative_selectors
* @param {ExtendedRelativeSelector[]} relative_selectors
* @param {Compiler.AST.CSS.Rule} rule
* @param {Compiler.AST.RegularElement | Compiler.AST.SvelteElement} element
* @returns {boolean}
Expand All @@ -171,7 +173,7 @@ function apply_selector(relative_selectors, rule, element) {
apply_combinator(relative_selector, parent_selectors, rule, element);

if (matched) {
if (!is_outer_global(relative_selector)) {
if (!is_outer_global(/** @type {Compiler.AST.CSS.RelativeSelector} */ (relative_selector))) {
relative_selector.metadata.scoped = true;
}

Expand All @@ -182,8 +184,8 @@ function apply_selector(relative_selectors, rule, element) {
}

/**
* @param {Compiler.AST.CSS.RelativeSelector} relative_selector
* @param {Compiler.AST.CSS.RelativeSelector[]} parent_selectors
* @param {ExtendedRelativeSelector} relative_selector
* @param {ExtendedRelativeSelector[]} parent_selectors
* @param {Compiler.AST.CSS.Rule} rule
* @param {Compiler.AST.RegularElement | Compiler.AST.SvelteElement | Compiler.AST.RenderTag | Compiler.AST.Component | Compiler.AST.SvelteComponent | Compiler.AST.SvelteSelf} node
* @returns {boolean}
Expand Down Expand Up @@ -234,7 +236,7 @@ function apply_combinator(relative_selector, parent_selectors, rule, node) {

case '+':
case '~': {
const siblings = get_possible_element_siblings(node, name === '+');
const siblings = get_possible_element_preceding_siblings(node, name === '+');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed to clarify that it retreats previous siblings.


let sibling_matched = false;

Expand Down Expand Up @@ -267,7 +269,7 @@ function apply_combinator(relative_selector, parent_selectors, rule, node) {
* it's a `:global(...)` or unscopeable selector, or
* is an `:is(...)` or `:where(...)` selector that contains
* a global selector
* @param {Compiler.AST.CSS.RelativeSelector} selector
* @param {ExtendedRelativeSelector} selector
* @param {Compiler.AST.CSS.Rule} rule
*/
function is_global(selector, rule) {
Expand Down Expand Up @@ -310,7 +312,7 @@ const regex_backslash_and_following_character = /\\(.)/g;
/**
* Ensure that `element` satisfies each simple selector in `relative_selector`
*
* @param {Compiler.AST.CSS.RelativeSelector} relative_selector
* @param {ExtendedRelativeSelector} relative_selector
* @param {Compiler.AST.CSS.Rule} rule
* @param {Compiler.AST.RegularElement | Compiler.AST.SvelteElement} element
* @returns {boolean}
Expand All @@ -331,13 +333,6 @@ function relative_selector_might_apply_to_node(relative_selector, rule, element)
// If we're called recursively from a :has(...) selector, we're on the way of checking if the other selectors match.
// In that case ignore this check (because we just came from this) to avoid an infinite loop.
if (has_selectors.length > 0) {
/** @type {Array<Compiler.AST.RegularElement | Compiler.AST.SvelteElement>} */
const child_elements = [];
/** @type {Array<Compiler.AST.RegularElement | Compiler.AST.SvelteElement>} */
const descendant_elements = [];
/** @type {Array<Compiler.AST.RegularElement | Compiler.AST.SvelteElement>} */
let sibling_elements; // do them lazy because it's rarely used and expensive to calculate

// If this is a :has inside a global selector, we gotta include the element itself, too,
// because the global selector might be for an element that's outside the component,
// e.g. :root:has(.scoped), :global(.foo):has(.scoped), or :root { &:has(.scoped) {} }
Expand All @@ -353,46 +348,35 @@ function relative_selector_might_apply_to_node(relative_selector, rule, element)
)
)
);
if (include_self) {
child_elements.push(element);
descendant_elements.push(element);
}

const seen = new Set();
// set them lazy because it's expensive to calculate
/** @type {Array<Compiler.AST.RegularElement | Compiler.AST.SvelteElement>} */
let descendant_elements;
/** @type {Array<Compiler.AST.RegularElement | Compiler.AST.SvelteElement>} */
let sibling_elements;
/** @type {Array<Compiler.AST.RegularElement | Compiler.AST.SvelteElement>} */
let sibling_descendant_elements;

/**
* @param {Compiler.AST.SvelteNode} node
* @param {{ is_child: boolean }} state
* @param {ExtendedRelativeSelector[]} selectors
*/
function walk_children(node, state) {
walk(node, state, {
_(node, context) {
if (node.type === 'RegularElement' || node.type === 'SvelteElement') {
descendant_elements.push(node);

if (context.state.is_child) {
child_elements.push(node);
context.state.is_child = false;
context.next();
context.state.is_child = true;
} else {
context.next();
}
} else if (node.type === 'RenderTag') {
for (const snippet of node.metadata.snippets) {
if (seen.has(snippet)) continue;
const get_elements = (selectors) => {
const left_most_combinator = selectors[0]?.combinator ?? descendant_combinator;

seen.add(snippet);
walk_children(snippet.body, context.state);
}
} else {
context.next();
}
}
});
}
if (left_most_combinator.name === ' ' || left_most_combinator.name === '>') {
descendant_elements ??= get_descendant_elements(element, include_self);
return descendant_elements;
}

walk_children(element.fragment, { is_child: true });
sibling_elements ??= get_following_sibling_elements(element, include_self);
if (selectors.some((s) => s.combinator?.name === ' ' || s.combinator?.name === '>')) {
Copy link
Contributor Author

@7nik 7nik Feb 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't cover s.combinator === null case - I suppose only the first selector can have no combinator.

sibling_descendant_elements ??= sibling_elements.flatMap((el) =>
get_descendant_elements(el, false)
);
return sibling_descendant_elements;
}
return sibling_elements;
};

// :has(...) is special in that it means "look downwards in the CSS tree". Since our matching algorithm goes
// upwards and back-to-front, we need to first check the selectors inside :has(...), then check the rest of the
Expand All @@ -403,28 +387,19 @@ function relative_selector_might_apply_to_node(relative_selector, rule, element)
let matched = false;

for (const complex_selector of complex_selectors) {
/** @type {ExtendedRelativeSelector[]} */
const selectors = truncate(complex_selector);
const left_most_combinator = selectors[0]?.combinator ?? descendant_combinator;
// In .x:has(> y), we want to search for y, ignoring the left-most combinator
// (else it would try to walk further up and fail because there are no selectors left)
const elements = get_elements(selectors);
// In .x:has(> y), we complete the selector by prepending a special one that
// matches only this `element`, otherwise it can mismatch with an ancestor element
if (selectors.length > 0) {
selectors[0] = {
...selectors[0],
combinator: null
};
selectors.unshift(make_element_selector(element));
}

const descendants =
left_most_combinator.name === '+' || left_most_combinator.name === '~'
? (sibling_elements ??= get_following_sibling_elements(element, include_self))
: left_most_combinator.name === '>'
? child_elements
: descendant_elements;

let selector_matched = false;

// Iterate over all descendant elements and check if the selector inside :has matches
for (const element of descendants) {
for (const element of elements) {
if (
selectors.length === 0 /* is :global(...) */ ||
(element.metadata.scoped && selector_matched) ||
Expand All @@ -445,6 +420,10 @@ function relative_selector_might_apply_to_node(relative_selector, rule, element)
for (const selector of other_selectors) {
if (selector.type === 'Percentage' || selector.type === 'Nth') continue;

if (selector.type === 'ElementSelector') {
return element === selector.element;
}

const name = selector.name.replace(regex_backslash_and_following_character, '$1');

switch (selector.type) {
Expand Down Expand Up @@ -686,6 +665,44 @@ function get_following_sibling_elements(element, include_self) {
return siblings;
}

/**
* @param {Compiler.AST.RegularElement | Compiler.AST.SvelteElement} element
* @param {boolean} include_self
*/
function get_descendant_elements(element, include_self) {
/** @type {Array<Compiler.AST.RegularElement | Compiler.AST.SvelteElement>} */
const descendants = include_self ? [element] : [];
const seen = new Set();

/**
* @param {Compiler.AST.SvelteNode} node
*/
function walk_children(node) {
walk(node, null, {
RegularElement(node, context) {
descendants.push(node);
context.next();
},
SvelteElement(node, context) {
descendants.push(node);
context.next();
},
RenderTag(node) {
for (const snippet of node.metadata.snippets) {
if (seen.has(snippet)) continue;

seen.add(snippet);
walk_children(snippet.body);
}
}
});
}

walk_children(element.fragment);

return descendants;
}

/**
* @param {any} operator
* @param {any} expected_value
Expand Down Expand Up @@ -847,7 +864,7 @@ function get_element_parent(node) {
* @param {Set<Compiler.AST.SnippetBlock>} seen
* @returns {Map<Compiler.AST.RegularElement | Compiler.AST.SvelteElement | Compiler.AST.SlotElement | Compiler.AST.RenderTag, NodeExistsValue>}
*/
function get_possible_element_siblings(node, adjacent_only, seen = new Set()) {
function get_possible_element_preceding_siblings(node, adjacent_only, seen = new Set()) {
/** @type {Map<Compiler.AST.RegularElement | Compiler.AST.SvelteElement | Compiler.AST.SlotElement | Compiler.AST.RenderTag, NodeExistsValue>} */
const result = new Map();
const path = node.metadata.path;
Expand Down Expand Up @@ -910,7 +927,7 @@ function get_possible_element_siblings(node, adjacent_only, seen = new Set()) {
seen.add(current);

for (const site of current.metadata.sites) {
const siblings = get_possible_element_siblings(site, adjacent_only, seen);
const siblings = get_possible_element_preceding_siblings(site, adjacent_only, seen);
add_to_map(siblings, result);

if (adjacent_only && current.metadata.sites.size === 1 && has_definite_elements(siblings)) {
Expand Down Expand Up @@ -1067,3 +1084,29 @@ function is_block(node) {
node.type === 'SlotElement'
);
}

/**
* @param {Compiler.AST.RegularElement | Compiler.AST.SvelteElement} element
* @return {ExtendedRelativeSelector}
*/
function make_element_selector(element) {
return {
type: 'RelativeSelector',
selectors: [
{
type: 'ElementSelector',
element,
start: -1,
end: -1
}
],
combinator: null,
metadata: {
is_global: false,
is_global_like: false,
scoped: false
},
start: -1,
end: -1
};
}
Loading