From 4fcd1cb1edab45b5240e5881335bf39976242ce9 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Thu, 8 Feb 2024 17:22:16 +0000 Subject: [PATCH] fix: add compiler error for each block mutations in runes mode (#10428) * fix: improve code generation for mutation to each block reference * fix: add compiler error for each block mutations in runes mode * lint * lint * lint * use existing validate_assignment logic * assignment, not mutation --------- Co-authored-by: Rich Harris --- .changeset/violet-pigs-jam.md | 5 +++ packages/svelte/src/compiler/errors.js | 3 +- .../compiler/phases/2-analyze/validation.js | 34 ++++++++++--------- .../runes-invalid-each-mutation/_config.js | 8 +++++ .../runes-invalid-each-mutation/main.svelte | 7 ++++ 5 files changed, 40 insertions(+), 17 deletions(-) create mode 100644 .changeset/violet-pigs-jam.md create mode 100644 packages/svelte/tests/compiler-errors/samples/runes-invalid-each-mutation/_config.js create mode 100644 packages/svelte/tests/compiler-errors/samples/runes-invalid-each-mutation/main.svelte diff --git a/.changeset/violet-pigs-jam.md b/.changeset/violet-pigs-jam.md new file mode 100644 index 0000000000..260cbbfae1 --- /dev/null +++ b/.changeset/violet-pigs-jam.md @@ -0,0 +1,5 @@ +--- +"svelte": patch +--- + +fix: add compiler error for each block mutations in runes mode diff --git a/packages/svelte/src/compiler/errors.js b/packages/svelte/src/compiler/errors.js index 414f5e3bd2..6588e71d97 100644 --- a/packages/svelte/src/compiler/errors.js +++ b/packages/svelte/src/compiler/errors.js @@ -206,7 +206,8 @@ const runes = { }`, /** @param {string} name */ 'invalid-runes-mode-import': (name) => `${name} cannot be used in runes mode`, - 'duplicate-props-rune': () => `Cannot use $props() more than once` + 'duplicate-props-rune': () => `Cannot use $props() more than once`, + 'invalid-each-assignment': () => `Cannot reassign each block argument in runes mode` }; /** @satisfies {Errors} */ diff --git a/packages/svelte/src/compiler/phases/2-analyze/validation.js b/packages/svelte/src/compiler/phases/2-analyze/validation.js index 1855079ef0..ca6bc00efe 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/validation.js +++ b/packages/svelte/src/compiler/phases/2-analyze/validation.js @@ -334,6 +334,9 @@ function is_tag_valid_with_parent(tag, parent_tag) { * @type {import('zimmerframe').Visitors} */ const validation = { + AssignmentExpression(node, context) { + validate_assignment(node, node.left, context.state); + }, BindDirective(node, context) { validate_no_const_assignment(node, node.expression, context.state.scope, true); @@ -655,6 +658,9 @@ const validation = { error(child, 'invalid-title-content'); } }, + UpdateExpression(node, context) { + validate_assignment(node, node.argument, context.state); + }, ExpressionTag(node, context) { if (!node.parent) return; if (context.state.parent_element) { @@ -908,24 +914,28 @@ function validate_no_const_assignment(node, argument, scope, is_binding) { function validate_assignment(node, argument, state) { validate_no_const_assignment(node, argument, state.scope, false); - let left = /** @type {import('estree').Expression | import('estree').Super} */ (argument); - - if (left.type === 'Identifier') { - const binding = state.scope.get(left.name); + if (state.analysis.runes && argument.type === 'Identifier') { + const binding = state.scope.get(argument.name); if (binding?.kind === 'derived') { error(node, 'invalid-derived-assignment'); } + + if (binding?.kind === 'each') { + error(node, 'invalid-each-assignment'); + } } + let object = /** @type {import('estree').Expression | import('estree').Super} */ (argument); + /** @type {import('estree').Expression | import('estree').PrivateIdentifier | null} */ let property = null; - while (left.type === 'MemberExpression') { - property = left.property; - left = left.object; + while (object.type === 'MemberExpression') { + property = object.property; + object = object.object; } - if (left.type === 'ThisExpression' && property?.type === 'PrivateIdentifier') { + if (object.type === 'ThisExpression' && property?.type === 'PrivateIdentifier') { if (state.private_derived_state.includes(property.name)) { error(node, 'invalid-derived-assignment'); } @@ -933,14 +943,6 @@ function validate_assignment(node, argument, state) { } export const validation_runes = merge(validation, a11y_validators, { - AssignmentExpression(node, { state, path }) { - const parent = path.at(-1); - if (parent && parent.type === 'ConstTag') return; - validate_assignment(node, node.left, state); - }, - UpdateExpression(node, { state }) { - validate_assignment(node, node.argument, state); - }, LabeledStatement(node, { path }) { if (node.label.name !== '$' || path.at(-1)?.type !== 'Program') return; error(node, 'invalid-legacy-reactive-statement'); diff --git a/packages/svelte/tests/compiler-errors/samples/runes-invalid-each-mutation/_config.js b/packages/svelte/tests/compiler-errors/samples/runes-invalid-each-mutation/_config.js new file mode 100644 index 0000000000..eee6ca5c8d --- /dev/null +++ b/packages/svelte/tests/compiler-errors/samples/runes-invalid-each-mutation/_config.js @@ -0,0 +1,8 @@ +import { test } from '../../test'; + +export default test({ + error: { + code: 'invalid-each-assignment', + message: 'Cannot reassign each block argument in runes mode' + } +}); diff --git a/packages/svelte/tests/compiler-errors/samples/runes-invalid-each-mutation/main.svelte b/packages/svelte/tests/compiler-errors/samples/runes-invalid-each-mutation/main.svelte new file mode 100644 index 0000000000..f9744c08de --- /dev/null +++ b/packages/svelte/tests/compiler-errors/samples/runes-invalid-each-mutation/main.svelte @@ -0,0 +1,7 @@ + + +{#each arr as value} + +{/each}