From f8bb4a35779f445af5ff942d1caaad0b2d99a8da Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Thu, 10 Apr 2025 12:33:19 +0200 Subject: [PATCH 1/4] fix: don't transform reassigned state in labeled statement in `$derived` --- .changeset/weak-doors-yell.md | 5 + packages/svelte/src/compiler/migrate/index.js | 92 ++++++++++--------- .../input.svelte | 6 ++ .../output.svelte | 10 ++ 4 files changed, 68 insertions(+), 45 deletions(-) create mode 100644 .changeset/weak-doors-yell.md create mode 100644 packages/svelte/tests/migrate/samples/labeled-statement-reassign-state/input.svelte create mode 100644 packages/svelte/tests/migrate/samples/labeled-statement-reassign-state/output.svelte diff --git a/.changeset/weak-doors-yell.md b/.changeset/weak-doors-yell.md new file mode 100644 index 000000000000..1b21783435a4 --- /dev/null +++ b/.changeset/weak-doors-yell.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: don't transform reassigned state in labeled statement in `$derived` diff --git a/packages/svelte/src/compiler/migrate/index.js b/packages/svelte/src/compiler/migrate/index.js index 9d79d88b2397..b32217954bcf 100644 --- a/packages/svelte/src/compiler/migrate/index.js +++ b/packages/svelte/src/compiler/migrate/index.js @@ -951,58 +951,60 @@ const instance_script = { const bindings = ids.map((id) => state.scope.get(id.name)); const reassigned_bindings = bindings.filter((b) => b?.reassigned); - if ( - node.body.expression.right.type !== 'Literal' && - !bindings.some((b) => b?.kind === 'store_sub') && - node.body.expression.left.type !== 'MemberExpression' - ) { - let { start, end } = /** @type {{ start: number, end: number }} */ ( - node.body.expression.right - ); + if (!bindings.some((b) => b?.kind === 'state')) { + if ( + node.body.expression.right.type !== 'Literal' && + !bindings.some((b) => b?.kind === 'store_sub') && + node.body.expression.left.type !== 'MemberExpression' + ) { + let { start, end } = /** @type {{ start: number, end: number }} */ ( + node.body.expression.right + ); - check_rune_binding('derived'); + check_rune_binding('derived'); - // $derived - state.str.update( - /** @type {number} */ (node.start), - /** @type {number} */ (node.body.expression.start), - 'let ' - ); + // $derived + state.str.update( + /** @type {number} */ (node.start), + /** @type {number} */ (node.body.expression.start), + 'let ' + ); - if (node.body.expression.right.type === 'SequenceExpression') { - while (state.str.original[start] !== '(') start -= 1; - while (state.str.original[end - 1] !== ')') end += 1; - } + if (node.body.expression.right.type === 'SequenceExpression') { + while (state.str.original[start] !== '(') start -= 1; + while (state.str.original[end - 1] !== ')') end += 1; + } - state.str.prependRight(start, `$derived(`); + state.str.prependRight(start, `$derived(`); - // in a case like `$: ({ a } = b())`, there's already a trailing parenthesis. - // otherwise, we need to add one - if (state.str.original[/** @type {number} */ (node.body.start)] !== '(') { - state.str.appendLeft(end, `)`); - } - - return; - } else { - for (const binding of reassigned_bindings) { - if (binding && (ids.includes(binding.node) || expression_ids.length === 0)) { - check_rune_binding('state'); - 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.prependLeft( - /** @type {number} */ (node.start), - `let ${binding.node.name}${init};\n${state.indent}` - ); + // in a case like `$: ({ a } = b())`, there's already a trailing parenthesis. + // otherwise, we need to add one + if (state.str.original[/** @type {number} */ (node.body.start)] !== '(') { + state.str.appendLeft(end, `)`); } - } - if (expression_ids.length === 0 && !bindings.some((b) => b?.kind === 'store_sub')) { - state.str.remove(/** @type {number} */ (node.start), /** @type {number} */ (node.end)); + return; + } else { + for (const binding of reassigned_bindings) { + if (binding && (ids.includes(binding.node) || expression_ids.length === 0)) { + check_rune_binding('state'); + 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.prependLeft( + /** @type {number} */ (node.start), + `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; + } } } } diff --git a/packages/svelte/tests/migrate/samples/labeled-statement-reassign-state/input.svelte b/packages/svelte/tests/migrate/samples/labeled-statement-reassign-state/input.svelte new file mode 100644 index 000000000000..0b5c13d8898b --- /dev/null +++ b/packages/svelte/tests/migrate/samples/labeled-statement-reassign-state/input.svelte @@ -0,0 +1,6 @@ + \ No newline at end of file diff --git a/packages/svelte/tests/migrate/samples/labeled-statement-reassign-state/output.svelte b/packages/svelte/tests/migrate/samples/labeled-statement-reassign-state/output.svelte new file mode 100644 index 000000000000..c2b36a6e3028 --- /dev/null +++ b/packages/svelte/tests/migrate/samples/labeled-statement-reassign-state/output.svelte @@ -0,0 +1,10 @@ + \ No newline at end of file From 6403a25fc17fcfabff20b42c1c8f36f0cbdba34b Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 10 Apr 2025 18:06:00 -0400 Subject: [PATCH 2/4] fix type so optional chaining is unnecessary --- packages/svelte/src/compiler/migrate/index.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/svelte/src/compiler/migrate/index.js b/packages/svelte/src/compiler/migrate/index.js index b32217954bcf..0a99b166f919 100644 --- a/packages/svelte/src/compiler/migrate/index.js +++ b/packages/svelte/src/compiler/migrate/index.js @@ -948,13 +948,13 @@ const instance_script = { 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); + const bindings = ids.map((id) => /** @type {Binding} */ (state.scope.get(id.name))); + const reassigned_bindings = bindings.filter((b) => b.reassigned); - if (!bindings.some((b) => b?.kind === 'state')) { + if (!bindings.some((b) => b.kind === 'state')) { if ( node.body.expression.right.type !== 'Literal' && - !bindings.some((b) => b?.kind === 'store_sub') && + !bindings.some((b) => b.kind === 'store_sub') && node.body.expression.left.type !== 'MemberExpression' ) { let { start, end } = /** @type {{ start: number, end: number }} */ ( @@ -1001,7 +1001,7 @@ const instance_script = { ); } } - if (expression_ids.length === 0 && !bindings.some((b) => b?.kind === 'store_sub')) { + if (expression_ids.length === 0 && !bindings.some((b) => b.kind === 'store_sub')) { state.str.remove(/** @type {number} */ (node.start), /** @type {number} */ (node.end)); return; } From 43a60fdc956d87ce83a070fd7bfbe3955f7ac1ec Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 10 Apr 2025 18:09:11 -0400 Subject: [PATCH 3/4] drive-by tidy up --- packages/svelte/src/compiler/migrate/index.js | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/packages/svelte/src/compiler/migrate/index.js b/packages/svelte/src/compiler/migrate/index.js index 0a99b166f919..ebd2a3068ed2 100644 --- a/packages/svelte/src/compiler/migrate/index.js +++ b/packages/svelte/src/compiler/migrate/index.js @@ -944,22 +944,20 @@ const instance_script = { node.body.type === 'ExpressionStatement' && 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 { left, right } = node.body.expression; + + const ids = extract_identifiers(left); + const [, expression_ids] = extract_all_identifiers_from_expression(right); const bindings = ids.map((id) => /** @type {Binding} */ (state.scope.get(id.name))); const reassigned_bindings = bindings.filter((b) => b.reassigned); - if (!bindings.some((b) => b.kind === 'state')) { + if (bindings.every((b) => b.kind === 'legacy_reactive')) { if ( - node.body.expression.right.type !== 'Literal' && - !bindings.some((b) => b.kind === 'store_sub') && - node.body.expression.left.type !== 'MemberExpression' + right.type !== 'Literal' && + bindings.every((b) => b.kind !== 'store_sub') && + left.type !== 'MemberExpression' ) { - let { start, end } = /** @type {{ start: number, end: number }} */ ( - node.body.expression.right - ); + let { start, end } = /** @type {{ start: number, end: number }} */ (right); check_rune_binding('derived'); @@ -970,7 +968,7 @@ const instance_script = { 'let ' ); - if (node.body.expression.right.type === 'SequenceExpression') { + if (right.type === 'SequenceExpression') { while (state.str.original[start] !== '(') start -= 1; while (state.str.original[end - 1] !== ')') end += 1; } @@ -992,7 +990,7 @@ const instance_script = { 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)})` + ? ` = $state(${state.str.original.substring(/** @type {number} */ (right.start), right.end)})` : ''; // implicitly-declared variable which we need to make explicit state.str.prependLeft( From 2090823c33df6cc41bd8c8f87e58e46bc6459f6d Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 10 Apr 2025 18:11:34 -0400 Subject: [PATCH 4/4] drive-by tidy up --- packages/svelte/src/compiler/migrate/index.js | 42 +++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/packages/svelte/src/compiler/migrate/index.js b/packages/svelte/src/compiler/migrate/index.js index ebd2a3068ed2..523389a25aef 100644 --- a/packages/svelte/src/compiler/migrate/index.js +++ b/packages/svelte/src/compiler/migrate/index.js @@ -949,7 +949,6 @@ const instance_script = { const ids = extract_identifiers(left); const [, expression_ids] = extract_all_identifiers_from_expression(right); const bindings = ids.map((id) => /** @type {Binding} */ (state.scope.get(id.name))); - const reassigned_bindings = bindings.filter((b) => b.reassigned); if (bindings.every((b) => b.kind === 'legacy_reactive')) { if ( @@ -982,28 +981,29 @@ const instance_script = { } return; - } else { - for (const binding of reassigned_bindings) { - if (binding && (ids.includes(binding.node) || expression_ids.length === 0)) { - check_rune_binding('state'); - const init = - binding.kind === 'state' - ? ' = $state()' - : expression_ids.length === 0 - ? ` = $state(${state.str.original.substring(/** @type {number} */ (right.start), right.end)})` - : ''; - // implicitly-declared variable which we need to make explicit - state.str.prependLeft( - /** @type {number} */ (node.start), - `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; + } + + for (const binding of bindings) { + if (binding.reassigned && (ids.includes(binding.node) || expression_ids.length === 0)) { + check_rune_binding('state'); + const init = + binding.kind === 'state' + ? ' = $state()' + : expression_ids.length === 0 + ? ` = $state(${state.str.original.substring(/** @type {number} */ (right.start), right.end)})` + : ''; + // implicitly-declared variable which we need to make explicit + state.str.prependLeft( + /** @type {number} */ (node.start), + `let ${binding.node.name}${init};\n${state.indent}` + ); } } + + if (expression_ids.length === 0 && bindings.every((b) => b.kind !== 'store_sub')) { + state.str.remove(/** @type {number} */ (node.start), /** @type {number} */ (node.end)); + return; + } } }