Skip to content

Commit 0529a50

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

File tree

7 files changed

+151
-5
lines changed

7 files changed

+151
-5
lines changed

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

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ export function EachBlock(node, context) {
2929
scope: /** @type {Scope} */ (context.state.scope.parent)
3030
};
3131

32-
const collection = build_expression(
32+
let collection = build_expression(
3333
{
3434
...context,
3535
state: parent_scope_state
@@ -38,6 +38,17 @@ export function EachBlock(node, context) {
3838
node.metadata.expression
3939
);
4040

41+
const destructured_pattern = get_destructured_pattern(node.context);
42+
43+
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);
50+
}
51+
4152
if (!each_node_meta.is_controlled) {
4253
context.state.template.push_comment();
4354
}
@@ -365,3 +376,34 @@ export function EachBlock(node, context) {
365376
function collect_parent_each_blocks(context) {
366377
return /** @type {AST.EachBlock[]} */ (context.path.filter((node) => node.type === 'EachBlock'));
367378
}
379+
380+
/**
381+
* @param {import('estree').Pattern | null | undefined} pattern
382+
* @returns {import('estree').ArrayPattern | import('estree').ObjectPattern | null}
383+
*/
384+
function get_destructured_pattern(pattern) {
385+
if (!pattern) return null;
386+
if (pattern.type === 'ArrayPattern' || pattern.type === 'ObjectPattern') {
387+
return pattern;
388+
}
389+
390+
return null;
391+
}
392+
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+
406+
function create_object_snapshot_mapper() {
407+
const value = b.id('$$value');
408+
return b.arrow([value], b.call('$.snapshot_object', value));
409+
}

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

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/** @import { BlockStatement, Expression, Statement } from 'estree' */
1+
/** @import { BlockStatement, Expression, Pattern, Statement } from 'estree' */
22
/** @import { AST } from '#compiler' */
33
/** @import { ComponentContext } from '../types.js' */
44
import * as b from '#compiler/builders';
@@ -12,7 +12,17 @@ export function EachBlock(node, context) {
1212
const state = context.state;
1313

1414
const each_node_meta = node.metadata;
15-
const collection = /** @type {Expression} */ (context.visit(node.expression));
15+
let collection = /** @type {Expression} */ (context.visit(node.expression));
16+
const destructured_pattern = get_destructured_pattern(node.context);
17+
18+
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);
25+
}
1626
const index =
1727
each_node_meta.contains_group_binding || !node.index ? each_node_meta.index : b.id(node.index);
1828

@@ -78,3 +88,34 @@ export function EachBlock(node, context) {
7888
state.template.push(...block.body, block_close);
7989
}
8090
}
91+
92+
/**
93+
* @param {Pattern | null} pattern
94+
* @returns {import('estree').ArrayPattern | import('estree').ObjectPattern | null}
95+
*/
96+
function get_destructured_pattern(pattern) {
97+
if (!pattern) return null;
98+
if (pattern.type === 'ArrayPattern' || pattern.type === 'ObjectPattern') {
99+
return pattern;
100+
}
101+
102+
return null;
103+
}
104+
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+
118+
function create_object_snapshot_mapper() {
119+
const value = b.id('$$value');
120+
return b.arrow([value], b.call('$.snapshot_object', value));
121+
}

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 } from '../shared/utils.js';
173+
export { noop, fallback, to_array, snapshot_array, 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 } from '../shared/utils.js';
457+
export { fallback, to_array, snapshot_array, 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: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,3 +116,37 @@ export function to_array(value, n) {
116116

117117
return array;
118118
}
119+
120+
/**
121+
* Snapshot items produced by an iterator so that destructured values reflect
122+
* what was yielded before the iterator mutates the value again.
123+
* @template T
124+
* @param {ArrayLike<T> | Iterable<T> | null | undefined} collection
125+
* @param {(value: T) => T} mapper
126+
* @returns {Array<T>}
127+
*/
128+
export function snapshot_each_value(collection, mapper) {
129+
if (collection == null) {
130+
return [];
131+
}
132+
133+
return is_array(collection) ? collection : array_from(collection, mapper);
134+
}
135+
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+
147+
/**
148+
* @param {any} value
149+
*/
150+
export function snapshot_object(value) {
151+
return value == null || typeof value !== 'object' ? value : { ...value };
152+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import { test } from '../../test';
2+
3+
export default test({
4+
html: `
5+
<p>0</p>
6+
<p>1</p>
7+
<p>2</p>
8+
<p>3</p>
9+
<p>4</p>
10+
`
11+
});
12+
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<svelte:options runes />
2+
3+
<script>
4+
function* gen() {
5+
const arr = [0];
6+
7+
for (let i = 0; i < 5; i += 1) {
8+
arr[0] = i;
9+
yield arr;
10+
}
11+
}
12+
</script>
13+
14+
{#each gen() as [item]}
15+
<p>{item}</p>
16+
{/each}
17+

0 commit comments

Comments
 (0)