Skip to content

Commit

Permalink
feat: better migration of single-assignment labeled statements (#13461)
Browse files Browse the repository at this point in the history
fixes #13460
fixes #13459
  • Loading branch information
paoloricciuti authored Oct 4, 2024
1 parent 7d47269 commit aa3f002
Show file tree
Hide file tree
Showing 4 changed files with 255 additions and 11 deletions.
5 changes: 5 additions & 0 deletions .changeset/great-dots-wonder.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

feat: support migration of single assignment labeled statements
146 changes: 135 additions & 11 deletions packages/svelte/src/compiler/migrate/index.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/** @import { VariableDeclarator, Node, Identifier } from 'estree' */
/** @import { VariableDeclarator, Node, Identifier, AssignmentExpression, LabeledStatement, ExpressionStatement } from 'estree' */
/** @import { Visitors } from 'zimmerframe' */
/** @import { ComponentAnalysis } from '../phases/types.js' */
/** @import { Scope, ScopeRoot } from '../phases/scope.js' */
Expand All @@ -10,7 +10,7 @@ import { regex_valid_component_name } from '../phases/1-parse/state/element.js';
import { analyze_component } from '../phases/2-analyze/index.js';
import { get_rune } from '../phases/scope.js';
import { reset, reset_warning_filter } from '../state.js';
import { extract_identifiers } from '../utils/ast.js';
import { extract_identifiers, extract_all_identifiers_from_expression } from '../utils/ast.js';
import { migrate_svelte_ignore } from '../utils/extract_svelte_ignore.js';
import { validate_component_options } from '../validate-options.js';
import { is_svg, is_void } from '../../utils.js';
Expand Down Expand Up @@ -90,7 +90,8 @@ export function migrate(source) {
},
legacy_imports: new Set(),
script_insertions: new Set(),
derived_components: new Map()
derived_components: new Map(),
derived_labeled_statements: new Set()
};

if (parsed.module) {
Expand Down Expand Up @@ -301,7 +302,8 @@ export function migrate(source) {
* names: Record<string, string>;
* legacy_imports: Set<string>;
* script_insertions: Set<string>;
* derived_components: Map<string, string>
* derived_components: Map<string, string>,
* derived_labeled_statements: Set<LabeledStatement>
* }} State
*/

