From 0529a5090f02eb836ce34cf57ff67c2d87fb1a4b Mon Sep 17 00:00:00 2001 From: AyushCoder9 Date: Tue, 25 Nov 2025 01:34:39 +0530 Subject: [PATCH 1/2] fix: snapshot destructured values in #each blocks to prevent mutation issues Fixes #17227 When using destructuring in #each blocks (e.g., {#each gen() as [item]}), if the generator mutates the yielded object, all items would end up with the same final value because they all reference the same object. This fix adds snapshotting for destructured patterns in each blocks, ensuring that each iteration gets a snapshot of the value at the time it was yielded, matching the behavior of a standard for...of loop. - Add snapshot_each_value, snapshot_array, and snapshot_object utilities - Apply snapshotting in both client and server EachBlock visitors - Add test case to prevent regression --- .../3-transform/client/visitors/EachBlock.js | 44 +++++++++++++++++- .../3-transform/server/visitors/EachBlock.js | 45 ++++++++++++++++++- packages/svelte/src/internal/client/index.js | 2 +- packages/svelte/src/internal/server/index.js | 2 +- packages/svelte/src/internal/shared/utils.js | 34 ++++++++++++++ .../each-destructure-generator/_config.js | 12 +++++ .../each-destructure-generator/main.svelte | 17 +++++++ 7 files changed, 151 insertions(+), 5 deletions(-) create mode 100644 packages/svelte/tests/runtime-runes/samples/each-destructure-generator/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/each-destructure-generator/main.svelte diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js index c0bfe272e5..0b13f52fae 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js @@ -29,7 +29,7 @@ export function EachBlock(node, context) { scope: /** @type {Scope} */ (context.state.scope.parent) }; - const collection = build_expression( + let collection = build_expression( { ...context, state: parent_scope_state @@ -38,6 +38,17 @@ export function EachBlock(node, context) { node.metadata.expression ); + const destructured_pattern = get_destructured_pattern(node.context); + + if (destructured_pattern) { + const mapper = + destructured_pattern.type === 'ArrayPattern' + ? create_array_snapshot_mapper(destructured_pattern) + : create_object_snapshot_mapper(); + + collection = b.call('$.snapshot_each_value', collection, mapper); + } + if (!each_node_meta.is_controlled) { context.state.template.push_comment(); } @@ -365,3 +376,34 @@ export function EachBlock(node, context) { function collect_parent_each_blocks(context) { return /** @type {AST.EachBlock[]} */ (context.path.filter((node) => node.type === 'EachBlock')); } + +/** + * @param {import('estree').Pattern | null | undefined} pattern + * @returns {import('estree').ArrayPattern | import('estree').ObjectPattern | null} + */ +function get_destructured_pattern(pattern) { + if (!pattern) return null; + if (pattern.type === 'ArrayPattern' || pattern.type === 'ObjectPattern') { + return pattern; + } + + return null; +} + +/** + * @param {import('estree').ArrayPattern} pattern + */ +function create_array_snapshot_mapper(pattern) { + const value = b.id('$$value'); + const has_rest = pattern.elements.some((element) => element?.type === 'RestElement'); + + return b.arrow( + [value], + b.call('$.snapshot_array', value, b.literal(pattern.elements.length), has_rest ? b.true : b.false) + ); +} + +function create_object_snapshot_mapper() { + const value = b.id('$$value'); + return b.arrow([value], b.call('$.snapshot_object', value)); +} diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/EachBlock.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/EachBlock.js index 3c0a8c1676..e684aa717d 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/EachBlock.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/EachBlock.js @@ -1,4 +1,4 @@ -/** @import { BlockStatement, Expression, Statement } from 'estree' */ +/** @import { BlockStatement, Expression, Pattern, Statement } from 'estree' */ /** @import { AST } from '#compiler' */ /** @import { ComponentContext } from '../types.js' */ import * as b from '#compiler/builders'; @@ -12,7 +12,17 @@ export function EachBlock(node, context) { const state = context.state; const each_node_meta = node.metadata; - const collection = /** @type {Expression} */ (context.visit(node.expression)); + let collection = /** @type {Expression} */ (context.visit(node.expression)); + const destructured_pattern = get_destructured_pattern(node.context); + + if (destructured_pattern) { + const mapper = + destructured_pattern.type === 'ArrayPattern' + ? create_array_snapshot_mapper(destructured_pattern) + : create_object_snapshot_mapper(); + + collection = b.call('$.snapshot_each_value', collection, mapper); + } const index = each_node_meta.contains_group_binding || !node.index ? each_node_meta.index : b.id(node.index); @@ -78,3 +88,34 @@ export function EachBlock(node, context) { state.template.push(...block.body, block_close); } } + +/** + * @param {Pattern | null} pattern + * @returns {import('estree').ArrayPattern | import('estree').ObjectPattern | null} + */ +function get_destructured_pattern(pattern) { + if (!pattern) return null; + if (pattern.type === 'ArrayPattern' || pattern.type === 'ObjectPattern') { + return pattern; + } + + return null; +} + +/** + * @param {import('estree').ArrayPattern} pattern + */ +function create_array_snapshot_mapper(pattern) { + const value = b.id('$$value'); + const has_rest = pattern.elements.some((element) => element?.type === 'RestElement'); + + return b.arrow( + [value], + b.call('$.snapshot_array', value, b.literal(pattern.elements.length), has_rest ? b.true : b.false) + ); +} + +function create_object_snapshot_mapper() { + const value = b.id('$$value'); + return b.arrow([value], b.call('$.snapshot_object', value)); +} diff --git a/packages/svelte/src/internal/client/index.js b/packages/svelte/src/internal/client/index.js index a2add3ec59..2d5379de9b 100644 --- a/packages/svelte/src/internal/client/index.js +++ b/packages/svelte/src/internal/client/index.js @@ -170,7 +170,7 @@ export { } from './dom/operations.js'; export { attr, clsx } from '../shared/attributes.js'; export { snapshot } from '../shared/clone.js'; -export { noop, fallback, to_array } from '../shared/utils.js'; +export { noop, fallback, to_array, snapshot_array, snapshot_each_value, snapshot_object } from '../shared/utils.js'; export { invalid_default_snippet, validate_dynamic_element_tag, diff --git a/packages/svelte/src/internal/server/index.js b/packages/svelte/src/internal/server/index.js index c0dbdbda14..6f2e495006 100644 --- a/packages/svelte/src/internal/server/index.js +++ b/packages/svelte/src/internal/server/index.js @@ -454,7 +454,7 @@ export { push_element, pop_element, validate_snippet_args } from './dev.js'; export { snapshot } from '../shared/clone.js'; -export { fallback, to_array } from '../shared/utils.js'; +export { fallback, to_array, snapshot_array, snapshot_each_value, snapshot_object } from '../shared/utils.js'; export { invalid_default_snippet, diff --git a/packages/svelte/src/internal/shared/utils.js b/packages/svelte/src/internal/shared/utils.js index 10f8597520..15f54ea0da 100644 --- a/packages/svelte/src/internal/shared/utils.js +++ b/packages/svelte/src/internal/shared/utils.js @@ -116,3 +116,37 @@ export function to_array(value, n) { return array; } + +/** + * Snapshot items produced by an iterator so that destructured values reflect + * what was yielded before the iterator mutates the value again. + * @template T + * @param {ArrayLike | Iterable | null | undefined} collection + * @param {(value: T) => T} mapper + * @returns {Array} + */ +export function snapshot_each_value(collection, mapper) { + if (collection == null) { + return []; + } + + return is_array(collection) ? collection : array_from(collection, mapper); +} + +/** + * @param {any} value + * @param {number} length + * @param {boolean} has_rest + * @returns {any[]} + */ +export function snapshot_array(value, length, has_rest) { + const array = to_array(value, has_rest ? undefined : length); + return array.slice(); +} + +/** + * @param {any} value + */ +export function snapshot_object(value) { + return value == null || typeof value !== 'object' ? value : { ...value }; +} diff --git a/packages/svelte/tests/runtime-runes/samples/each-destructure-generator/_config.js b/packages/svelte/tests/runtime-runes/samples/each-destructure-generator/_config.js new file mode 100644 index 0000000000..31f0f8ff98 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/each-destructure-generator/_config.js @@ -0,0 +1,12 @@ +import { test } from '../../test'; + +export default test({ + html: ` +

