fix: correctly prune key/each blocks (#14403)

* fix: correctly prune key blocks * fix pruning of each blocks * simplify * make more explicit * changeset * helperise/robustify
sveltejs · Nov 22, 2024 · 37e6c7f · 37e6c7f
1 parent e721d96
commit 37e6c7f
Show file tree

Hide file tree

Showing 13 changed files with 156 additions and 128 deletions.
diff --git a/.changeset/fuzzy-lies-battle.md b/.changeset/fuzzy-lies-battle.md
@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: correctly prune each blocks
diff --git a/.changeset/wicked-buses-work.md b/.changeset/wicked-buses-work.md
@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: correctly prune key blocks
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
@@ -955,7 +955,7 @@ function get_possible_element_siblings(node, adjacent_only) {
 			if (adjacent_only) {
 				break;
 			}
-		} else if (prev.type === 'EachBlock' || prev.type === 'IfBlock' || prev.type === 'AwaitBlock') {
+		} else if (is_block(prev)) {
 			const possible_last_child = get_possible_last_child(prev, adjacent_only);
 			add_to_map(possible_last_child, result);
 			if (adjacent_only && has_definite_elements(possible_last_child)) {
@@ -979,7 +979,7 @@ function get_possible_element_siblings(node, adjacent_only) {
 		while (
 			// @ts-expect-error TODO
 			(parent = parent?.parent) &&
-			(parent.type === 'EachBlock' || parent.type === 'IfBlock' || parent.type === 'AwaitBlock')
+			is_block(parent)
 		) {
 			const possible_siblings = get_possible_element_siblings(parent, adjacent_only);
 			add_to_map(possible_siblings, result);
@@ -1000,73 +1000,57 @@ function get_possible_element_siblings(node, adjacent_only) {
 }
 
 /**
- * @param {Compiler.AST.EachBlock | Compiler.AST.IfBlock | Compiler.AST.AwaitBlock} relative_selector
+ * @param {Compiler.AST.EachBlock | Compiler.AST.IfBlock | Compiler.AST.AwaitBlock | Compiler.AST.KeyBlock} node
  * @param {boolean} adjacent_only
  * @returns {Map<Compiler.AST.RegularElement, NodeExistsValue>}
  */
-function get_possible_last_child(relative_selector, adjacent_only) {
+function get_possible_last_child(node, adjacent_only) {
 	/** @typedef {Map<Compiler.AST.RegularElement, NodeExistsValue>} NodeMap */
 
+	/** @type {Array<Compiler.AST.Fragment | undefined | null>} */
+	let fragments = [];
+
+	switch (node.type) {
+		case 'EachBlock':
+			fragments.push(node.body, node.fallback);
+			break;
+
+		case 'IfBlock':
+			fragments.push(node.consequent, node.alternate);
+			break;
+
+		case 'AwaitBlock':
+			fragments.push(node.pending, node.then, node.catch);
+			break;
+
+		case 'KeyBlock':
+			fragments.push(node.fragment);
+			break;
+	}
+
 	/** @type {NodeMap} */
 	const result = new Map();
-	if (relative_selector.type === 'EachBlock') {
-		/** @type {NodeMap} */
-		const each_result = loop_child(relative_selector.body.nodes, adjacent_only);
-
-		/** @type {NodeMap} */
-		const else_result = relative_selector.fallback
-			? loop_child(relative_selector.fallback.nodes, adjacent_only)
-			: new Map();
-		const not_exhaustive = !has_definite_elements(else_result);
-		if (not_exhaustive) {
-			mark_as_probably(each_result);
-			mark_as_probably(else_result);
-		}
-		add_to_map(each_result, result);
-		add_to_map(else_result, result);
-	} else if (relative_selector.type === 'IfBlock') {
-		/** @type {NodeMap} */
-		const if_result = loop_child(relative_selector.consequent.nodes, adjacent_only);
-
-		/** @type {NodeMap} */
-		const else_result = relative_selector.alternate
-			? loop_child(relative_selector.alternate.nodes, adjacent_only)
-			: new Map();
-		const not_exhaustive = !has_definite_elements(if_result) || !has_definite_elements(else_result);
-		if (not_exhaustive) {
-			mark_as_probably(if_result);
-			mark_as_probably(else_result);
+
+	let exhaustive = true;
+
+	for (const fragment of fragments) {
+		if (fragment == null) {
+			exhaustive = false;
+			continue;
 		}
-		add_to_map(if_result, result);
-		add_to_map(else_result, result);
-	} else if (relative_selector.type === 'AwaitBlock') {
-		/** @type {NodeMap} */
-		const pending_result = relative_selector.pending
-			? loop_child(relative_selector.pending.nodes, adjacent_only)
-			: new Map();
-
-		/** @type {NodeMap} */
-		const then_result = relative_selector.then
-			? loop_child(relative_selector.then.nodes, adjacent_only)
-			: new Map();
-
-		/** @type {NodeMap} */
-		const catch_result = relative_selector.catch
-			? loop_child(relative_selector.catch.nodes, adjacent_only)
-			: new Map();
-		const not_exhaustive =
-			!has_definite_elements(pending_result) ||
-			!has_definite_elements(then_result) ||
-			!has_definite_elements(catch_result);
-		if (not_exhaustive) {
-			mark_as_probably(pending_result);
-			mark_as_probably(then_result);
-			mark_as_probably(catch_result);
+
+		const map = loop_child(fragment.nodes, adjacent_only);
+		exhaustive &&= has_definite_elements(map);
+
+		add_to_map(map, result);
+	}
+
+	if (!exhaustive) {
+		for (const key of result.keys()) {
+			result.set(key, NODE_PROBABLY_EXISTS);
 		}
-		add_to_map(pending_result, result);
-		add_to_map(then_result, result);
-		add_to_map(catch_result, result);
 	}
+
 	return result;
 }
 
@@ -1107,13 +1091,6 @@ function higher_existence(exist1, exist2) {
 	return exist1 > exist2 ? exist1 : exist2;
 }
 
-/** @param {Map<Compiler.AST.RegularElement, NodeExistsValue>} result */
-function mark_as_probably(result) {
-	for (const key of result.keys()) {
-		result.set(key, NODE_PROBABLY_EXISTS);
-	}
-}
-
 /**
  * @param {Compiler.SvelteNode[]} children
  * @param {boolean} adjacent_only
@@ -1132,11 +1109,7 @@ function loop_child(children, adjacent_only) {
 			if (adjacent_only) {
 				break;
 			}
-		} else if (
-			child.type === 'EachBlock' ||
-			child.type === 'IfBlock' ||
-			child.type === 'AwaitBlock'
-		) {
+		} else if (is_block(child)) {
 			const child_result = get_possible_last_child(child, adjacent_only);
 			add_to_map(child_result, result);
 			if (adjacent_only && has_definite_elements(child_result)) {
@@ -1147,3 +1120,16 @@ function loop_child(children, adjacent_only) {
 
 	return result;
 }
+
+/**
+ * @param {Compiler.SvelteNode} node
+ * @returns {node is Compiler.AST.IfBlock | Compiler.AST.EachBlock | Compiler.AST.AwaitBlock | Compiler.AST.KeyBlock}
+ */
+function is_block(node) {
+	return (
+		node.type === 'IfBlock' ||
+		node.type === 'EachBlock' ||
+		node.type === 'AwaitBlock' ||
+		node.type === 'KeyBlock'
+	);
+}
diff --git a/packages/svelte/tests/css/samples/general-siblings-combinator-key/_config.js b/packages/svelte/tests/css/samples/general-siblings-combinator-key/_config.js
@@ -0,0 +1,5 @@
+import { test } from '../../test';
+
+export default test({
+	warnings: []
+});
diff --git a/packages/svelte/tests/css/samples/general-siblings-combinator-key/expected.css b/packages/svelte/tests/css/samples/general-siblings-combinator-key/expected.css
@@ -0,0 +1,4 @@
+
+	.a.svelte-xyz ~ .b:where(.svelte-xyz) { color: green; }
+	.a.svelte-xyz ~ .c:where(.svelte-xyz) { color: green; }
+	.b.svelte-xyz ~ .c:where(.svelte-xyz) { color: green; }
diff --git a/packages/svelte/tests/css/samples/general-siblings-combinator-key/input.svelte b/packages/svelte/tests/css/samples/general-siblings-combinator-key/input.svelte
@@ -0,0 +1,13 @@
+<div class="a"></div>
+
+{#key x}
+	<div class="b"></div>
+{/key}
+
+<div class="c"></div>
+
+<style>
+	.a ~ .b { color: green; }
+	.a ~ .c { color: green; }
+	.b ~ .c { color: green; }
+</style>
diff --git a/packages/svelte/tests/css/samples/siblings-combinator-each-else-nested/_config.js b/packages/svelte/tests/css/samples/siblings-combinator-each-else-nested/_config.js
@@ -5,62 +5,38 @@ export default test({
 		{
 			code: 'css_unused_selector',
 			message: 'Unused CSS selector ".a + .c"',
-			start: { character: 478, column: 1, line: 23 },
-			end: { character: 485, column: 8, line: 23 }
-		},
-		{
-			code: 'css_unused_selector',
-			message: 'Unused CSS selector ".a + .g"',
-			start: { character: 505, column: 1, line: 24 },
-			end: { character: 512, column: 8, line: 24 }
+			start: { character: 586, column: 1, line: 27 },
+			end: { character: 593, column: 8, line: 27 }
 		},
 		{
 			code: 'css_unused_selector',
 			message: 'Unused CSS selector ".b + .e"',
-			start: { character: 532, column: 1, line: 25 },
-			end: { character: 539, column: 8, line: 25 }
-		},
-		{
-			code: 'css_unused_selector',
-			message: 'Unused CSS selector ".c + .g"',
-			start: { character: 559, column: 1, line: 26 },
-			end: { character: 566, column: 8, line: 26 }
-		},
-		{
-			code: 'css_unused_selector',
-			message: 'Unused CSS selector ".c + .k"',
-			start: { character: 586, column: 1, line: 27 },
-			end: { character: 593, column: 8, line: 27 }
+			start: { character: 611, column: 1, line: 28 },
+			end: { character: 618, column: 8, line: 28 }
 		},
 		{
 			code: 'css_unused_selector',
 			message: 'Unused CSS selector ".d + .d"',
-			start: { character: 613, column: 1, line: 28 },
-			end: { character: 620, column: 8, line: 28 }
+			start: { character: 636, column: 1, line: 29 },
+			end: { character: 643, column: 8, line: 29 }
 		},
 		{
 			code: 'css_unused_selector',
 			message: 'Unused CSS selector ".e + .f"',
-			start: { character: 640, column: 1, line: 29 },
-			end: { character: 647, column: 8, line: 29 }
+			start: { character: 661, column: 1, line: 30 },
+			end: { character: 668, column: 8, line: 30 }
 		},
 		{
 			code: 'css_unused_selector',
 			message: 'Unused CSS selector ".f + .f"',
-			start: { character: 667, column: 1, line: 30 },
-			end: { character: 674, column: 8, line: 30 }
-		},
-		{
-			code: 'css_unused_selector',
-			message: 'Unused CSS selector ".g + .j"',
-			start: { character: 694, column: 1, line: 31 },
-			end: { character: 701, column: 8, line: 31 }
+			start: { character: 686, column: 1, line: 31 },
+			end: { character: 693, column: 8, line: 31 }
 		},
 		{
 			code: 'css_unused_selector',
 			message: 'Unused CSS selector ".g + .h + .i + .j"',
-			start: { character: 721, column: 1, line: 32 },
-			end: { character: 738, column: 18, line: 32 }
+			start: { character: 711, column: 1, line: 32 },
+			end: { character: 728, column: 18, line: 32 }
 		}
 	]
 });
diff --git a/packages/svelte/tests/css/samples/siblings-combinator-each-else-nested/expected.css b/packages/svelte/tests/css/samples/siblings-combinator-each-else-nested/expected.css
@@ -1,27 +1,28 @@
+
 	.a.svelte-xyz + .e:where(.svelte-xyz) { color: green; }
 	.a.svelte-xyz + .f:where(.svelte-xyz) { color: green; }
+	.a.svelte-xyz + .g:where(.svelte-xyz) { color: green; }
 	.b.svelte-xyz + .c:where(.svelte-xyz) { color: green; }
 	.b.svelte-xyz + .d:where(.svelte-xyz) { color: green; }
 	.c.svelte-xyz + .e:where(.svelte-xyz) { color: green; }
 	.c.svelte-xyz + .f:where(.svelte-xyz) { color: green; }
+	.c.svelte-xyz + .g:where(.svelte-xyz) { color: green; }
+	.c.svelte-xyz + .k:where(.svelte-xyz) { color: green; }
 	.d.svelte-xyz + .e:where(.svelte-xyz) { color: green; }
 	.d.svelte-xyz + .f:where(.svelte-xyz) { color: green; }
 	.e.svelte-xyz + .e:where(.svelte-xyz) { color: green; }
 	.i.svelte-xyz + .j:where(.svelte-xyz) { color: green; }
 	.g.svelte-xyz + .h:where(.svelte-xyz) + .j:where(.svelte-xyz) { color: green; }
 	.g.svelte-xyz + .i:where(.svelte-xyz) + .j:where(.svelte-xyz) { color: green; }
+	.g.svelte-xyz + .j:where(.svelte-xyz) { color: green; }
 	.m.svelte-xyz + .m:where(.svelte-xyz) { color: green; }
 	.m.svelte-xyz + .l:where(.svelte-xyz) { color: green; }
 	.l.svelte-xyz + .m:where(.svelte-xyz) { color: green; }
 
 	/* no match */
-	/* (unused) .a + .c { color: green; }*/
-	/* (unused) .a + .g { color: green; }*/
-	/* (unused) .b + .e { color: green; }*/
-	/* (unused) .c + .g { color: green; }*/
-	/* (unused) .c + .k { color: green; }*/
-	/* (unused) .d + .d { color: green; }*/
-	/* (unused) .e + .f { color: green; }*/
-	/* (unused) .f + .f { color: green; }*/
-	/* (unused) .g + .j { color: green; }*/
-	/* (unused) .g + .h + .i + .j { color: green; }*/
+	/* (unused) .a + .c { color: red; }*/
+	/* (unused) .b + .e { color: red; }*/
+	/* (unused) .d + .d { color: red; }*/
+	/* (unused) .e + .f { color: red; }*/
+	/* (unused) .f + .f { color: red; }*/
+	/* (unused) .g + .h + .i + .j { color: red; }*/
diff --git a/packages/svelte/tests/css/samples/siblings-combinator-each-else-nested/expected.html b/packages/svelte/tests/css/samples/siblings-combinator-each-else-nested/expected.html
@@ -1,3 +1,3 @@
 <div class="a svelte-xyz"></div>
 <div class="f svelte-xyz"></div>
-<div class="k"></div>
+<div class="k svelte-xyz"></div>
diff --git a/packages/svelte/tests/css/samples/siblings-combinator-each-else-nested/input.svelte b/packages/svelte/tests/css/samples/siblings-combinator-each-else-nested/input.svelte
@@ -5,31 +5,31 @@
 <style>
 	.a + .e { color: green; }
 	.a + .f { color: green; }
+	.a + .g { color: green; }
 	.b + .c { color: green; }
 	.b + .d { color: green; }
 	.c + .e { color: green; }
 	.c + .f { color: green; }
+	.c + .g { color: green; }
+	.c + .k { color: green; }
 	.d + .e { color: green; }
 	.d + .f { color: green; }
 	.e + .e { color: green; }
 	.i + .j { color: green; }
 	.g + .h + .j { color: green; }
 	.g + .i + .j { color: green; }
+	.g + .j { color: green; }
 	.m + .m { color: green; }
 	.m + .l { color: green; }
 	.l + .m { color: green; }
 
 	/* no match */
-	.a + .c { color: green; }
-	.a + .g { color: green; }
-	.b + .e { color: green; }
-	.c + .g { color: green; }
-	.c + .k { color: green; }
-	.d + .d { color: green; }
-	.e + .f { color: green; }
-	.f + .f { color: green; }
-	.g + .j { color: green; }
-	.g + .h + .i + .j { color: green; }
+	.a + .c { color: red; }
+	.b + .e { color: red; }
+	.d + .d { color: red; }
+	.e + .f { color: red; }
+	.f + .f { color: red; }
+	.g + .h + .i + .j { color: red; }
 </style>
 
 <div class="a"></div>

diff --git a/packages/svelte/tests/css/samples/siblings-combinator-key/_config.js b/packages/svelte/tests/css/samples/siblings-combinator-key/_config.js
@@ -0,0 +1,12 @@
+import { test } from '../../test';
+
+export default test({
+	warnings: [
+		{
+			code: 'css_unused_selector',
+			message: 'Unused CSS selector ".a + .c"',
+			start: { character: 166, column: 1, line: 14 },
+			end: { character: 173, column: 8, line: 14 }
+		}
+	]
+});
diff --git a/packages/svelte/tests/css/samples/siblings-combinator-key/expected.css b/packages/svelte/tests/css/samples/siblings-combinator-key/expected.css
@@ -0,0 +1,6 @@
+
+	.a.svelte-xyz + .b:where(.svelte-xyz) { color: green; }
+	.b.svelte-xyz + .c:where(.svelte-xyz) { color: green; }
+
+	/* no match */
+	/* (unused) .a + .c { color: red; }*/