From 134d43539e7c722669bd33a911cd95639c3af6ef Mon Sep 17 00:00:00 2001 From: "S. Elliott Johnson" Date: Wed, 23 Apr 2025 00:19:08 -0600 Subject: [PATCH 1/5] feat: State declarations in class constructors --- .../2-analyze/visitors/CallExpression.js | 100 +++++++++++++++++- 1 file changed, 97 insertions(+), 3 deletions(-) diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js index 904817b014e4..683e343aa56a 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js @@ -116,9 +116,11 @@ export function CallExpression(node, context) { case '$derived': case '$derived.by': if ( - (parent.type !== 'VariableDeclarator' || - get_parent(context.path, -3).type === 'ConstTag') && - !(parent.type === 'PropertyDefinition' && !parent.static && !parent.computed) + !( + call_expression_is_variable_declaration(parent, context) || + call_expression_is_class_property_definition(parent) || + call_expression_is_valid_class_property_assignment_in_constructor(parent, context) + ) ) { e.state_invalid_placement(node, rune); } @@ -270,3 +272,95 @@ function get_function_label(nodes) { return parent.id.name; } } + +/** + * + * @param {AST.SvelteNode} parent + * @param {Context} context + */ +function call_expression_is_variable_declaration(parent, context) { + return parent.type === 'VariableDeclarator' && get_parent(context.path, -3).type !== 'ConstTag'; +} + +/** + * + * @param {AST.SvelteNode} parent + */ +function call_expression_is_class_property_definition(parent) { + return parent.type === 'PropertyDefinition' && !parent.static && !parent.computed; +} + +/** + * + * @param {AST.SvelteNode} parent + * @param {Context} context + * @returns + */ +function call_expression_is_valid_class_property_assignment_in_constructor(parent, context) { + return ( + expression_is_assignment_to_top_level_property_of_this(parent) && + current_node_is_in_constructor_root_or_control_flow_blocks(context) + ); +} + +/** + * yes: + * - `this.foo = bar` + * + * no: + * - `this = bar` + * - `this.foo.baz = bar` + * - `anything_other_than_this = bar` + * + * @param {AST.SvelteNode} node + */ +function expression_is_assignment_to_top_level_property_of_this(node) { + return ( + node.type === 'AssignmentExpression' && + node.operator === '=' && + node.left.type === 'MemberExpression' && + node.left.object.type === 'ThisExpression' && + node.left.property.type === 'Identifier' + ); +} + +/** + * @param {AST.SvelteNode} node + */ +function node_is_constructor(node) { + return ( + node.type === 'MethodDefinition' && + node.key.type === 'Identifier' && + node.key.name === 'constructor' + ); +} + +// if blocks are just IfStatements with BlockStatements or other IfStatements as consequents +const allowed_parent_types = new Set([ + 'IfStatement', + 'BlockStatement', + 'SwitchCase', + 'SwitchStatement' +]); + +/** + * Succeeds if the node's only direct parents are `if` / `else if` / `else` blocks _and_ + * those blocks are the direct children of the constructor. + * + * @param {Context} context + */ +function current_node_is_in_constructor_root_or_control_flow_blocks(context) { + let parent_index = -3; // this gets us from CallExpression -> AssignmentExpression -> ExpressionStatement -> Whatever is here + while (true) { + const grandparent = get_parent(context.path, parent_index - 1); + const parent = get_parent(context.path, parent_index); + if (grandparent && node_is_constructor(grandparent)) { + // if this is the case then `parent` is the FunctionExpression + return true; + } + if (!allowed_parent_types.has(parent.type)) { + return false; + } + parent_index--; + } +} From fb8d6d7975ff53e833779cb0ac8d3e117f768fb8 Mon Sep 17 00:00:00 2001 From: "S. Elliott Johnson" Date: Thu, 24 Apr 2025 15:02:43 -0600 Subject: [PATCH 2/5] feat: Analysis phase --- .../98-reference/.generated/compile-errors.md | 33 +++- .../svelte/messages/compile-errors/script.md | 31 +++- packages/svelte/src/compiler/errors.js | 15 +- .../src/compiler/phases/2-analyze/index.js | 10 +- .../src/compiler/phases/2-analyze/types.d.ts | 5 +- .../visitors/AssignmentExpression.js | 1 + .../2-analyze/visitors/CallExpression.js | 80 +--------- .../phases/2-analyze/visitors/ClassBody.js | 30 ---- .../2-analyze/visitors/ClassDeclaration.js | 3 +- .../2-analyze/visitors/PropertyDefinition.js | 12 ++ .../visitors/shared/class-analysis.js | 147 ++++++++++++++++++ .../class-state-field-static/_config.js | 2 +- .../samples/runes-no-rune-each/_config.js | 3 +- .../runes-wrong-derived-placement/_config.js | 2 +- .../runes-wrong-state-placement/_config.js | 3 +- .../const-tag-invalid-rune-usage/errors.json | 2 +- 16 files changed, 257 insertions(+), 122 deletions(-) delete mode 100644 packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js create mode 100644 packages/svelte/src/compiler/phases/2-analyze/visitors/PropertyDefinition.js create mode 100644 packages/svelte/src/compiler/phases/2-analyze/visitors/shared/class-analysis.js diff --git a/documentation/docs/98-reference/.generated/compile-errors.md b/documentation/docs/98-reference/.generated/compile-errors.md index e8669ead533d..fb0591b884a5 100644 --- a/documentation/docs/98-reference/.generated/compile-errors.md +++ b/documentation/docs/98-reference/.generated/compile-errors.md @@ -208,6 +208,37 @@ Cannot assign to %thing% Cannot bind to %thing% ``` +### constructor_state_reassignment + +``` +Cannot redeclare stateful field `%name%` in the constructor. The field was originally declared here: `%original_location%` +``` + +To create stateful class fields in the constructor, the rune assignment must be the _first_ assignment to the class field. +Assignments thereafter must not use the rune. + +```ts +constructor() { + this.count = $state(0); + this.count = $state(1); // invalid, assigning to the same property with `$state` again +} + +constructor() { + this.count = $state(0); + this.count = $state.raw(1); // invalid, assigning to the same property with a different rune +} + +constructor() { + this.count = 0; + this.count = $state(1); // invalid, this property was created as a regular property, not state +} + +constructor() { + this.count = $state(0); + this.count = 1; // valid, this is setting the state that has already been declared +} +``` + ### css_empty_declaration ``` @@ -855,7 +886,7 @@ Cannot export state from a module if it is reassigned. Either export a function ### state_invalid_placement ``` -`%rune%(...)` can only be used as a variable declaration initializer or a class field +`%rune%(...)` can only be used as a variable declaration initializer, a class field declaration, or the first assignment to a class field at the top level of the constructor. ``` ### store_invalid_scoped_subscription diff --git a/packages/svelte/messages/compile-errors/script.md b/packages/svelte/messages/compile-errors/script.md index aabcbeae4812..780c9f4d2863 100644 --- a/packages/svelte/messages/compile-errors/script.md +++ b/packages/svelte/messages/compile-errors/script.md @@ -10,6 +10,35 @@ > Cannot bind to %thing% +## constructor_state_reassignment + +> Cannot redeclare stateful field `%name%` in the constructor. The field was originally declared here: `%original_location%` + +To create stateful class fields in the constructor, the rune assignment must be the _first_ assignment to the class field. +Assignments thereafter must not use the rune. + +```ts +constructor() { + this.count = $state(0); + this.count = $state(1); // invalid, assigning to the same property with `$state` again +} + +constructor() { + this.count = $state(0); + this.count = $state.raw(1); // invalid, assigning to the same property with a different rune +} + +constructor() { + this.count = 0; + this.count = $state(1); // invalid, this property was created as a regular property, not state +} + +constructor() { + this.count = $state(0); + this.count = 1; // valid, this is setting the state that has already been declared +} +``` + ## declaration_duplicate > `%name%` has already been declared @@ -218,7 +247,7 @@ It's possible to export a snippet from a `