Skip to content

Commit 81a34aa

Browse files
Rich-Harrisdummdidumm
Rich-Harris
and
dummdidumm
authored
fix: handle derived destructured iterators (#16015)
* revert #15813 * failing test * tweak * tweak * WIP * WIP * WIP * WIP * WIP * working * tweak * changeset * tweak description * Update packages/svelte/src/compiler/utils/ast.js Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com> --------- Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>
1 parent 2e27c5d commit 81a34aa

File tree

16 files changed

+348
-238
lines changed

16 files changed

+348
-238
lines changed

‎.changeset/purple-dryers-mate.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'svelte': patch
3+
---
4+
5+
fix: handle derived destructured iterators

‎packages/svelte/src/compiler/migrate/index.js‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -603,15 +603,15 @@ const instance_script = {
603603
);
604604
// Turn export let into props. It's really really weird because export let { x: foo, z: [bar]} = ..
605605
// means that foo and bar are the props (i.e. the leafs are the prop names), not x and z.
606-
// const tmp = state.scope.generate('tmp');
607-
// const paths = extract_paths(declarator.id);
606+
// const tmp = b.id(state.scope.generate('tmp'));
607+
// const paths = extract_paths(declarator.id, tmp);
608608
// state.props_pre.push(
609-
// b.declaration('const', b.id(tmp), visit(declarator.init!) as Expression)
609+
// b.declaration('const', tmp, visit(declarator.init!) as Expression)
610610
// );
611611
// for (const path of paths) {
612612
// const name = (path.node as Identifier).name;
613613
// const binding = state.scope.get(name)!;
614-
// const value = path.expression!(b.id(tmp));
614+
// const value = path.expression;
615615
// if (binding.kind === 'bindable_prop' || binding.kind === 'rest_prop') {
616616
// state.props.push({
617617
// local: name,

‎packages/svelte/src/compiler/phases/2-analyze/visitors/VariableDeclarator.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import * as e from '../../../errors.js';
77
import * as w from '../../../warnings.js';
88
import { extract_paths } from '../../../utils/ast.js';
99
import { equal } from '../../../utils/assert.js';
10+
import * as b from '#compiler/builders';
1011

1112
/**
1213
* @param {VariableDeclarator} node
@@ -18,7 +19,7 @@ export function VariableDeclarator(node, context) {
1819
if (context.state.analysis.runes) {
1920
const init = node.init;
2021
const rune = get_rune(init, context.state.scope);
21-
const paths = extract_paths(node.id);
22+
const { paths } = extract_paths(node.id, b.id('dummy'));
2223

2324
for (const path of paths) {
2425
validate_identifier_name(context.state.scope.get(/** @type {Identifier} */ (path.node).name));

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

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -234,13 +234,21 @@ export function EachBlock(node, context) {
234234
} else if (node.context) {
235235
const unwrapped = (flags & EACH_ITEM_REACTIVE) !== 0 ? b.call('$.get', item) : item;
236236

237-
for (const path of extract_paths(node.context)) {
237+
const { inserts, paths } = extract_paths(node.context, unwrapped);
238+
239+
for (const { id, value } of inserts) {
240+
id.name = context.state.scope.generate('$$array');
241+
child_state.transform[id.name] = { read: get_value };
242+
243+
const expression = /** @type {Expression} */ (context.visit(b.thunk(value), child_state));
244+
declarations.push(b.var(id, b.call('$.derived', expression)));
245+
}
246+
247+
for (const path of paths) {
238248
const name = /** @type {Identifier} */ (path.node).name;
239249
const needs_derived = path.has_default_value; // to ensure that default value is only called once
240250

241-
const fn = b.thunk(
242-
/** @type {Expression} */ (context.visit(path.expression?.(unwrapped), child_state))
243-
);
251+
const fn = b.thunk(/** @type {Expression} */ (context.visit(path.expression, child_state)));
244252

245253
declarations.push(b.let(path.node, needs_derived ? b.call('$.derived_safe_equal', fn) : fn));
246254

@@ -249,7 +257,7 @@ export function EachBlock(node, context) {
249257
child_state.transform[name] = {
250258
read,
251259
assign: (_, value) => {
252-
const left = /** @type {Pattern} */ (path.update_expression(unwrapped));
260+
const left = /** @type {Pattern} */ (path.update_expression);
253261
return b.sequence([b.assignment('=', left, value), ...sequence]);
254262
},
255263
mutate: (_, mutation) => {

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

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,14 +43,21 @@ export function SnippetBlock(node, context) {
4343
let arg_alias = `$$arg${i}`;
4444
args.push(b.id(arg_alias));
4545

46-
const paths = extract_paths(argument);
46+
const { inserts, paths } = extract_paths(argument, b.maybe_call(b.id(arg_alias)));
47+
48+
for (const { id, value } of inserts) {
49+
id.name = context.state.scope.generate('$$array');
50+
transform[id.name] = { read: get_value };
51+
52+
declarations.push(
53+
b.var(id, b.call('$.derived', /** @type {Expression} */ (context.visit(b.thunk(value)))))
54+
);
55+
}
4756

4857
for (const path of paths) {
4958
const name = /** @type {Identifier} */ (path.node).name;
5059
const needs_derived = path.has_default_value; // to ensure that default value is only called once
51-
const fn = b.thunk(
52-
/** @type {Expression} */ (context.visit(path.expression?.(b.maybe_call(b.id(arg_alias)))))
53-
);
60+
const fn = b.thunk(/** @type {Expression} */ (context.visit(path.expression)));
5461

5562
declarations.push(b.let(path.node, needs_derived ? b.call('$.derived_safe_equal', fn) : fn));
5663

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

Lines changed: 90 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,13 @@
22
/** @import { Binding } from '#compiler' */
33
/** @import { ComponentClientTransformState, ComponentContext } from '../types' */
44
import { dev } from '../../../../state.js';
5-
import { build_pattern, extract_paths } from '../../../../utils/ast.js';
5+
import { extract_paths } from '../../../../utils/ast.js';
66
import * as b from '#compiler/builders';
77
import * as assert from '../../../../utils/assert.js';
88
import { get_rune } from '../../../scope.js';
99
import { get_prop_source, is_prop_source, is_state_source, should_proxy } from '../utils.js';
1010
import { is_hoisted_function } from '../../utils.js';
11+
import { get_value } from './shared/declarations.js';
1112

1213
/**
1314
* @param {VariableDeclaration} node
@@ -116,7 +117,7 @@ export function VariableDeclaration(node, context) {
116117
}
117118

118119
const args = /** @type {CallExpression} */ (init).arguments;
119-
const value = args.length > 0 ? /** @type {Expression} */ (context.visit(args[0])) : b.void0;
120+
const value = /** @type {Expression} */ (args[0]) ?? b.void0; // TODO do we need the void 0? can we just omit it altogether?
120121

121122
if (rune === '$state' || rune === '$state.raw') {
122123
/**
@@ -137,24 +138,34 @@ export function VariableDeclaration(node, context) {
137138
};
138139

139140
if (declarator.id.type === 'Identifier') {
141+
const expression = /** @type {Expression} */ (context.visit(value));
142+
140143
declarations.push(
141-
b.declarator(declarator.id, create_state_declarator(declarator.id, value))
144+
b.declarator(declarator.id, create_state_declarator(declarator.id, expression))
142145
);
143146
} else {
144-
const [pattern, replacements] = build_pattern(declarator.id, context.state.scope);
147+
const tmp = b.id(context.state.scope.generate('tmp'));
148+
const { inserts, paths } = extract_paths(declarator.id, tmp);
149+
145150
declarations.push(
146-
b.declarator(pattern, value),
147-
.../** @type {[Identifier, Identifier][]} */ ([...replacements]).map(
148-
([original, replacement]) => {
149-
const binding = context.state.scope.get(original.name);
150-
return b.declarator(
151-
original,
152-
binding?.kind === 'state' || binding?.kind === 'raw_state'
153-
? create_state_declarator(binding.node, replacement)
154-
: replacement
155-
);
156-
}
157-
)
151+
b.declarator(tmp, value),
152+
...inserts.map(({ id, value }) => {
153+
id.name = context.state.scope.generate('$$array');
154+
context.state.transform[id.name] = { read: get_value };
155+
156+
const expression = /** @type {Expression} */ (context.visit(b.thunk(value)));
157+
return b.declarator(id, b.call('$.derived', expression));
158+
}),
159+
...paths.map((path) => {
160+
const value = /** @type {Expression} */ (context.visit(path.expression));
161+
const binding = context.state.scope.get(/** @type {Identifier} */ (path.node).name);
162+
return b.declarator(
163+
path.node,
164+
binding?.kind === 'state' || binding?.kind === 'raw_state'
165+
? create_state_declarator(binding.node, value)
166+
: value
167+
);
168+
})
158169
);
159170
}
160171

@@ -163,44 +174,41 @@ export function VariableDeclaration(node, context) {
163174

164175
if (rune === '$derived' || rune === '$derived.by') {
165176
if (declarator.id.type === 'Identifier') {
166-
declarations.push(
167-
b.declarator(
168-
declarator.id,
169-
b.call('$.derived', rune === '$derived.by' ? value : b.thunk(value))
170-
)
171-
);
177+
let expression = /** @type {Expression} */ (context.visit(value));
178+
if (rune === '$derived') expression = b.thunk(expression);
179+
180+
declarations.push(b.declarator(declarator.id, b.call('$.derived', expression)));
172181
} else {
173-
const [pattern, replacements] = build_pattern(declarator.id, context.state.scope);
174182
const init = /** @type {CallExpression} */ (declarator.init);
175183

176-
/** @type {Identifier} */
177-
let id;
178184
let rhs = value;
179185

180-
if (rune === '$derived' && init.arguments[0].type === 'Identifier') {
181-
id = init.arguments[0];
182-
} else {
183-
id = b.id(context.state.scope.generate('$$d'));
186+
if (rune !== '$derived' || init.arguments[0].type !== 'Identifier') {
187+
const id = b.id(context.state.scope.generate('$$d'));
184188
rhs = b.call('$.get', id);
185189

186-
declarations.push(
187-
b.declarator(id, b.call('$.derived', rune === '$derived.by' ? value : b.thunk(value)))
188-
);
190+
let expression = /** @type {Expression} */ (context.visit(value));
191+
if (rune === '$derived') expression = b.thunk(expression);
192+
193+
declarations.push(b.declarator(id, b.call('$.derived', expression)));
189194
}
190195

191-
for (let i = 0; i < replacements.size; i++) {
192-
const [original, replacement] = [...replacements][i];
193-
declarations.push(
194-
b.declarator(
195-
original,
196-
b.call(
197-
'$.derived',
198-
b.arrow([], b.block([b.let(pattern, rhs), b.return(replacement)]))
199-
)
200-
)
201-
);
196+
const { inserts, paths } = extract_paths(declarator.id, rhs);
197+
198+
for (const { id, value } of inserts) {
199+
id.name = context.state.scope.generate('$$array');
200+
context.state.transform[id.name] = { read: get_value };
201+
202+
const expression = /** @type {Expression} */ (context.visit(b.thunk(value)));
203+
declarations.push(b.declarator(id, b.call('$.derived', expression)));
204+
}
205+
206+
for (const path of paths) {
207+
const expression = /** @type {Expression} */ (context.visit(path.expression));
208+
declarations.push(b.declarator(path.node, b.call('$.derived', b.thunk(expression))));
202209
}
203210
}
211+
204212
continue;
205213
}
206214
}
@@ -229,20 +237,29 @@ export function VariableDeclaration(node, context) {
229237
if (declarator.id.type !== 'Identifier') {
230238
// Turn export let into props. It's really really weird because export let { x: foo, z: [bar]} = ..
231239
// means that foo and bar are the props (i.e. the leafs are the prop names), not x and z.
232-
const tmp = context.state.scope.generate('tmp');
233-
const paths = extract_paths(declarator.id);
240+
const tmp = b.id(context.state.scope.generate('tmp'));
241+
const { inserts, paths } = extract_paths(declarator.id, tmp);
234242

235243
declarations.push(
236244
b.declarator(
237-
b.id(tmp),
245+
tmp,
238246
/** @type {Expression} */ (context.visit(/** @type {Expression} */ (declarator.init)))
239247
)
240248
);
241249

250+
for (const { id, value } of inserts) {
251+
id.name = context.state.scope.generate('$$array');
252+
context.state.transform[id.name] = { read: get_value };
253+
254+
const expression = /** @type {Expression} */ (context.visit(b.thunk(value)));
255+
declarations.push(b.declarator(id, b.call('$.derived', expression)));
256+
}
257+
242258
for (const path of paths) {
243259
const name = /** @type {Identifier} */ (path.node).name;
244260
const binding = /** @type {Binding} */ (context.state.scope.get(name));
245-
const value = path.expression?.(b.id(tmp));
261+
const value = /** @type {Expression} */ (context.visit(path.expression));
262+
246263
declarations.push(
247264
b.declarator(
248265
path.node,
@@ -276,7 +293,7 @@ export function VariableDeclaration(node, context) {
276293
declarations.push(
277294
...create_state_declarators(
278295
declarator,
279-
context.state,
296+
context,
280297
/** @type {Expression} */ (declarator.init && context.visit(declarator.init))
281298
)
282299
);
@@ -296,32 +313,41 @@ export function VariableDeclaration(node, context) {
296313
/**
297314
* Creates the output for a state declaration in legacy mode.
298315
* @param {VariableDeclarator} declarator
299-
* @param {ComponentClientTransformState} scope
316+
* @param {ComponentContext} context
300317
* @param {Expression} value
301318
*/
302-
function create_state_declarators(declarator, { scope, analysis }, value) {
319+
function create_state_declarators(declarator, context, value) {
303320
if (declarator.id.type === 'Identifier') {
304321
return [
305322
b.declarator(
306323
declarator.id,
307-
b.call('$.mutable_source', value, analysis.immutable ? b.true : undefined)
324+
b.call('$.mutable_source', value, context.state.analysis.immutable ? b.true : undefined)
308325
)
309326
];
310327
}
311328

312-
const [pattern, replacements] = build_pattern(declarator.id, scope);
329+
const tmp = b.id(context.state.scope.generate('tmp'));
330+
const { inserts, paths } = extract_paths(declarator.id, tmp);
331+
313332
return [
314-
b.declarator(pattern, value),
315-
.../** @type {[Identifier, Identifier][]} */ ([...replacements]).map(
316-
([original, replacement]) => {
317-
const binding = scope.get(original.name);
318-
return b.declarator(
319-
original,
320-
binding?.kind === 'state'
321-
? b.call('$.mutable_source', replacement, analysis.immutable ? b.true : undefined)
322-
: replacement
323-
);
324-
}
325-
)
333+
b.declarator(tmp, value),
334+
...inserts.map(({ id, value }) => {
335+
id.name = context.state.scope.generate('$$array');
336+
context.state.transform[id.name] = { read: get_value };
337+
338+
const expression = /** @type {Expression} */ (context.visit(b.thunk(value)));
339+
return b.declarator(id, b.call('$.derived', expression));
340+
}),
341+
...paths.map((path) => {
342+
const value = /** @type {Expression} */ (context.visit(path.expression));
343+
const binding = context.state.scope.get(/** @type {Identifier} */ (path.node).name);
344+
345+
return b.declarator(
346+
path.node,
347+
binding?.kind === 'state'
348+
? b.call('$.mutable_source', value, context.state.analysis.immutable ? b.true : undefined)
349+
: value
350+
);
351+
})
326352
];
327353
}

0 commit comments

Comments
 (0)