Skip to content

Commit

Permalink
fix: prevent infinite loops when pruning CSS (#14474)
Browse files Browse the repository at this point in the history
fixes #14472

---------

Co-authored-by: Simon Holthausen <simon.holthausen@vercel.com>
  • Loading branch information
Rich-Harris and dummdidumm authored Nov 29, 2024
1 parent ca3690f commit a60e837
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 3 deletions.
5 changes: 5 additions & 0 deletions .changeset/itchy-timers-relax.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: prevent infinite loops when pruning CSS
29 changes: 27 additions & 2 deletions packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,12 @@ const nesting_selector = {
}
};

/**
* Snippets encountered already (avoids infinite loops)
* @type {Set<Compiler.AST.SnippetBlock>}
*/
const seen = new Set();

/**
*
* @param {Compiler.Css.StyleSheet} stylesheet
Expand All @@ -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)
) {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -335,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
Expand All @@ -355,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 {
Expand Down Expand Up @@ -618,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, {
Expand All @@ -629,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);
}
}
Expand Down Expand Up @@ -804,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<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) {
function get_possible_element_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 @@ -865,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)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
20 changes: 20 additions & 0 deletions packages/svelte/tests/css/samples/render-tag-loop/_config.js
Original file line number Diff line number Diff line change
@@ -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
}
}
]
});
10 changes: 10 additions & 0 deletions packages/svelte/tests/css/samples/render-tag-loop/expected.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@

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;
}
25 changes: 25 additions & 0 deletions packages/svelte/tests/css/samples/render-tag-loop/input.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
{#snippet a()}
{@render b()}
<div>
{@render b()}
</div>
{/snippet}

{#snippet b()}
{@render a()}
<div>
{@render a()}
</div>
{/snippet}

<style>
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;
}
</style>

0 comments on commit a60e837

Please sign in to comment.