0

+

1

+

2

+

3

+

4

+ ` +}); + diff --git a/packages/svelte/tests/runtime-runes/samples/each-destructure-generator/main.svelte b/packages/svelte/tests/runtime-runes/samples/each-destructure-generator/main.svelte new file mode 100644 index 0000000000..75a795bfeb --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/each-destructure-generator/main.svelte @@ -0,0 +1,17 @@ + + + + +{#each gen() as [item]} +

{item}

+{/each} + From 8348132c9c1d6bf5b9c916a634742c15a17e0bd1 Mon Sep 17 00:00:00 2001 From: AyushCoder9 Date: Tue, 25 Nov 2025 10:08:41 +0530 Subject: [PATCH 2/2] fix: destructure array patterns immediately in #each blocks to match for...of behavior Fixes #17227 When using array destructuring in #each blocks (e.g., {#each gen() as [item]}), if the generator mutates the yielded array object, all items would end up with the same final value because destructuring happened after the collection was converted to an array, rather than immediately during iteration. This fix ensures that array destructuring happens immediately during iteration, capturing values at the time they are yielded, matching the behavior of a standard for...of loop with destructuring. - Add to_array_destructured utility that destructures during iteration - Apply immediate destructuring for array patterns in both client and server EachBlock visitors - Keep object destructuring using snapshotting (shallow copy) - Add test case to prevent regression --- .../3-transform/client/visitors/EachBlock.js | 38 ++++++------- .../3-transform/server/visitors/EachBlock.js | 37 ++++++------ packages/svelte/src/internal/client/index.js | 2 +- packages/svelte/src/internal/server/index.js | 2 +- packages/svelte/src/internal/shared/utils.js | 57 +++++++++++++++---- 5 files changed, 85 insertions(+), 51 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js index 0b13f52fae..c9c94096f4 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js @@ -41,12 +41,25 @@ export function EachBlock(node, context) { const destructured_pattern = get_destructured_pattern(node.context); if (destructured_pattern) { - const mapper = - destructured_pattern.type === 'ArrayPattern' - ? create_array_snapshot_mapper(destructured_pattern) - : create_object_snapshot_mapper(); - - collection = b.call('$.snapshot_each_value', collection, mapper); + if (destructured_pattern.type === 'ArrayPattern') { + // For array destructuring, we need to destructure immediately during iteration + // to match for...of behavior, capturing values before the generator mutates them + const indices = []; + for (let i = 0; i < destructured_pattern.elements.length; i++) { + const element = destructured_pattern.elements[i]; + if (element && element.type !== 'RestElement') { + indices.push(i); + } + } + collection = b.call( + '$.to_array_destructured', + collection, + b.array(indices.map((i) => b.literal(i))) + ); + } else { + // For object destructuring, we still need to snapshot to capture values + collection = b.call('$.snapshot_each_value', collection, create_object_snapshot_mapper()); + } } if (!each_node_meta.is_controlled) { @@ -390,19 +403,6 @@ function get_destructured_pattern(pattern) { return null; } -/** - * @param {import('estree').ArrayPattern} pattern - */ -function create_array_snapshot_mapper(pattern) { - const value = b.id('$$value'); - const has_rest = pattern.elements.some((element) => element?.type === 'RestElement'); - - return b.arrow( - [value], - b.call('$.snapshot_array', value, b.literal(pattern.elements.length), has_rest ? b.true : b.false) - ); -} - function create_object_snapshot_mapper() { const value = b.id('$$value'); return b.arrow([value], b.call('$.snapshot_object', value)); diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/EachBlock.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/EachBlock.js index e684aa717d..1430c1071c 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/EachBlock.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/EachBlock.js @@ -16,12 +16,24 @@ export function EachBlock(node, context) { const destructured_pattern = get_destructured_pattern(node.context); if (destructured_pattern) { - const mapper = - destructured_pattern.type === 'ArrayPattern' - ? create_array_snapshot_mapper(destructured_pattern) - : create_object_snapshot_mapper(); - - collection = b.call('$.snapshot_each_value', collection, mapper); + if (destructured_pattern.type === 'ArrayPattern') { + // For array destructuring, destructure immediately during iteration + const indices = []; + for (let i = 0; i < destructured_pattern.elements.length; i++) { + const element = destructured_pattern.elements[i]; + if (element && element.type !== 'RestElement') { + indices.push(i); + } + } + collection = b.call( + '$.to_array_destructured', + collection, + b.array(indices.map((i) => b.literal(i))) + ); + } else { + // For object destructuring, we still need to snapshot + collection = b.call('$.snapshot_each_value', collection, create_object_snapshot_mapper()); + } } const index = each_node_meta.contains_group_binding || !node.index ? each_node_meta.index : b.id(node.index); @@ -102,19 +114,6 @@ function get_destructured_pattern(pattern) { return null; } -/** - * @param {import('estree').ArrayPattern} pattern - */ -function create_array_snapshot_mapper(pattern) { - const value = b.id('$$value'); - const has_rest = pattern.elements.some((element) => element?.type === 'RestElement'); - - return b.arrow( - [value], - b.call('$.snapshot_array', value, b.literal(pattern.elements.length), has_rest ? b.true : b.false) - ); -} - function create_object_snapshot_mapper() { const value = b.id('$$value'); return b.arrow([value], b.call('$.snapshot_object', value)); diff --git a/packages/svelte/src/internal/client/index.js b/packages/svelte/src/internal/client/index.js index 2d5379de9b..037592c145 100644 --- a/packages/svelte/src/internal/client/index.js +++ b/packages/svelte/src/internal/client/index.js @@ -170,7 +170,7 @@ export { } from './dom/operations.js'; export { attr, clsx } from '../shared/attributes.js'; export { snapshot } from '../shared/clone.js'; -export { noop, fallback, to_array, snapshot_array, snapshot_each_value, snapshot_object } from '../shared/utils.js'; +export { noop, fallback, to_array, to_array_destructured, snapshot_each_value, snapshot_object } from '../shared/utils.js'; export { invalid_default_snippet, validate_dynamic_element_tag, diff --git a/packages/svelte/src/internal/server/index.js b/packages/svelte/src/internal/server/index.js index 6f2e495006..a85f9e68bc 100644 --- a/packages/svelte/src/internal/server/index.js +++ b/packages/svelte/src/internal/server/index.js @@ -454,7 +454,7 @@ export { push_element, pop_element, validate_snippet_args } from './dev.js'; export { snapshot } from '../shared/clone.js'; -export { fallback, to_array, snapshot_array, snapshot_each_value, snapshot_object } from '../shared/utils.js'; +export { fallback, to_array, to_array_destructured, snapshot_each_value, snapshot_object } from '../shared/utils.js'; export { invalid_default_snippet, diff --git a/packages/svelte/src/internal/shared/utils.js b/packages/svelte/src/internal/shared/utils.js index 15f54ea0da..655d714833 100644 --- a/packages/svelte/src/internal/shared/utils.js +++ b/packages/svelte/src/internal/shared/utils.js @@ -117,9 +117,55 @@ export function to_array(value, n) { return array; } +/** + * Convert an iterable to an array, immediately destructuring array elements + * at the specified indices. This ensures that when a generator yields the same + * array object multiple times (mutating it), we capture the values at iteration + * time, matching for...of behavior. + * + * Returns an array where each element is a new array containing the destructured + * values, so that extract_paths can process them correctly. + * @template T + * @param {ArrayLike | Iterable | null | undefined} collection + * @param {number[]} destructure_indices - Array indices to extract from each element + * @returns {Array} + */ +export function to_array_destructured(collection, destructure_indices) { + if (collection == null) { + return []; + } + + const result = []; + + // Helper to destructure a single element + const destructure_element = (element) => { + const destructured = []; + for (let j = 0; j < destructure_indices.length; j++) { + destructured.push(element?.[destructure_indices[j]]); + } + return destructured; + }; + + // If already an array, destructure each element immediately + if (is_array(collection)) { + for (let i = 0; i < collection.length; i++) { + result.push(destructure_element(collection[i])); + } + return result; + } + + // For iterables, destructure during iteration + for (const element of collection) { + result.push(destructure_element(element)); + } + + return result; +} + /** * Snapshot items produced by an iterator so that destructured values reflect * what was yielded before the iterator mutates the value again. + * Used for object destructuring where we need to shallow copy the object. * @template T * @param {ArrayLike | Iterable | null | undefined} collection * @param {(value: T) => T} mapper @@ -133,17 +179,6 @@ export function snapshot_each_value(collection, mapper) { return is_array(collection) ? collection : array_from(collection, mapper); } -/** - * @param {any} value - * @param {number} length - * @param {boolean} has_rest - * @returns {any[]} - */ -export function snapshot_array(value, length, has_rest) { - const array = to_array(value, has_rest ? undefined : length); - return array.slice(); -} - /** * @param {any} value */