fix: properly analyze group expressions (#10410)

fixes #9947 for real
closes #10379

Author: dummdidumm

.changeset/forty-suns-smile.md

@@ -0,0 +1,5 @@
+---
+"svelte": patch
+---
+
+fix: properly analyze group expressions

packages/svelte/src/compiler/phases/2-analyze/index.js

@@ -1014,7 +1014,7 @@ const common_visitors = {
 		// entries as keys.
 		i = context.path.length;
 		const each_blocks = [];
-		const expression_ids = extract_all_identifiers_from_expression(node.expression);
+		const [keypath, expression_ids] = extract_all_identifiers_from_expression(node.expression);
 		let ids = expression_ids;
 		while (i--) {
 			const parent = context.path[i];
@@ -1027,7 +1027,7 @@ const common_visitors = {
 					}
 					each_blocks.push(parent);
 					ids = ids.filter((id) => !references.includes(id));
-					ids.push(...extract_all_identifiers_from_expression(parent.expression));
+					ids.push(...extract_all_identifiers_from_expression(parent.expression)[1]);
 				}
 			}
 		}
@@ -1038,8 +1038,8 @@ const common_visitors = {
 		//  but this is a limitation of the current static analysis we do; it also never worked in Svelte 4)
 		const bindings = expression_ids.map((id) => context.state.scope.get(id.name));
 		let group_name;
-		outer: for (const [b, group] of context.state.analysis.binding_groups) {
-			if (b.length !== bindings.length) continue;
+		outer: for (const [[key, b], group] of context.state.analysis.binding_groups) {
+			if (b.length !== bindings.length || key !== keypath) continue;
 			for (let i = 0; i < bindings.length; i++) {
 				if (bindings[i] !== b[i]) continue outer;
 			}
@@ -1048,7 +1048,7 @@ const common_visitors = {
 
 		if (!group_name) {
 			group_name = context.state.scope.root.unique('binding_group');
-			context.state.analysis.binding_groups.set(bindings, group_name);
+			context.state.analysis.binding_groups.set([keypath, bindings], group_name);
 		}
 
 		node.metadata = {

packages/svelte/src/compiler/phases/types.d.ts

@@ -67,7 +67,7 @@ export interface ComponentAnalysis extends Analysis {
 	inject_styles: boolean;
 	reactive_statements: Map<LabeledStatement, ReactiveStatement>;
 	/** Identifiers that make up the `bind:group` expression -> internal group binding name */
-	binding_groups: Map<Array<Binding | null>, Identifier>;
+	binding_groups: Map<[key: string, bindings: Array<Binding | null>], Identifier>;
 	slot_names: Set<string>;
 }
 

packages/svelte/src/compiler/utils/ast.js

@@ -96,13 +96,15 @@ export function extract_identifiers(param, nodes = []) {
 }
 
 /**
- * Extracts all identifiers from an expression.
+ * Extracts all identifiers and a stringified keypath from an expression.
  * @param {import('estree').Expression} expr
- * @returns {import('estree').Identifier[]}
+ * @returns {[keypath: string, ids: import('estree').Identifier[]]}
  */
 export function extract_all_identifiers_from_expression(expr) {
 	/** @type {import('estree').Identifier[]} */
 	let nodes = [];
+	/** @type {string[]} */
+	let keypath = [];
 
 	walk(
 		expr,
@@ -113,11 +115,30 @@ export function extract_all_identifiers_from_expression(expr) {
 				if (parent?.type !== 'MemberExpression' || parent.property !== node || parent.computed) {
 					nodes.push(node);
 				}
+
+				if (parent?.type === 'MemberExpression' && parent.computed && parent.property === node) {
+					keypath.push(`[${node.name}]`);
+				} else {
+					keypath.push(node.name);
+				}
+			},
+			Literal(node, { path }) {
+				const value = typeof node.value === 'string' ? `"${node.value}"` : String(node.value);
+				const parent = path.at(-1);
+				if (parent?.type === 'MemberExpression' && parent.computed && parent.property === node) {
+					keypath.push(`[${value}]`);
+				} else {
+					keypath.push(value);
+				}
+			},
+			ThisExpression(_, { next }) {
+				keypath.push('this');
+				next();
 			}
 		}
 	);
 
-	return nodes;
+	return [keypath.join('.'), nodes];
 }
 
 /**

packages/svelte/tests/runtime-legacy/samples/binding-input-group-each-16/_config.js

@@ -0,0 +1,23 @@
+import { test } from '../../test';
+
+export default test({
+	async test({ assert, target }) {
+		const checkboxes = /** @type {NodeListOf<HTMLInputElement>} */ (
+			target.querySelectorAll('input[type="checkbox"]')
+		);
+
+		assert.isFalse(checkboxes[0].checked);
+		assert.isTrue(checkboxes[1].checked);
+		assert.isFalse(checkboxes[2].checked);
+
+		await checkboxes[1].click();
+
+		const noChecked = target.querySelector('#output')?.innerHTML;
+		assert.equal(noChecked, '');
+
+		await checkboxes[1].click();
+
+		const oneChecked = target.querySelector('#output')?.innerHTML;
+		assert.equal(oneChecked, 'Mint choc chip');
+	}
+});

packages/svelte/tests/runtime-legacy/samples/binding-input-group-each-16/main.svelte

@@ -0,0 +1,17 @@
+<script lang="ts">
+	import { writable } from 'svelte/store';
+	let menu = ['Cookies and cream', 'Mint choc chip', 'Raspberry ripple'];
+	let order = writable({flavours: ['Mint choc chip'], scoops: 1 });
+</script>
+
+<form method="POST">
+	<input type="radio" bind:group={$order.scoops} name="scoops" value={1} /> One scoop
+	<input type="radio" bind:group={$order.scoops} name="scoops" value={2} /> Two scoops
+	<input type="radio" bind:group={$order.scoops} name="scoops" value={3} /> Three scoops
+
+	{#each menu as flavour}
+		<input type="checkbox" bind:group={$order.flavours} name="flavours" value={flavour} /> {flavour}
+	{/each}
+</form>
+
+<div id="output">{$order.flavours.join('+')}</div>