Skip to content

Commit 8348132

Browse files
committed
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
1 parent 0529a50 commit 8348132

File tree

5 files changed

+85
-51
lines changed

5 files changed

+85
-51
lines changed

packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,25 @@ export function EachBlock(node, context) {
4141
const destructured_pattern = get_destructured_pattern(node.context);
4242

4343
if (destructured_pattern) {
44-
const mapper =
45-
destructured_pattern.type === 'ArrayPattern'
46-
? create_array_snapshot_mapper(destructured_pattern)
47-
: create_object_snapshot_mapper();
48-
49-
collection = b.call('$.snapshot_each_value', collection, mapper);
44+
if (destructured_pattern.type === 'ArrayPattern') {
45+
// For array destructuring, we need to destructure immediately during iteration
46+
// to match for...of behavior, capturing values before the generator mutates them
47+
const indices = [];
48+
for (let i = 0; i < destructured_pattern.elements.length; i++) {
49+
const element = destructured_pattern.elements[i];
50+
if (element && element.type !== 'RestElement') {
51+
indices.push(i);
52+
}
53+
}
54+
collection = b.call(
55+
'$.to_array_destructured',
56+
collection,
57+
b.array(indices.map((i) => b.literal(i)))
58+
);
59+
} else {
60+
// For object destructuring, we still need to snapshot to capture values
61+
collection = b.call('$.snapshot_each_value', collection, create_object_snapshot_mapper());
62+
}
5063
}
5164

5265
if (!each_node_meta.is_controlled) {
@@ -390,19 +403,6 @@ function get_destructured_pattern(pattern) {
390403
return null;
391404
}
392405

393-
/**
394-
* @param {import('estree').ArrayPattern} pattern
395-
*/
396-
function create_array_snapshot_mapper(pattern) {
397-
const value = b.id('$$value');
398-
const has_rest = pattern.elements.some((element) => element?.type === 'RestElement');
399-
400-
return b.arrow(
401-
[value],
402-
b.call('$.snapshot_array', value, b.literal(pattern.elements.length), has_rest ? b.true : b.false)
403-
);
404-
}
405-
406406
function create_object_snapshot_mapper() {
407407
const value = b.id('$$value');
408408
return b.arrow([value], b.call('$.snapshot_object', value));

packages/svelte/src/compiler/phases/3-transform/server/visitors/EachBlock.js

Lines changed: 18 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,24 @@ export function EachBlock(node, context) {
1616
const destructured_pattern = get_destructured_pattern(node.context);
1717

1818
if (destructured_pattern) {
19-
const mapper =
20-
destructured_pattern.type === 'ArrayPattern'
21-
? create_array_snapshot_mapper(destructured_pattern)
22-
: create_object_snapshot_mapper();
23-
24-
collection = b.call('$.snapshot_each_value', collection, mapper);
19+
if (destructured_pattern.type === 'ArrayPattern') {
20+
// For array destructuring, destructure immediately during iteration
21+
const indices = [];
22+
for (let i = 0; i < destructured_pattern.elements.length; i++) {
23+
const element = destructured_pattern.elements[i];
24+
if (element && element.type !== 'RestElement') {
25+
indices.push(i);
26+
}
27+
}
28+
collection = b.call(
29+
'$.to_array_destructured',
30+
collection,
31+
b.array(indices.map((i) => b.literal(i)))
32+
);
33+
} else {
34+
// For object destructuring, we still need to snapshot
35+
collection = b.call('$.snapshot_each_value', collection, create_object_snapshot_mapper());
36+
}
2537
}
2638
const index =
2739
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) {
102114
return null;
103115
}
104116

105-
/**
106-
* @param {import('estree').ArrayPattern} pattern
107-
*/
108-
function create_array_snapshot_mapper(pattern) {
109-
const value = b.id('$$value');
110-
const has_rest = pattern.elements.some((element) => element?.type === 'RestElement');
111-
112-
return b.arrow(
113-
[value],
114-
b.call('$.snapshot_array', value, b.literal(pattern.elements.length), has_rest ? b.true : b.false)
115-
);
116-
}
117-
118117
function create_object_snapshot_mapper() {
119118
const value = b.id('$$value');
120119
return b.arrow([value], b.call('$.snapshot_object', value));

packages/svelte/src/internal/client/index.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ export {
170170
} from './dom/operations.js';
171171
export { attr, clsx } from '../shared/attributes.js';
172172
export { snapshot } from '../shared/clone.js';
173-
export { noop, fallback, to_array, snapshot_array, snapshot_each_value, snapshot_object } from '../shared/utils.js';
173+
export { noop, fallback, to_array, to_array_destructured, snapshot_each_value, snapshot_object } from '../shared/utils.js';
174174
export {
175175
invalid_default_snippet,
176176
validate_dynamic_element_tag,

packages/svelte/src/internal/server/index.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -454,7 +454,7 @@ export { push_element, pop_element, validate_snippet_args } from './dev.js';
454454

455455
export { snapshot } from '../shared/clone.js';
456456

457-
export { fallback, to_array, snapshot_array, snapshot_each_value, snapshot_object } from '../shared/utils.js';
457+
export { fallback, to_array, to_array_destructured, snapshot_each_value, snapshot_object } from '../shared/utils.js';
458458

459459
export {
460460
invalid_default_snippet,

packages/svelte/src/internal/shared/utils.js

Lines changed: 46 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -117,9 +117,55 @@ export function to_array(value, n) {
117117
return array;
118118
}
119119

120+
/**
121+
* Convert an iterable to an array, immediately destructuring array elements
122+
* at the specified indices. This ensures that when a generator yields the same
123+
* array object multiple times (mutating it), we capture the values at iteration
124+
* time, matching for...of behavior.
125+
*
126+
* Returns an array where each element is a new array containing the destructured
127+
* values, so that extract_paths can process them correctly.
128+
* @template T
129+
* @param {ArrayLike<T> | Iterable<T> | null | undefined} collection
130+
* @param {number[]} destructure_indices - Array indices to extract from each element
131+
* @returns {Array<any[]>}
132+
*/
133+
export function to_array_destructured(collection, destructure_indices) {
134+
if (collection == null) {
135+
return [];
136+
}
137+
138+
const result = [];
139+
140+
// Helper to destructure a single element
141+
const destructure_element = (element) => {
142+
const destructured = [];
143+
for (let j = 0; j < destructure_indices.length; j++) {
144+
destructured.push(element?.[destructure_indices[j]]);
145+
}
146+
return destructured;
147+
};
148+
149+
// If already an array, destructure each element immediately
150+
if (is_array(collection)) {
151+
for (let i = 0; i < collection.length; i++) {
152+
result.push(destructure_element(collection[i]));
153+
}
154+
return result;
155+
}
156+
157+
// For iterables, destructure during iteration
158+
for (const element of collection) {
159+
result.push(destructure_element(element));
160+
}
161+
162+
return result;
163+
}
164+
120165
/**
121166
* Snapshot items produced by an iterator so that destructured values reflect
122167
* what was yielded before the iterator mutates the value again.
168+
* Used for object destructuring where we need to shallow copy the object.
123169
* @template T
124170
* @param {ArrayLike<T> | Iterable<T> | null | undefined} collection
125171
* @param {(value: T) => T} mapper
@@ -133,17 +179,6 @@ export function snapshot_each_value(collection, mapper) {
133179
return is_array(collection) ? collection : array_from(collection, mapper);
134180
}
135181

136-
/**
137-
* @param {any} value
138-
* @param {number} length
139-
* @param {boolean} has_rest
140-
* @returns {any[]}
141-
*/
142-
export function snapshot_array(value, length, has_rest) {
143-
const array = to_array(value, has_rest ? undefined : length);
144-
return array.slice();
145-
}
146-
147182
/**
148183
* @param {any} value
149184
*/

0 commit comments

Comments
 (0)