From d2cf4f6e4b9594363cb544d2304cc5e797aa57bc Mon Sep 17 00:00:00 2001 From: 7nik Date: Tue, 4 Feb 2025 16:31:24 +0200 Subject: [PATCH 1/5] fix: correctly visit elements that may match :has() during pruning --- .changeset/cuddly-turtles-work.md | 5 + .../phases/2-analyze/css/css-prune.js | 168 +++++++++++------- .../svelte/tests/css/samples/has/_config.js | 134 +++++++------- .../svelte/tests/css/samples/has/expected.css | 10 ++ .../svelte/tests/css/samples/has/input.svelte | 18 ++ 5 files changed, 214 insertions(+), 121 deletions(-) create mode 100644 .changeset/cuddly-turtles-work.md diff --git a/.changeset/cuddly-turtles-work.md b/.changeset/cuddly-turtles-work.md new file mode 100644 index 000000000000..1f17659a395e --- /dev/null +++ b/.changeset/cuddly-turtles-work.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: correctly visit elements that may match :has() during pruning diff --git a/packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js b/packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js index fc8108e46e8e..816ad17d1d5f 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js +++ b/packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js @@ -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 & { selectors: Array }} ExtendedRelativeSelector */ const NODE_PROBABLY_EXISTS = 0; const NODE_DEFINITELY_EXISTS = 1; @@ -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 === '+'); let sibling_matched = false; @@ -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} @@ -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} */ - const child_elements = []; - /** @type {Array} */ - const descendant_elements = []; - /** @type {Array} */ - 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) {} } @@ -353,46 +348,33 @@ 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} */ + let descendant_elements; + /** @type {Array} */ + let sibling_elements; + /** @type {Array} */ + 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 === '>')) { + 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 @@ -403,32 +385,23 @@ 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) || - apply_selector(selectors, rule, element) + apply_selector(/** @type {Compiler.AST.CSS.RelativeSelector[]} */ (selectors), rule, element) ) { complex_selector.metadata.used = true; selector_matched = matched = true; @@ -445,6 +418,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) { @@ -686,6 +663,51 @@ 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} */ + const descendants = include_self ? [element] : []; + const seen = new Set(); + + /** + * @param {Compiler.AST.SvelteNode} node + * @param {{ is_child: boolean }} state + */ + function walk_children(node, state) { + walk(node, state, { + _(node, context) { + if (node.type === 'RegularElement' || node.type === 'SvelteElement') { + descendants.push(node); + + if (context.state.is_child) { + 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; + + seen.add(snippet); + walk_children(snippet.body, context.state); + } + } else { + context.next(); + } + } + }); + } + + walk_children(element.fragment, { is_child: true }); + + return descendants; +} + /** * @param {any} operator * @param {any} expected_value @@ -847,7 +869,7 @@ function get_element_parent(node) { * @param {Set} seen * @returns {Map} */ -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} */ const result = new Map(); const path = node.metadata.path; @@ -910,7 +932,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)) { @@ -1067,3 +1089,27 @@ 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, + }; +} \ No newline at end of file diff --git a/packages/svelte/tests/css/samples/has/_config.js b/packages/svelte/tests/css/samples/has/_config.js index 8d89d98cbdb0..df9a45ac6cfd 100644 --- a/packages/svelte/tests/css/samples/has/_config.js +++ b/packages/svelte/tests/css/samples/has/_config.js @@ -6,210 +6,224 @@ export default test({ code: 'css_unused_selector', message: 'Unused CSS selector ".unused:has(y)"', start: { - line: 33, + line: 41, column: 1, - character: 330 + character: 378 }, end: { - line: 33, + line: 41, column: 15, - character: 344 + character: 392 } }, { code: 'css_unused_selector', message: 'Unused CSS selector ".unused:has(:global(y))"', start: { - line: 36, + line: 44, column: 1, - character: 365 + character: 413 }, end: { - line: 36, + line: 44, column: 24, - character: 388 + character: 436 } }, { code: 'css_unused_selector', message: 'Unused CSS selector "x:has(.unused)"', start: { - line: 39, + line: 47, column: 1, - character: 409 + character: 457 }, end: { - line: 39, + line: 47, column: 15, - character: 423 + character: 471 } }, { code: 'css_unused_selector', message: 'Unused CSS selector ":global(.foo):has(.unused)"', start: { - line: 42, + line: 50, column: 1, - character: 444 + character: 492 }, end: { - line: 42, + line: 50, column: 27, - character: 470 + character: 518 } }, { code: 'css_unused_selector', message: 'Unused CSS selector "x:has(y):has(.unused)"', start: { - line: 52, + line: 60, column: 1, - character: 578 + character: 626 }, end: { - line: 52, + line: 60, column: 22, - character: 599 + character: 647 } }, { code: 'css_unused_selector', message: 'Unused CSS selector ".unused"', start: { - line: 71, + line: 79, column: 2, - character: 804 + character: 852 }, end: { - line: 71, + line: 79, column: 9, - character: 811 + character: 859 } }, { code: 'css_unused_selector', message: 'Unused CSS selector ".unused x:has(y)"', start: { - line: 87, + line: 95, column: 1, - character: 958 + character: 1006 }, end: { - line: 87, + line: 95, column: 17, - character: 974 + character: 1022 } }, { code: 'css_unused_selector', message: 'Unused CSS selector ".unused:has(.unused)"', start: { - line: 90, + line: 98, column: 1, - character: 995 + character: 1043 }, end: { - line: 90, + line: 98, column: 21, - character: 1015 + character: 1063 } }, { code: 'css_unused_selector', message: 'Unused CSS selector "x:has(> z)"', start: { - line: 100, + line: 108, column: 1, - character: 1115 + character: 1163 }, end: { - line: 100, + line: 108, column: 11, - character: 1125 + character: 1173 } }, { code: 'css_unused_selector', message: 'Unused CSS selector "x:has(> d)"', start: { - line: 103, + line: 111, column: 1, - character: 1146 + character: 1194 }, end: { - line: 103, + line: 111, column: 11, - character: 1156 + character: 1204 } }, { code: 'css_unused_selector', message: 'Unused CSS selector "x:has(~ y)"', start: { - line: 123, + line: 131, column: 1, - character: 1348 + character: 1396 }, end: { - line: 123, + line: 131, column: 11, - character: 1358 + character: 1406 } }, { code: 'css_unused_selector', message: 'Unused CSS selector "f:has(~ d)"', start: { - line: 133, + line: 141, column: 1, - character: 1446 + character: 1494 }, end: { - line: 133, + line: 141, column: 11, - character: 1456 + character: 1504 } }, { code: 'css_unused_selector', message: 'Unused CSS selector ":has(.unused)"', start: { - line: 141, + line: 149, column: 2, - character: 1529 + character: 1577 }, end: { - line: 141, + line: 149, column: 15, - character: 1542 + character: 1590 } }, { code: 'css_unused_selector', message: 'Unused CSS selector "&:has(.unused)"', start: { - line: 147, + line: 155, column: 2, - character: 1600 + character: 1648 }, end: { - line: 147, + line: 155, column: 16, - character: 1614 + character: 1662 } }, { code: 'css_unused_selector', message: 'Unused CSS selector ":global(.foo):has(.unused)"', start: { - line: 155, + line: 163, column: 1, - character: 1684 + character: 1732 }, end: { - line: 155, + line: 163, column: 27, - character: 1710 + character: 1758 + } + }, + { + code: 'css_unused_selector', + message: 'Unused CSS selector "h:has(> h > i)"', + start: { + line: 170, + column: 1, + character: 1817 + }, + end: { + line: 170, + column: 15, + character: 1831 } } ] diff --git a/packages/svelte/tests/css/samples/has/expected.css b/packages/svelte/tests/css/samples/has/expected.css index b257370d61f3..c0b884e00c3e 100644 --- a/packages/svelte/tests/css/samples/has/expected.css +++ b/packages/svelte/tests/css/samples/has/expected.css @@ -143,3 +143,13 @@ /* (unused) :global(.foo):has(.unused) { color: red; }*/ + + g.svelte-xyz:has(> h:where(.svelte-xyz) > i:where(.svelte-xyz)) { + color: green; + } + /* (unused) h:has(> h > i) { + color: red; + }*/ + g.svelte-xyz:has(+ j:where(.svelte-xyz) > k:where(.svelte-xyz)) { + color: green; + } diff --git a/packages/svelte/tests/css/samples/has/input.svelte b/packages/svelte/tests/css/samples/has/input.svelte index 9b254996bf30..53372c3f0ef1 100644 --- a/packages/svelte/tests/css/samples/has/input.svelte +++ b/packages/svelte/tests/css/samples/has/input.svelte @@ -9,6 +9,14 @@ + + + + + + + + From 7fc15149c5e9a37f669047ca1a16e1e2ad318971 Mon Sep 17 00:00:00 2001 From: 7nik Date: Tue, 4 Feb 2025 16:32:56 +0200 Subject: [PATCH 2/5] format --- .../phases/2-analyze/css/css-prune.js | 34 ++++++++++++------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js b/packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js index 816ad17d1d5f..6d76928d936b 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js +++ b/packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js @@ -369,12 +369,14 @@ function relative_selector_might_apply_to_node(relative_selector, rule, element) } sibling_elements ??= get_following_sibling_elements(element, include_self); - if (selectors.some(s => s.combinator?.name === ' ' || s.combinator?.name === '>')) { - sibling_descendant_elements ??= sibling_elements.flatMap(el => get_descendant_elements(el, false)); + if (selectors.some((s) => s.combinator?.name === ' ' || s.combinator?.name === '>')) { + 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 @@ -401,7 +403,11 @@ function relative_selector_might_apply_to_node(relative_selector, rule, element) if ( selectors.length === 0 /* is :global(...) */ || (element.metadata.scoped && selector_matched) || - apply_selector(/** @type {Compiler.AST.CSS.RelativeSelector[]} */ (selectors), rule, element) + apply_selector( + /** @type {Compiler.AST.CSS.RelativeSelector[]} */ (selectors), + rule, + element + ) ) { complex_selector.metadata.used = true; selector_matched = matched = true; @@ -1097,19 +1103,21 @@ function is_block(node) { function make_element_selector(element) { return { type: 'RelativeSelector', - selectors: [{ - type: 'ElementSelector', - element, - start: -1, - end: -1, - }], + selectors: [ + { + type: 'ElementSelector', + element, + start: -1, + end: -1 + } + ], combinator: null, metadata: { is_global: false, is_global_like: false, - scoped: false, + scoped: false }, start: -1, - end: -1, + end: -1 }; -} \ No newline at end of file +} From 68b4715e85f0c8dde48ea410fb20a2d133d5aeb4 Mon Sep 17 00:00:00 2001 From: 7nik Date: Tue, 4 Feb 2025 16:56:28 +0200 Subject: [PATCH 3/5] put type narrowing to the correct place --- .../compiler/phases/2-analyze/css/css-prune.js | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js b/packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js index 6d76928d936b..d8bdbe748738 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js +++ b/packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js @@ -158,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} @@ -173,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; } @@ -184,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} @@ -269,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) { @@ -403,11 +403,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( - /** @type {Compiler.AST.CSS.RelativeSelector[]} */ (selectors), - rule, - element - ) + apply_selector(selectors, rule, element) ) { complex_selector.metadata.used = true; selector_matched = matched = true; From 63a32c8aebc617c66afc8bdc36d1392b6da1e216 Mon Sep 17 00:00:00 2001 From: 7nik Date: Tue, 4 Feb 2025 18:28:38 +0200 Subject: [PATCH 4/5] drop unused logic --- .../phases/2-analyze/css/css-prune.js | 41 ++++++++----------- 1 file changed, 17 insertions(+), 24 deletions(-) diff --git a/packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js b/packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js index d8bdbe748738..b725fd6698a4 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js +++ b/packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js @@ -676,36 +676,29 @@ function get_descendant_elements(element, include_self) { /** * @param {Compiler.AST.SvelteNode} node - * @param {{ is_child: boolean }} state */ - function walk_children(node, state) { - walk(node, state, { - _(node, context) { - if (node.type === 'RegularElement' || node.type === 'SvelteElement') { - descendants.push(node); - - if (context.state.is_child) { - 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; + 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, context.state); - } - } else { - context.next(); + seen.add(snippet); + walk_children(snippet.body); } - } + }, }); } - walk_children(element.fragment, { is_child: true }); + walk_children(element.fragment); return descendants; } From b41004a7a375fb02b78491348d4e16ef580f2422 Mon Sep 17 00:00:00 2001 From: 7nik Date: Tue, 4 Feb 2025 18:34:06 +0200 Subject: [PATCH 5/5] lint --- packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js b/packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js index b725fd6698a4..dd4b5f85161a 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js +++ b/packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js @@ -694,7 +694,7 @@ function get_descendant_elements(element, include_self) { seen.add(snippet); walk_children(snippet.body); } - }, + } }); }