diff --git a/.changeset/forty-suns-smile.md b/.changeset/forty-suns-smile.md new file mode 100644 index 000000000000..493b0b04747e --- /dev/null +++ b/.changeset/forty-suns-smile.md @@ -0,0 +1,5 @@ +--- +"svelte": patch +--- + +fix: properly analyze group expressions diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index 6f8bfd2064d9..a7d4a9180ab0 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/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 = { diff --git a/packages/svelte/src/compiler/phases/types.d.ts b/packages/svelte/src/compiler/phases/types.d.ts index ba1c14788910..273c8112b1f7 100644 --- a/packages/svelte/src/compiler/phases/types.d.ts +++ b/packages/svelte/src/compiler/phases/types.d.ts @@ -68,7 +68,7 @@ export interface ComponentAnalysis extends Analysis { inject_styles: boolean; reactive_statements: Map; /** Identifiers that make up the `bind:group` expression -> internal group binding name */ - binding_groups: Map, Identifier>; + binding_groups: Map<[key: string, bindings: Array], Identifier>; slot_names: Set; } diff --git a/packages/svelte/src/compiler/utils/ast.js b/packages/svelte/src/compiler/utils/ast.js index 9f16679bc730..33bdc451c869 100644 --- a/packages/svelte/src/compiler/utils/ast.js +++ b/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]; } /** diff --git a/packages/svelte/tests/runtime-legacy/samples/binding-input-group-each-16/_config.js b/packages/svelte/tests/runtime-legacy/samples/binding-input-group-each-16/_config.js new file mode 100644 index 000000000000..54ddd20af17d --- /dev/null +++ b/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} */ ( + 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'); + } +}); diff --git a/packages/svelte/tests/runtime-legacy/samples/binding-input-group-each-16/main.svelte b/packages/svelte/tests/runtime-legacy/samples/binding-input-group-each-16/main.svelte new file mode 100644 index 000000000000..f9aed83ed245 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/binding-input-group-each-16/main.svelte @@ -0,0 +1,17 @@ + + +
+ One scoop + Two scoops + Three scoops + + {#each menu as flavour} + {flavour} + {/each} +
+ +
{$order.flavours.join('+')}