Skip to content

Commit aa3f002

Browse files
paoloricciutipaoloricciuti
authored
feat: better migration of single-assignment labeled statements (#13461)
fixes #13460 fixes #13459
1 parent 7d47269 commit aa3f002

File tree

4 files changed

+255
-11
lines changed

4 files changed

+255
-11
lines changed

‎.changeset/great-dots-wonder.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+
feat: support migration of single assignment labeled statements

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

Lines changed: 135 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/** @import { VariableDeclarator, Node, Identifier } from 'estree' */
1+
/** @import { VariableDeclarator, Node, Identifier, AssignmentExpression, LabeledStatement, ExpressionStatement } from 'estree' */
22
/** @import { Visitors } from 'zimmerframe' */
33
/** @import { ComponentAnalysis } from '../phases/types.js' */
44
/** @import { Scope, ScopeRoot } from '../phases/scope.js' */
@@ -10,7 +10,7 @@ import { regex_valid_component_name } from '../phases/1-parse/state/element.js';
1010
import { analyze_component } from '../phases/2-analyze/index.js';
1111
import { get_rune } from '../phases/scope.js';
1212
import { reset, reset_warning_filter } from '../state.js';
13-
import { extract_identifiers } from '../utils/ast.js';
13+
import { extract_identifiers, extract_all_identifiers_from_expression } from '../utils/ast.js';
1414
import { migrate_svelte_ignore } from '../utils/extract_svelte_ignore.js';
1515
import { validate_component_options } from '../validate-options.js';
1616
import { is_svg, is_void } from '../../utils.js';
@@ -90,7 +90,8 @@ export function migrate(source) {
9090
},
9191
legacy_imports: new Set(),
9292
script_insertions: new Set(),
93-
derived_components: new Map()
93+
derived_components: new Map(),
94+
derived_labeled_statements: new Set()
9495
};
9596

9697
if (parsed.module) {
@@ -301,7 +302,8 @@ export function migrate(source) {
301302
* names: Record<string, string>;
302303
* legacy_imports: Set<string>;
303304
* script_insertions: Set<string>;
304-
* derived_components: Map<string, string>
305+
* derived_components: Map<string, string>,
306+
* derived_labeled_statements: Set<LabeledStatement>
305307
* }} State
306308
*/
307309