Expand Down Expand Up @@ -349,7 +351,7 @@ const instance_script = {
state.str.remove(/** @type {number} */ (node.start), /** @type {number} */ (node.end));
}
},
VariableDeclaration(node, { state, path }) {
VariableDeclaration(node, { state, path, visit }) {
if (state.scope !== state.analysis.instance.scope) {
return;
}
Expand Down Expand Up @@ -470,10 +472,118 @@ const instance_script = {
state.str.prependLeft(start, '$state(');
state.str.appendRight(end, ')');
} else {
state.str.prependLeft(
/** @type {number} */ (declarator.id.typeAnnotation?.end ?? declarator.id.end),
' = $state()'
/**
* @type {AssignmentExpression | undefined}
*/
let assignment_in_labeled;
/**
* @type {LabeledStatement | undefined}
*/
let labeled_statement;

// Analyze declaration bindings to see if they're exclusively updated within a single reactive statement
const possible_derived = bindings.every((binding) =>
binding.references.every((reference) => {
const declaration = reference.path.find((el) => el.type === 'VariableDeclaration');
const assignment = reference.path.find((el) => el.type === 'AssignmentExpression');
const update = reference.path.find((el) => el.type === 'UpdateExpression');
const labeled = reference.path.find(
(el) => el.type === 'LabeledStatement' && el.label.name === '$'
);

if (assignment && labeled) {
if (assignment_in_labeled) return false;
assignment_in_labeled = /** @type {AssignmentExpression} */ (assignment);
labeled_statement = /** @type {LabeledStatement} */ (labeled);
}

return !update && (declaration || (labeled && assignment) || (!labeled && !assignment));
})
);

const labeled_has_single_assignment =
labeled_statement?.body.type === 'BlockStatement' &&
labeled_statement.body.body.length === 1;

const is_expression_assignment =
labeled_statement?.body.type === 'ExpressionStatement' &&
labeled_statement.body.expression.type === 'AssignmentExpression';

let should_be_state = false;

if (is_expression_assignment) {
const body = /**@type {ExpressionStatement}*/ (labeled_statement?.body);
const expression = /**@type {AssignmentExpression}*/ (body.expression);
const [, ids] = extract_all_identifiers_from_expression(expression.right);
if (ids.length === 0) {
should_be_state = true;
state.derived_labeled_statements.add(
/** @type {LabeledStatement} */ (labeled_statement)
);
}
}

if (
!should_be_state &&
possible_derived &&
assignment_in_labeled &&
labeled_statement &&
(labeled_has_single_assignment || is_expression_assignment)
) {
// Someone wrote a `$: { ... }` statement which we can turn into a `$derived`
state.str.appendRight(
/** @type {number} */ (declarator.id.typeAnnotation?.end ?? declarator.id.end),
' = $derived('
);
visit(assignment_in_labeled.right);
state.str.appendRight(
/** @type {number} */ (declarator.id.typeAnnotation?.end ?? declarator.id.end),
state.str
.snip(
/** @type {number} */ (assignment_in_labeled.right.start),
/** @type {number} */ (assignment_in_labeled.right.end)
)
.toString()
);
state.str.remove(
/** @type {number} */ (labeled_statement.start),
/** @type {number} */ (labeled_statement.end)
);
state.str.appendRight(
/** @type {number} */ (declarator.id.typeAnnotation?.end ?? declarator.id.end),
')'
);
state.derived_labeled_statements.add(labeled_statement);
} else {
state.str.prependLeft(
/** @type {number} */ (declarator.id.typeAnnotation?.end ?? declarator.id.end),
' = $state('
);
if (should_be_state) {
// someone wrote a `$: foo = ...` statement which we can turn into `let foo = $state(...)`
state.str.appendRight(
/** @type {number} */ (declarator.id.typeAnnotation?.end ?? declarator.id.end),
state.str
.snip(
/** @type {number} */ (
/** @type {AssignmentExpression} */ (assignment_in_labeled).right.start
),
/** @type {number} */ (
/** @type {AssignmentExpression} */ (assignment_in_labeled).right.end
)
)
.toString()
);
state.str.remove(
/** @type {number} */ (/** @type {LabeledStatement} */ (labeled_statement).start),
/** @type {number} */ (/** @type {LabeledStatement} */ (labeled_statement).end)
);
}
state.str.appendRight(
/** @type {number} */ (declarator.id.typeAnnotation?.end ?? declarator.id.end),
')'
);
}
}
}

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

next();

Expand All @@ -512,6 +623,9 @@ const instance_script = {
node.body.expression.type === 'AssignmentExpression'
) {
const ids = extract_identifiers(node.body.expression.left);
const [, expression_ids] = extract_all_identifiers_from_expression(
node.body.expression.right
);
const bindings = ids.map((id) => state.scope.get(id.name));
const reassigned_bindings = bindings.filter((b) => b?.reassigned);
if (reassigned_bindings.length === 0 && !bindings.some((b) => b?.kind === 'store_sub')) {
Expand Down Expand Up @@ -542,14 +656,24 @@ const instance_script = {
return;
} else {
for (const binding of reassigned_bindings) {
if (binding && ids.includes(binding.node)) {
if (binding && (ids.includes(binding.node) || expression_ids.length === 0)) {
const init =
binding.kind === 'state'
? ' = $state()'
: expression_ids.length === 0
? ` = $state(${state.str.original.substring(/** @type {number} */ (node.body.expression.right.start), node.body.expression.right.end)})`
: '';
// implicitly-declared variable which we need to make explicit
state.str.prependRight(
state.str.prependLeft(
/** @type {number} */ (node.start),
`let ${binding.node.name}${binding.kind === 'state' ? ' = $state()' : ''};\n${state.indent}`
`let ${binding.node.name}${init};\n${state.indent}`
);
}
}
if (expression_ids.length === 0 && !bindings.some((b) => b?.kind === 'store_sub')) {
state.str.remove(/** @type {number} */ (node.start), /** @type {number} */ (node.end));
return;
}
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
<script>
let count = 0;
let double;
$:{
double = count * 2;
}
let quadruple;
$:{
quadruple = count * 4;
console.log("i have a side effect")
}
let eight_times;
$:{
// updated
eight_times = count * 8;
}
let sixteen_times;
$:{
// reassigned outside labeled statement
sixteen_times = count * 16;
}
let alot_times;
$:{
// reassigned in multiple labeled
alot_times = count * 32;
}
$:{
// reassigned in multiple labeled
alot_times = count * 32;
}
let evenmore;
let evenmore_doubled;
$:{
// multiple stuff in label
evenmore = count * 64;
evenmore_doubled = evenmore * 2;
}
let almost_infinity;
$: almost_infinity = count * 128;
let should_be_state;
$: should_be_state = 42;
$: should_be_state_too = 42;
</script>

<button on:click={()=>{
count++;
eight_times++;
sixteen_times += 1;
should_be_state_too++;
}}>click</button>
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
<script>
import { run } from 'svelte/legacy';
let count = $state(0);
let double = $derived(count * 2);
let quadruple = $state();
run(() => {
quadruple = count * 4;
console.log("i have a side effect")
});
let eight_times = $state();
run(() => {
// updated
eight_times = count * 8;
});
let sixteen_times = $state();
run(() => {
// reassigned outside labeled statement
sixteen_times = count * 16;
});
let alot_times = $state();
run(() => {
// reassigned in multiple labeled
alot_times = count * 32;
});
run(() => {
// reassigned in multiple labeled
alot_times = count * 32;
});
let evenmore = $state();
let evenmore_doubled = $state();
run(() => {
// multiple stuff in label
evenmore = count * 64;
evenmore_doubled = evenmore * 2;
});
let almost_infinity = $derived(count * 128);
let should_be_state = $state(42);
let should_be_state_too = $state(42);
</script>

<button onclick={()=>{
count++;
eight_times++;
sixteen_times += 1;
should_be_state_too++;
}}>click</button>

0 comments on commit aa3f002

Please sign in to comment.