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 c0bfe272e5ee..0b13f52fae2f 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 3c0a8c167696..e684aa717d35 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 a2add3ec5978..2d5379de9bae 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 c0dbdbda14f6..6f2e4950066a 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 10f8597520d9..15f54ea0da47 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 000000000000..31f0f8ff9841 --- /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 000000000000..75a795bfeb7f --- /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 0b13f52fae2f..c9c94096f47b 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 e684aa717d35..1430c1071c32 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 2d5379de9bae..037592c1459d 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 6f2e4950066a..a85f9e68bca9 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 15f54ea0da47..655d71483346 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 */