@@ -349,7 +351,7 @@ const instance_script = {
349351
state.str.remove(/** @type {number} */ (node.start), /** @type {number} */ (node.end));
350352
}
351353
},
352-
VariableDeclaration(node, { state, path }) {
354+
VariableDeclaration(node, { state, path, visit }) {
353355
if (state.scope !== state.analysis.instance.scope) {
354356
return;
355357
}
@@ -470,10 +472,118 @@ const instance_script = {
470472
state.str.prependLeft(start, '$state(');
471473
state.str.appendRight(end, ')');
472474
} else {
473-
state.str.prependLeft(
474-
/** @type {number} */ (declarator.id.typeAnnotation?.end ?? declarator.id.end),
475-
' = $state()'
475+
/**
476+
* @type {AssignmentExpression | undefined}
477+
*/
478+
let assignment_in_labeled;
479+
/**
480+
* @type {LabeledStatement | undefined}
481+
*/
482+
let labeled_statement;
483+
484+
// Analyze declaration bindings to see if they're exclusively updated within a single reactive statement
485+
const possible_derived = bindings.every((binding) =>
486+
binding.references.every((reference) => {
487+
const declaration = reference.path.find((el) => el.type === 'VariableDeclaration');
488+
const assignment = reference.path.find((el) => el.type === 'AssignmentExpression');
489+
const update = reference.path.find((el) => el.type === 'UpdateExpression');
490+
const labeled = reference.path.find(
491+
(el) => el.type === 'LabeledStatement' && el.label.name === '$'
492+
);
493+
494+
if (assignment && labeled) {
495+
if (assignment_in_labeled) return false;
496+
assignment_in_labeled = /** @type {AssignmentExpression} */ (assignment);
497+
labeled_statement = /** @type {LabeledStatement} */ (labeled);
498+
}
499+
500+
return !update && (declaration || (labeled && assignment) || (!labeled && !assignment));
501+
})
476502
);
503+
504+
const labeled_has_single_assignment =
505+
labeled_statement?.body.type === 'BlockStatement' &&
506+
labeled_statement.body.body.length === 1;
507+
508+
const is_expression_assignment =
509+
labeled_statement?.body.type === 'ExpressionStatement' &&
510+
labeled_statement.body.expression.type === 'AssignmentExpression';
511+
512+
let should_be_state = false;
513+
514+
if (is_expression_assignment) {
515+
const body = /**@type {ExpressionStatement}*/ (labeled_statement?.body);
516+
const expression = /**@type {AssignmentExpression}*/ (body.expression);
517+
const [, ids] = extract_all_identifiers_from_expression(expression.right);
518+
if (ids.length === 0) {
519+
should_be_state = true;
520+
state.derived_labeled_statements.add(
521+
/** @type {LabeledStatement} */ (labeled_statement)
522+
);
523+
}
524+
}
525+
526+
if (
527+
!should_be_state &&
528+
possible_derived &&
529+
assignment_in_labeled &&
530+
labeled_statement &&
531+
(labeled_has_single_assignment || is_expression_assignment)
532+
) {
533+
// Someone wrote a `$: { ... }` statement which we can turn into a `$derived`
534+
state.str.appendRight(
535+
/** @type {number} */ (declarator.id.typeAnnotation?.end ?? declarator.id.end),
536+
' = $derived('
537+
);
538+
visit(assignment_in_labeled.right);
539+
state.str.appendRight(
540+
/** @type {number} */ (declarator.id.typeAnnotation?.end ?? declarator.id.end),
541+
state.str
542+
.snip(
543+
/** @type {number} */ (assignment_in_labeled.right.start),
544+
/** @type {number} */ (assignment_in_labeled.right.end)
545+
)
546+
.toString()
547+
);
548+
state.str.remove(
549+
/** @type {number} */ (labeled_statement.start),
550+
/** @type {number} */ (labeled_statement.end)
551+
);
552+
state.str.appendRight(
553+
/** @type {number} */ (declarator.id.typeAnnotation?.end ?? declarator.id.end),
554+
')'
555+
);
556+
state.derived_labeled_statements.add(labeled_statement);
557+
} else {
558+
state.str.prependLeft(
559+
/** @type {number} */ (declarator.id.typeAnnotation?.end ?? declarator.id.end),
560+
' = $state('
561+
);
562+
if (should_be_state) {
563+
// someone wrote a `$: foo = ...` statement which we can turn into `let foo = $state(...)`
564+
state.str.appendRight(
565+
/** @type {number} */ (declarator.id.typeAnnotation?.end ?? declarator.id.end),
566+
state.str
567+
.snip(
568+
/** @type {number} */ (
569+
/** @type {AssignmentExpression} */ (assignment_in_labeled).right.start
570+
),
571+
/** @type {number} */ (
572+
/** @type {AssignmentExpression} */ (assignment_in_labeled).right.end
573+
)
574+
)
575+
.toString()
576+
);
577+
state.str.remove(
578+
/** @type {number} */ (/** @type {LabeledStatement} */ (labeled_statement).start),
579+
/** @type {number} */ (/** @type {LabeledStatement} */ (labeled_statement).end)
580+
);
581+
}
582+
state.str.appendRight(
583+
/** @type {number} */ (declarator.id.typeAnnotation?.end ?? declarator.id.end),
584+
')'
585+
);
586+
}
477587
}
478588
}
479589

@@ -504,6 +614,7 @@ const instance_script = {
504614
if (state.analysis.runes) return;
505615
if (path.length > 1) return;
506616
if (node.label.name !== '$') return;
617+
if (state.derived_labeled_statements.has(node)) return;
507618

508619
next();
509620

@@ -512,6 +623,9 @@ const instance_script = {
512623
node.body.expression.type === 'AssignmentExpression'
513624
) {
514625
const ids = extract_identifiers(node.body.expression.left);
626+
const [, expression_ids] = extract_all_identifiers_from_expression(
627+
node.body.expression.right
628+
);
515629
const bindings = ids.map((id) => state.scope.get(id.name));
516630
const reassigned_bindings = bindings.filter((b) => b?.reassigned);
517631
if (reassigned_bindings.length === 0 && !bindings.some((b) => b?.kind === 'store_sub')) {
@@ -542,14 +656,24 @@ const instance_script = {
542656
return;
543657
} else {
544658
for (const binding of reassigned_bindings) {
545-
if (binding && ids.includes(binding.node)) {
659+
if (binding && (ids.includes(binding.node) || expression_ids.length === 0)) {
660+
const init =
661+
binding.kind === 'state'
662+
? ' = $state()'
663+
: expression_ids.length === 0
664+
? ` = $state(${state.str.original.substring(/** @type {number} */ (node.body.expression.right.start), node.body.expression.right.end)})`
665+
: '';
546666
// implicitly-declared variable which we need to make explicit
547-
state.str.prependRight(
667+
state.str.prependLeft(
548668
/** @type {number} */ (node.start),
549-
`let ${binding.node.name}${binding.kind === 'state' ? ' = $state()' : ''};\n${state.indent}`
669+
`let ${binding.node.name}${init};\n${state.indent}`
550670
);
551671
}
552672
}
673+
if (expression_ids.length === 0 && !bindings.some((b) => b?.kind === 'store_sub')) {
674+
state.str.remove(/** @type {number} */ (node.start), /** @type {number} */ (node.end));
675+
return;
676+
}
553677
}
554678
}
555679

‎packages/svelte/tests/migrate/samples/single-assignment-labeled/input.svelte

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
<script>
2+
let count = 0;
3+
let double;
4+
$:{
5+
double = count * 2;
6+
}
7+
8+
let quadruple;
9+
$:{
10+
quadruple = count * 4;
11+
console.log("i have a side effect")
12+
}
13+
14+
let eight_times;
15+
$:{
16+
// updated
17+
eight_times = count * 8;
18+
}
19+
20+
let sixteen_times;
21+
$:{
22+
// reassigned outside labeled statement
23+
sixteen_times = count * 16;
24+
}
25+
26+
let alot_times;
27+
$:{
28+
// reassigned in multiple labeled
29+
alot_times = count * 32;
30+
}
31+
$:{
32+
// reassigned in multiple labeled
33+
alot_times = count * 32;
34+
}
35+
36+
let evenmore;
37+
let evenmore_doubled;
38+
$:{
39+
// multiple stuff in label
40+
evenmore = count * 64;
41+
evenmore_doubled = evenmore * 2;
42+
}
43+
44+
let almost_infinity;
45+
$: almost_infinity = count * 128;
46+
47+
let should_be_state;
48+
$: should_be_state = 42;
49+
$: should_be_state_too = 42;
50+
</script>
51+
52+
<button on:click={()=>{
53+
count++;
54+
eight_times++;
55+
sixteen_times += 1;
56+
should_be_state_too++;
57+
}}>click</button>

‎packages/svelte/tests/migrate/samples/single-assignment-labeled/output.svelte

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
<script>
2+
import { run } from 'svelte/legacy';
3+
4+
let count = $state(0);
5+
let double = $derived(count * 2);
6+
7+
8+
let quadruple = $state();
9+
run(() => {
10+
quadruple = count * 4;
11+
console.log("i have a side effect")
12+
});
13+
14+
let eight_times = $state();
15+
run(() => {
16+
// updated
17+
eight_times = count * 8;
18+
});
19+
20+
let sixteen_times = $state();
21+
run(() => {
22+
// reassigned outside labeled statement
23+
sixteen_times = count * 16;
24+
});
25+
26+
let alot_times = $state();
27+
run(() => {
28+
// reassigned in multiple labeled
29+
alot_times = count * 32;
30+
});
31+
run(() => {
32+
// reassigned in multiple labeled
33+
alot_times = count * 32;
34+
});
35+
36+
let evenmore = $state();
37+
let evenmore_doubled = $state();
38+
run(() => {
39+
// multiple stuff in label
40+
evenmore = count * 64;
41+
evenmore_doubled = evenmore * 2;
42+
});
43+
44+
let almost_infinity = $derived(count * 128);
45+
46+
47+
let should_be_state = $state(42);
48+
49+
let should_be_state_too = $state(42);
50+
51+
</script>
52+
53+
<button onclick={()=>{
54+
count++;
55+
eight_times++;
56+
sixteen_times += 1;
57+
should_be_state_too++;
58+
}}>click</button>

0 commit comments

Comments
 (0)