From b14b17f9d3f334e75b59f2043819e3ec06b2ff79 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 29 Nov 2024 13:28:13 -0500 Subject: [PATCH 1/3] fix: prevent infinite loops when pruning CSS --- .changeset/itchy-timers-relax.md | 5 +++++ .../compiler/phases/2-analyze/css/css-prune.js | 11 +++++++++++ .../2-analyze/visitors/shared/component.js | 2 +- .../css/samples/render-tag-loop/expected.css | 4 ++++ .../css/samples/render-tag-loop/input.svelte | 17 +++++++++++++++++ 5 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 .changeset/itchy-timers-relax.md create mode 100644 packages/svelte/tests/css/samples/render-tag-loop/expected.css create mode 100644 packages/svelte/tests/css/samples/render-tag-loop/input.svelte diff --git a/.changeset/itchy-timers-relax.md b/.changeset/itchy-timers-relax.md new file mode 100644 index 000000000000..5d0913ba62de --- /dev/null +++ b/.changeset/itchy-timers-relax.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: prevent infinite loops when pruning CSS 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 0c38114a306e..013b93afa4bc 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 @@ -43,6 +43,12 @@ const nesting_selector = { } }; +/** + * Snippets encountered already (avoids infinite loops) + * @type {Set} + */ +const seen = new Set(); + /** * * @param {Compiler.Css.StyleSheet} stylesheet @@ -60,6 +66,8 @@ export function prune(stylesheet, element) { ComplexSelector(node) { const selectors = get_relative_selectors(node); + seen.clear(); + if ( apply_selector(selectors, /** @type {Compiler.Css.Rule} */ (node.metadata.rule), element) ) { @@ -193,6 +201,9 @@ function apply_combinator(relative_selector, parent_selectors, rule, node) { const parent = path[i]; if (parent.type === 'SnippetBlock') { + if (seen.has(parent)) return true; + seen.add(parent); + for (const site of parent.metadata.sites) { if (apply_combinator(relative_selector, parent_selectors, rule, site)) { return true; diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/component.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/component.js index 84a7e2f1303f..04bf3d2ff3bf 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/component.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/component.js @@ -51,7 +51,7 @@ export function visit_component(node, context) { if (binding?.initial?.type === 'SnippetBlock') { node.metadata.snippets.add(binding.initial); } - } else { + } else if (expression.type !== 'Literal') { resolved = false; } } diff --git a/packages/svelte/tests/css/samples/render-tag-loop/expected.css b/packages/svelte/tests/css/samples/render-tag-loop/expected.css new file mode 100644 index 000000000000..e463c0b689aa --- /dev/null +++ b/packages/svelte/tests/css/samples/render-tag-loop/expected.css @@ -0,0 +1,4 @@ + + div.svelte-xyz div:where(.svelte-xyz) { + color: red; + } diff --git a/packages/svelte/tests/css/samples/render-tag-loop/input.svelte b/packages/svelte/tests/css/samples/render-tag-loop/input.svelte new file mode 100644 index 000000000000..bb7a47f6b6fa --- /dev/null +++ b/packages/svelte/tests/css/samples/render-tag-loop/input.svelte @@ -0,0 +1,17 @@ +{#snippet a()} +
+ {@render b()} +
+{/snippet} + +{#snippet b()} +
+ {@render a()} +
+{/snippet} + + From 6a4d1bb9468af340817b1c2b3a3ce1152b994d5c Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 29 Nov 2024 21:44:56 +0100 Subject: [PATCH 2/3] this one aswell --- .../src/compiler/phases/2-analyze/css/css-prune.js | 10 ++++++++++ .../tests/css/samples/render-tag-loop/expected.css | 5 ++++- .../tests/css/samples/render-tag-loop/input.svelte | 5 ++++- 3 files changed, 18 insertions(+), 2 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 013b93afa4bc..f05cd7e4ae3d 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 @@ -346,6 +346,8 @@ function relative_selector_might_apply_to_node(relative_selector, rule, element) descendant_elements.push(element); } + const seen = new Set(); + /** * @param {Compiler.SvelteNode} node * @param {{ is_child: boolean }} state @@ -366,6 +368,9 @@ function relative_selector_might_apply_to_node(relative_selector, rule, element) } } 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 { @@ -629,6 +634,8 @@ function get_following_sibling_elements(element, include_self) { // ...then walk them, starting from the node after the one // containing the element in question + const seen = new Set(); + /** @param {Compiler.SvelteNode} node */ function get_siblings(node) { walk(node, null, { @@ -640,6 +647,9 @@ function get_following_sibling_elements(element, include_self) { }, RenderTag(node) { for (const snippet of node.metadata.snippets) { + if (seen.has(snippet)) continue; + + seen.add(snippet); get_siblings(snippet.body); } } diff --git a/packages/svelte/tests/css/samples/render-tag-loop/expected.css b/packages/svelte/tests/css/samples/render-tag-loop/expected.css index e463c0b689aa..580c1f05fbbc 100644 --- a/packages/svelte/tests/css/samples/render-tag-loop/expected.css +++ b/packages/svelte/tests/css/samples/render-tag-loop/expected.css @@ -1,4 +1,7 @@ div.svelte-xyz div:where(.svelte-xyz) { - color: red; + color: green; + } + div.svelte-xyz:has(div:where(.svelte-xyz)) { + color: green; } diff --git a/packages/svelte/tests/css/samples/render-tag-loop/input.svelte b/packages/svelte/tests/css/samples/render-tag-loop/input.svelte index bb7a47f6b6fa..192eb3d2f127 100644 --- a/packages/svelte/tests/css/samples/render-tag-loop/input.svelte +++ b/packages/svelte/tests/css/samples/render-tag-loop/input.svelte @@ -12,6 +12,9 @@ From a1330c88119211148d3258f010c64a2716d5239a Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 29 Nov 2024 22:12:40 +0100 Subject: [PATCH 3/3] another one --- .../phases/2-analyze/css/css-prune.js | 8 ++++++-- .../css/samples/render-tag-loop/_config.js | 20 +++++++++++++++++++ .../css/samples/render-tag-loop/expected.css | 5 ++++- .../css/samples/render-tag-loop/input.svelte | 5 +++++ 4 files changed, 35 insertions(+), 3 deletions(-) create mode 100644 packages/svelte/tests/css/samples/render-tag-loop/_config.js 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 f05cd7e4ae3d..83c50c73d029 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 @@ -825,9 +825,10 @@ function get_element_parent(node) { /** * @param {Compiler.AST.RegularElement | Compiler.AST.SvelteElement | Compiler.AST.RenderTag | Compiler.AST.Component | Compiler.AST.SvelteComponent | Compiler.AST.SvelteSelf} node * @param {boolean} adjacent_only + * @param {Set} seen * @returns {Map} */ -function get_possible_element_siblings(node, adjacent_only) { +function get_possible_element_siblings(node, adjacent_only, seen = new Set()) { /** @type {Map} */ const result = new Map(); const path = node.metadata.path; @@ -886,8 +887,11 @@ function get_possible_element_siblings(node, adjacent_only) { } if (current.type === 'SnippetBlock') { + if (seen.has(current)) break; + seen.add(current); + for (const site of current.metadata.sites) { - const siblings = get_possible_element_siblings(site, adjacent_only); + const siblings = get_possible_element_siblings(site, adjacent_only, seen); add_to_map(siblings, result); if (adjacent_only && current.metadata.sites.size === 1 && has_definite_elements(siblings)) { diff --git a/packages/svelte/tests/css/samples/render-tag-loop/_config.js b/packages/svelte/tests/css/samples/render-tag-loop/_config.js new file mode 100644 index 000000000000..f623b92cc38b --- /dev/null +++ b/packages/svelte/tests/css/samples/render-tag-loop/_config.js @@ -0,0 +1,20 @@ +import { test } from '../../test'; + +export default test({ + warnings: [ + { + code: 'css_unused_selector', + message: 'Unused CSS selector "div + div"', + start: { + line: 19, + column: 1, + character: 185 + }, + end: { + line: 19, + column: 10, + character: 194 + } + } + ] +}); diff --git a/packages/svelte/tests/css/samples/render-tag-loop/expected.css b/packages/svelte/tests/css/samples/render-tag-loop/expected.css index 580c1f05fbbc..1f05ddb41cc7 100644 --- a/packages/svelte/tests/css/samples/render-tag-loop/expected.css +++ b/packages/svelte/tests/css/samples/render-tag-loop/expected.css @@ -1,7 +1,10 @@ - div.svelte-xyz div:where(.svelte-xyz) { + div div.svelte-xyz { color: green; } + /* (unused) div + div { + color: red; /* this is marked as unused, but only because we've written an infinite loop - worth fixing? *\/ + }*/ div.svelte-xyz:has(div:where(.svelte-xyz)) { color: green; } diff --git a/packages/svelte/tests/css/samples/render-tag-loop/input.svelte b/packages/svelte/tests/css/samples/render-tag-loop/input.svelte index 192eb3d2f127..ade8df574489 100644 --- a/packages/svelte/tests/css/samples/render-tag-loop/input.svelte +++ b/packages/svelte/tests/css/samples/render-tag-loop/input.svelte @@ -1,10 +1,12 @@ {#snippet a()} + {@render b()}
{@render b()}
{/snippet} {#snippet b()} + {@render a()}
{@render a()}
@@ -14,6 +16,9 @@ div div { color: green; } + div + div { + color: red; /* this is marked as unused, but only because we've written an infinite loop - worth fixing? */ + } div:has(div) { color: green; }