Skip to content

Commit 37e6c7f

Browse files
Rich-HarrisRich-Harris
authored
fix: correctly prune key/each blocks (#14403)
* fix: correctly prune key blocks * fix pruning of each blocks * simplify * make more explicit * changeset * helperise/robustify
1 parent e721d96 commit 37e6c7f

File tree

13 files changed

+156
-128
lines changed

13 files changed

+156
-128
lines changed

‎.changeset/fuzzy-lies-battle.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'svelte': patch
3+
---
4+
5+
fix: correctly prune each blocks

‎.changeset/wicked-buses-work.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'svelte': patch
3+
---
4+
5+
fix: correctly prune key blocks

‎packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js

+57-71
Original file line numberDiff line numberDiff line change
@@ -955,7 +955,7 @@ function get_possible_element_siblings(node, adjacent_only) {
955955
if (adjacent_only) {
956956
break;
957957
}
958-
} else if (prev.type === 'EachBlock' || prev.type === 'IfBlock' || prev.type === 'AwaitBlock') {
958+
} else if (is_block(prev)) {
959959
const possible_last_child = get_possible_last_child(prev, adjacent_only);
960960
add_to_map(possible_last_child, result);
961961
if (adjacent_only && has_definite_elements(possible_last_child)) {
@@ -979,7 +979,7 @@ function get_possible_element_siblings(node, adjacent_only) {
979979
while (
980980
// @ts-expect-error TODO
981981
(parent = parent?.parent) &&
982-
(parent.type === 'EachBlock' || parent.type === 'IfBlock' || parent.type === 'AwaitBlock')
982+
is_block(parent)
983983
) {
984984
const possible_siblings = get_possible_element_siblings(parent, adjacent_only);
985985
add_to_map(possible_siblings, result);
@@ -1000,73 +1000,57 @@ function get_possible_element_siblings(node, adjacent_only) {
10001000
}
10011001

10021002
/**
1003-
* @param {Compiler.AST.EachBlock | Compiler.AST.IfBlock | Compiler.AST.AwaitBlock} relative_selector
1003+
* @param {Compiler.AST.EachBlock | Compiler.AST.IfBlock | Compiler.AST.AwaitBlock | Compiler.AST.KeyBlock} node
10041004
* @param {boolean} adjacent_only
10051005
* @returns {Map<Compiler.AST.RegularElement, NodeExistsValue>}
10061006
*/
1007-
function get_possible_last_child(relative_selector, adjacent_only) {
1007+
function get_possible_last_child(node, adjacent_only) {
10081008
/** @typedef {Map<Compiler.AST.RegularElement, NodeExistsValue>} NodeMap */
10091009

1010+
/** @type {Array<Compiler.AST.Fragment | undefined | null>} */
1011+
let fragments = [];
1012+
1013+
switch (node.type) {
1014+
case 'EachBlock':
1015+
fragments.push(node.body, node.fallback);
1016+
break;
1017+
1018+
case 'IfBlock':
1019+
fragments.push(node.consequent, node.alternate);
1020+
break;
1021+
1022+
case 'AwaitBlock':
1023+
fragments.push(node.pending, node.then, node.catch);
1024+
break;
1025+
1026+
case 'KeyBlock':
1027+
fragments.push(node.fragment);
1028+
break;
1029+
}
1030+
10101031
/** @type {NodeMap} */
10111032
const result = new Map();
1012-
if (relative_selector.type === 'EachBlock') {
1013-
/** @type {NodeMap} */
1014-
const each_result = loop_child(relative_selector.body.nodes, adjacent_only);
1015-
1016-
/** @type {NodeMap} */
1017-
const else_result = relative_selector.fallback
1018-
? loop_child(relative_selector.fallback.nodes, adjacent_only)
1019-
: new Map();
1020-
const not_exhaustive = !has_definite_elements(else_result);
1021-
if (not_exhaustive) {
1022-
mark_as_probably(each_result);
1023-
mark_as_probably(else_result);
1024-
}
1025-
add_to_map(each_result, result);
1026-
add_to_map(else_result, result);
1027-
} else if (relative_selector.type === 'IfBlock') {
1028-
/** @type {NodeMap} */
1029-
const if_result = loop_child(relative_selector.consequent.nodes, adjacent_only);
1030-
1031-
/** @type {NodeMap} */
1032-
const else_result = relative_selector.alternate
1033-
? loop_child(relative_selector.alternate.nodes, adjacent_only)
1034-
: new Map();
1035-
const not_exhaustive = !has_definite_elements(if_result) || !has_definite_elements(else_result);
1036-
if (not_exhaustive) {
1037-
mark_as_probably(if_result);
1038-
mark_as_probably(else_result);
1033+
1034+
let exhaustive = true;
1035+
1036+
for (const fragment of fragments) {
1037+
if (fragment == null) {
1038+
exhaustive = false;
1039+
continue;
10391040
}
1040-
add_to_map(if_result, result);
1041-
add_to_map(else_result, result);
1042-
} else if (relative_selector.type === 'AwaitBlock') {
1043-
/** @type {NodeMap} */
1044-
const pending_result = relative_selector.pending
1045-
? loop_child(relative_selector.pending.nodes, adjacent_only)
1046-
: new Map();
1047-
1048-
/** @type {NodeMap} */
1049-
const then_result = relative_selector.then
1050-
? loop_child(relative_selector.then.nodes, adjacent_only)
1051-
: new Map();
1052-
1053-
/** @type {NodeMap} */
1054-
const catch_result = relative_selector.catch
1055-
? loop_child(relative_selector.catch.nodes, adjacent_only)
1056-
: new Map();
1057-
const not_exhaustive =
1058-
!has_definite_elements(pending_result) ||
1059-
!has_definite_elements(then_result) ||
1060-
!has_definite_elements(catch_result);
1061-
if (not_exhaustive) {
1062-
mark_as_probably(pending_result);
1063-
mark_as_probably(then_result);
1064-
mark_as_probably(catch_result);
1041+
1042+
const map = loop_child(fragment.nodes, adjacent_only);
1043+
exhaustive &&= has_definite_elements(map);
1044+
1045+
add_to_map(map, result);
1046+
}
1047+
1048+
if (!exhaustive) {
1049+
for (const key of result.keys()) {
1050+
result.set(key, NODE_PROBABLY_EXISTS);
10651051
}
1066-
add_to_map(pending_result, result);
1067-
add_to_map(then_result, result);
1068-
add_to_map(catch_result, result);
10691052
}
1053+
10701054
return result;
10711055
}
10721056

@@ -1107,13 +1091,6 @@ function higher_existence(exist1, exist2) {
11071091
return exist1 > exist2 ? exist1 : exist2;
11081092
}
11091093

1110-
/** @param {Map<Compiler.AST.RegularElement, NodeExistsValue>} result */
1111-
function mark_as_probably(result) {
1112-
for (const key of result.keys()) {
1113-
result.set(key, NODE_PROBABLY_EXISTS);
1114-
}
1115-
}
1116-
11171094
/**
11181095
* @param {Compiler.SvelteNode[]} children
11191096
* @param {boolean} adjacent_only
@@ -1132,11 +1109,7 @@ function loop_child(children, adjacent_only) {
11321109
if (adjacent_only) {
11331110
break;
11341111
}
1135-
} else if (
1136-
child.type === 'EachBlock' ||
1137-
child.type === 'IfBlock' ||
1138-
child.type === 'AwaitBlock'
1139-
) {
1112+
} else if (is_block(child)) {
11401113
const child_result = get_possible_last_child(child, adjacent_only);
11411114
add_to_map(child_result, result);
11421115
if (adjacent_only && has_definite_elements(child_result)) {
@@ -1147,3 +1120,16 @@ function loop_child(children, adjacent_only) {
11471120

11481121
return result;
11491122
}
1123+
1124+
/**
1125+
* @param {Compiler.SvelteNode} node
1126+
* @returns {node is Compiler.AST.IfBlock | Compiler.AST.EachBlock | Compiler.AST.AwaitBlock | Compiler.AST.KeyBlock}
1127+
*/
1128+
function is_block(node) {
1129+
return (
1130+
node.type === 'IfBlock' ||
1131+
node.type === 'EachBlock' ||
1132+
node.type === 'AwaitBlock' ||
1133+
node.type === 'KeyBlock'
1134+
);
1135+
}

‎packages/svelte/tests/css/samples/general-siblings-combinator-key/_config.js

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import { test } from '../../test';
2+
3+
export default test({
4+
warnings: []
5+
});

‎packages/svelte/tests/css/samples/general-siblings-combinator-key/expected.css

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
2+
.a.svelte-xyz ~ .b:where(.svelte-xyz) { color: green; }
3+
.a.svelte-xyz ~ .c:where(.svelte-xyz) { color: green; }
4+
.b.svelte-xyz ~ .c:where(.svelte-xyz) { color: green; }

‎packages/svelte/tests/css/samples/general-siblings-combinator-key/input.svelte

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<div class="a"></div>
2+
3+
{#key x}
4+
<div class="b"></div>
5+
{/key}
6+
7+
<div class="c"></div>
8+
9+
<style>
10+
.a ~ .b { color: green; }
11+
.a ~ .c { color: green; }
12+
.b ~ .c { color: green; }
13+
</style>

‎packages/svelte/tests/css/samples/siblings-combinator-each-else-nested/_config.js

+12-36
Original file line numberDiff line numberDiff line change
@@ -5,62 +5,38 @@ export default test({
55
{
66
code: 'css_unused_selector',
77
message: 'Unused CSS selector ".a + .c"',
8-
start: { character: 478, column: 1, line: 23 },
9-
end: { character: 485, column: 8, line: 23 }
10-
},
11-
{
12-
code: 'css_unused_selector',
13-
message: 'Unused CSS selector ".a + .g"',
14-
start: { character: 505, column: 1, line: 24 },
15-
end: { character: 512, column: 8, line: 24 }
8+
start: { character: 586, column: 1, line: 27 },
9+
end: { character: 593, column: 8, line: 27 }
1610
},
1711
{
1812
code: 'css_unused_selector',
1913
message: 'Unused CSS selector ".b + .e"',
20-
start: { character: 532, column: 1, line: 25 },
21-
end: { character: 539, column: 8, line: 25 }
22-
},
23-
{
24-
code: 'css_unused_selector',
25-
message: 'Unused CSS selector ".c + .g"',
26-
start: { character: 559, column: 1, line: 26 },
27-
end: { character: 566, column: 8, line: 26 }
28-
},
29-
{
30-
code: 'css_unused_selector',
31-
message: 'Unused CSS selector ".c + .k"',
32-
start: { character: 586, column: 1, line: 27 },
33-
end: { character: 593, column: 8, line: 27 }
14+
start: { character: 611, column: 1, line: 28 },
15+
end: { character: 618, column: 8, line: 28 }
3416
},
3517
{
3618
code: 'css_unused_selector',
3719
message: 'Unused CSS selector ".d + .d"',
38-
start: { character: 613, column: 1, line: 28 },
39-
end: { character: 620, column: 8, line: 28 }
20+
start: { character: 636, column: 1, line: 29 },
21+
end: { character: 643, column: 8, line: 29 }
4022
},
4123
{
4224
code: 'css_unused_selector',
4325
message: 'Unused CSS selector ".e + .f"',
44-
start: { character: 640, column: 1, line: 29 },
45-
end: { character: 647, column: 8, line: 29 }
26+
start: { character: 661, column: 1, line: 30 },
27+
end: { character: 668, column: 8, line: 30 }
4628
},
4729
{
4830
code: 'css_unused_selector',
4931
message: 'Unused CSS selector ".f + .f"',
50-
start: { character: 667, column: 1, line: 30 },
51-
end: { character: 674, column: 8, line: 30 }
52-
},
53-
{
54-
code: 'css_unused_selector',
55-
message: 'Unused CSS selector ".g + .j"',
56-
start: { character: 694, column: 1, line: 31 },
57-
end: { character: 701, column: 8, line: 31 }
32+
start: { character: 686, column: 1, line: 31 },
33+
end: { character: 693, column: 8, line: 31 }
5834
},
5935
{
6036
code: 'css_unused_selector',
6137
message: 'Unused CSS selector ".g + .h + .i + .j"',
62-
start: { character: 721, column: 1, line: 32 },
63-
end: { character: 738, column: 18, line: 32 }
38+
start: { character: 711, column: 1, line: 32 },
39+
end: { character: 728, column: 18, line: 32 }
6440
}
6541
]
6642
});

‎packages/svelte/tests/css/samples/siblings-combinator-each-else-nested/expected.css

+11-10
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,28 @@
1+
12
.a.svelte-xyz + .e:where(.svelte-xyz) { color: green; }
23
.a.svelte-xyz + .f:where(.svelte-xyz) { color: green; }
4+
.a.svelte-xyz + .g:where(.svelte-xyz) { color: green; }
35
.b.svelte-xyz + .c:where(.svelte-xyz) { color: green; }
46
.b.svelte-xyz + .d:where(.svelte-xyz) { color: green; }
57
.c.svelte-xyz + .e:where(.svelte-xyz) { color: green; }
68
.c.svelte-xyz + .f:where(.svelte-xyz) { color: green; }
9+
.c.svelte-xyz + .g:where(.svelte-xyz) { color: green; }
10+
.c.svelte-xyz + .k:where(.svelte-xyz) { color: green; }
711
.d.svelte-xyz + .e:where(.svelte-xyz) { color: green; }
812
.d.svelte-xyz + .f:where(.svelte-xyz) { color: green; }
913
.e.svelte-xyz + .e:where(.svelte-xyz) { color: green; }
1014
.i.svelte-xyz + .j:where(.svelte-xyz) { color: green; }
1115
.g.svelte-xyz + .h:where(.svelte-xyz) + .j:where(.svelte-xyz) { color: green; }
1216
.g.svelte-xyz + .i:where(.svelte-xyz) + .j:where(.svelte-xyz) { color: green; }
17+
.g.svelte-xyz + .j:where(.svelte-xyz) { color: green; }
1318
.m.svelte-xyz + .m:where(.svelte-xyz) { color: green; }
1419
.m.svelte-xyz + .l:where(.svelte-xyz) { color: green; }
1520
.l.svelte-xyz + .m:where(.svelte-xyz) { color: green; }
1621

1722
/* no match */
18-
/* (unused) .a + .c { color: green; }*/
19-
/* (unused) .a + .g { color: green; }*/
20-
/* (unused) .b + .e { color: green; }*/
21-
/* (unused) .c + .g { color: green; }*/
22-
/* (unused) .c + .k { color: green; }*/
23-
/* (unused) .d + .d { color: green; }*/
24-
/* (unused) .e + .f { color: green; }*/
25-
/* (unused) .f + .f { color: green; }*/
26-
/* (unused) .g + .j { color: green; }*/
27-
/* (unused) .g + .h + .i + .j { color: green; }*/
23+
/* (unused) .a + .c { color: red; }*/
24+
/* (unused) .b + .e { color: red; }*/
25+
/* (unused) .d + .d { color: red; }*/
26+
/* (unused) .e + .f { color: red; }*/
27+
/* (unused) .f + .f { color: red; }*/
28+
/* (unused) .g + .h + .i + .j { color: red; }*/

‎packages/svelte/tests/css/samples/siblings-combinator-each-else-nested/expected.html

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
<div class="a svelte-xyz"></div>
22
<div class="f svelte-xyz"></div>
3-
<div class="k"></div>
3+
<div class="k svelte-xyz"></div>

‎packages/svelte/tests/css/samples/siblings-combinator-each-else-nested/input.svelte

+10-10
Original file line numberDiff line numberDiff line change
@@ -5,31 +5,31 @@
55
<style>
66
.a + .e { color: green; }
77
.a + .f { color: green; }
8+
.a + .g { color: green; }
89
.b + .c { color: green; }
910
.b + .d { color: green; }
1011
.c + .e { color: green; }
1112
.c + .f { color: green; }
13+
.c + .g { color: green; }
14+
.c + .k { color: green; }
1215
.d + .e { color: green; }
1316
.d + .f { color: green; }
1417
.e + .e { color: green; }
1518
.i + .j { color: green; }
1619
.g + .h + .j { color: green; }
1720
.g + .i + .j { color: green; }
21+
.g + .j { color: green; }
1822
.m + .m { color: green; }
1923
.m + .l { color: green; }
2024
.l + .m { color: green; }
2125
2226
/* no match */
23-
.a + .c { color: green; }
24-
.a + .g { color: green; }
25-
.b + .e { color: green; }
26-
.c + .g { color: green; }
27-
.c + .k { color: green; }
28-
.d + .d { color: green; }
29-
.e + .f { color: green; }
30-
.f + .f { color: green; }
31-
.g + .j { color: green; }
32-
.g + .h + .i + .j { color: green; }
27+
.a + .c { color: red; }
28+
.b + .e { color: red; }
29+
.d + .d { color: red; }
30+
.e + .f { color: red; }
31+
.f + .f { color: red; }
32+
.g + .h + .i + .j { color: red; }
3333
</style>
3434

3535
<div class="a"></div>

‎packages/svelte/tests/css/samples/siblings-combinator-key/_config.js

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import { test } from '../../test';
2+
3+
export default test({
4+
warnings: [
5+
{
6+
code: 'css_unused_selector',
7+
message: 'Unused CSS selector ".a + .c"',
8+
start: { character: 166, column: 1, line: 14 },
9+
end: { character: 173, column: 8, line: 14 }
10+
}
11+
]
12+
});

‎packages/svelte/tests/css/samples/siblings-combinator-key/expected.css

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
2+
.a.svelte-xyz + .b:where(.svelte-xyz) { color: green; }
3+
.b.svelte-xyz + .c:where(.svelte-xyz) { color: green; }
4+
5+
/* no match */
6+
/* (unused) .a + .c { color: red; }*/

0 commit comments

Comments
 (0)