From 134d43539e7c722669bd33a911cd95639c3af6ef Mon Sep 17 00:00:00 2001 From: "S. Elliott Johnson" Date: Wed, 23 Apr 2025 00:19:08 -0600 Subject: [PATCH 01/64] 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 02/64] 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 ` + + diff --git a/packages/svelte/tests/runtime-runes/samples/class-state-constructor-derived-unowned/_config.js b/packages/svelte/tests/runtime-runes/samples/class-state-constructor-derived-unowned/_config.js new file mode 100644 index 000000000000..4cf1aea213dc --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/class-state-constructor-derived-unowned/_config.js @@ -0,0 +1,45 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + // The component context class instance gets shared between tests, strangely, causing hydration to fail? + mode: ['client', 'server'], + + async test({ assert, target, logs }) { + const btn = target.querySelector('button'); + + flushSync(() => { + btn?.click(); + }); + + assert.deepEqual(logs, [0, 'class trigger false', 'local trigger false', 1]); + + flushSync(() => { + btn?.click(); + }); + + assert.deepEqual(logs, [0, 'class trigger false', 'local trigger false', 1, 2]); + + flushSync(() => { + btn?.click(); + }); + + assert.deepEqual(logs, [0, 'class trigger false', 'local trigger false', 1, 2, 3]); + + flushSync(() => { + btn?.click(); + }); + + assert.deepEqual(logs, [ + 0, + 'class trigger false', + 'local trigger false', + 1, + 2, + 3, + 4, + 'class trigger true', + 'local trigger true' + ]); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/class-state-constructor-derived-unowned/main.svelte b/packages/svelte/tests/runtime-runes/samples/class-state-constructor-derived-unowned/main.svelte new file mode 100644 index 000000000000..03687d01bb3d --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/class-state-constructor-derived-unowned/main.svelte @@ -0,0 +1,37 @@ + + + + + diff --git a/packages/svelte/tests/runtime-runes/samples/class-state-constructor-subclass/_config.js b/packages/svelte/tests/runtime-runes/samples/class-state-constructor-subclass/_config.js new file mode 100644 index 000000000000..32cca6c69375 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/class-state-constructor-subclass/_config.js @@ -0,0 +1,20 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + html: ``, + + test({ assert, target }) { + const btn = target.querySelector('button'); + + flushSync(() => { + btn?.click(); + }); + assert.htmlEqual(target.innerHTML, ``); + + flushSync(() => { + btn?.click(); + }); + assert.htmlEqual(target.innerHTML, ``); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/class-state-constructor-subclass/main.svelte b/packages/svelte/tests/runtime-runes/samples/class-state-constructor-subclass/main.svelte new file mode 100644 index 000000000000..d8feb554cd18 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/class-state-constructor-subclass/main.svelte @@ -0,0 +1,22 @@ + + + diff --git a/packages/svelte/tests/runtime-runes/samples/class-state-constructor/_config.js b/packages/svelte/tests/runtime-runes/samples/class-state-constructor/_config.js new file mode 100644 index 000000000000..f35dc57228a1 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/class-state-constructor/_config.js @@ -0,0 +1,20 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + html: ``, + + test({ assert, target }) { + const btn = target.querySelector('button'); + + flushSync(() => { + btn?.click(); + }); + assert.htmlEqual(target.innerHTML, ``); + + flushSync(() => { + btn?.click(); + }); + assert.htmlEqual(target.innerHTML, ``); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/class-state-constructor/main.svelte b/packages/svelte/tests/runtime-runes/samples/class-state-constructor/main.svelte new file mode 100644 index 000000000000..aa8ba1658b03 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/class-state-constructor/main.svelte @@ -0,0 +1,18 @@ + + + diff --git a/packages/svelte/tests/validator/samples/class-state-constructor-1/errors.json b/packages/svelte/tests/validator/samples/class-state-constructor-1/errors.json new file mode 100644 index 000000000000..eaff12b89645 --- /dev/null +++ b/packages/svelte/tests/validator/samples/class-state-constructor-1/errors.json @@ -0,0 +1,14 @@ +[ + { + "code": "constructor_state_reassignment", + "message": "Cannot redeclare stateful field `count` in the constructor. The field was originally declared here: `(unknown):2:1`", + "start": { + "line": 5, + "column": 2 + }, + "end": { + "line": 5, + "column": 24 + } + } +] diff --git a/packages/svelte/tests/validator/samples/class-state-constructor-1/input.svelte.js b/packages/svelte/tests/validator/samples/class-state-constructor-1/input.svelte.js new file mode 100644 index 000000000000..05cd4d9d9d64 --- /dev/null +++ b/packages/svelte/tests/validator/samples/class-state-constructor-1/input.svelte.js @@ -0,0 +1,7 @@ +export class Counter { + count = $state(0); + + constructor() { + this.count = $state(0); + } +} diff --git a/packages/svelte/tests/validator/samples/class-state-constructor-2/errors.json b/packages/svelte/tests/validator/samples/class-state-constructor-2/errors.json new file mode 100644 index 000000000000..a27d7411d1f6 --- /dev/null +++ b/packages/svelte/tests/validator/samples/class-state-constructor-2/errors.json @@ -0,0 +1,14 @@ +[ + { + "code": "constructor_state_reassignment", + "message": "Cannot redeclare stateful field `count` in the constructor. The field was originally declared here: `(unknown):3:2`", + "start": { + "line": 5, + "column": 2 + }, + "end": { + "line": 5, + "column": 24 + } + } +] diff --git a/packages/svelte/tests/validator/samples/class-state-constructor-2/input.svelte.js b/packages/svelte/tests/validator/samples/class-state-constructor-2/input.svelte.js new file mode 100644 index 000000000000..e37be4b3e691 --- /dev/null +++ b/packages/svelte/tests/validator/samples/class-state-constructor-2/input.svelte.js @@ -0,0 +1,7 @@ +export class Counter { + constructor() { + this.count = $state(0); + this.count = 1; + this.count = $state(0); + } +} diff --git a/packages/svelte/tests/validator/samples/class-state-constructor-3/errors.json b/packages/svelte/tests/validator/samples/class-state-constructor-3/errors.json new file mode 100644 index 000000000000..8017794a679d --- /dev/null +++ b/packages/svelte/tests/validator/samples/class-state-constructor-3/errors.json @@ -0,0 +1,14 @@ +[ + { + "code": "constructor_state_reassignment", + "message": "Cannot redeclare stateful field `count` in the constructor. The field was originally declared here: `(unknown):3:2`", + "start": { + "line": 5, + "column": 2 + }, + "end": { + "line": 5, + "column": 28 + } + } +] diff --git a/packages/svelte/tests/validator/samples/class-state-constructor-3/input.svelte.js b/packages/svelte/tests/validator/samples/class-state-constructor-3/input.svelte.js new file mode 100644 index 000000000000..f9196ff3cd51 --- /dev/null +++ b/packages/svelte/tests/validator/samples/class-state-constructor-3/input.svelte.js @@ -0,0 +1,7 @@ +export class Counter { + constructor() { + this.count = $state(0); + this.count = 1; + this.count = $state.raw(0); + } +} diff --git a/packages/svelte/tests/validator/samples/class-state-constructor-4/errors.json b/packages/svelte/tests/validator/samples/class-state-constructor-4/errors.json new file mode 100644 index 000000000000..9f959874c80e --- /dev/null +++ b/packages/svelte/tests/validator/samples/class-state-constructor-4/errors.json @@ -0,0 +1,14 @@ +[ + { + "code": "state_invalid_placement", + "message": "`$state(...)` 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.", + "start": { + "line": 4, + "column": 16 + }, + "end": { + "line": 4, + "column": 25 + } + } +] diff --git a/packages/svelte/tests/validator/samples/class-state-constructor-4/input.svelte.js b/packages/svelte/tests/validator/samples/class-state-constructor-4/input.svelte.js new file mode 100644 index 000000000000..bf1aada1b5df --- /dev/null +++ b/packages/svelte/tests/validator/samples/class-state-constructor-4/input.svelte.js @@ -0,0 +1,7 @@ +export class Counter { + constructor() { + if (true) { + this.count = $state(0); + } + } +} From ac42ad5580125ab3eb99eeee7618d521a8100671 Mon Sep 17 00:00:00 2001 From: "S. Elliott Johnson" Date: Tue, 29 Apr 2025 16:22:57 -0400 Subject: [PATCH 08/64] final cleanup?? --- .../2-analyze/visitors/PropertyDefinition.js | 2 +- .../visitors/shared/class-analysis.js | 58 ++++++++++++++----- .../client/visitors/AssignmentExpression.js | 2 +- .../visitors/shared/client-class-analysis.js | 12 ++-- .../server/visitors/AssignmentExpression.js | 2 +- .../visitors/shared/server-class-analysis.js | 11 ++-- .../3-transform/shared/class_analysis.js | 52 +++++++++++++---- .../phases/3-transform/shared/types.d.ts | 33 ++++++++--- .../compiler/phases/3-transform/types.d.ts | 2 +- packages/svelte/src/compiler/phases/nodes.js | 1 + .../_config.js | 3 + .../main.svelte | 9 +++ .../class-state-constructor-5/errors.json | 14 +++++ .../class-state-constructor-5/input.svelte.js | 7 +++ .../class-state-constructor-6/errors.json | 14 +++++ .../class-state-constructor-6/input.svelte.js | 6 ++ 16 files changed, 182 insertions(+), 46 deletions(-) create mode 100644 packages/svelte/tests/runtime-runes/samples/class-state-constructor-conflicting-get-name/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/class-state-constructor-conflicting-get-name/main.svelte create mode 100644 packages/svelte/tests/validator/samples/class-state-constructor-5/errors.json create mode 100644 packages/svelte/tests/validator/samples/class-state-constructor-5/input.svelte.js create mode 100644 packages/svelte/tests/validator/samples/class-state-constructor-6/errors.json create mode 100644 packages/svelte/tests/validator/samples/class-state-constructor-6/input.svelte.js diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/PropertyDefinition.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/PropertyDefinition.js index 6bf5eb73d5db..aed7ff7fe8e8 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/PropertyDefinition.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/PropertyDefinition.js @@ -7,6 +7,6 @@ * @param {Context} context */ export function PropertyDefinition(node, context) { - context.state.class_state?.register?.(node, context); + context.state.class_state?.register(node, context); context.next(); } diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/class-analysis.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/class-analysis.js index 4b3d758200b6..02b585fd4dba 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/class-analysis.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/class-analysis.js @@ -1,4 +1,4 @@ -/** @import { AssignmentExpression, PropertyDefinition, Expression } from 'estree' */ +/** @import { AssignmentExpression, PropertyDefinition, Expression, Identifier, PrivateIdentifier, Literal, MethodDefinition } from 'estree' */ /** @import { AST } from '#compiler' */ /** @import { Context } from '../../types' */ /** @import { StateCreationRuneName } from '../../../../../utils.js' */ @@ -15,9 +15,11 @@ import { is_state_creation_rune } from '../../../../../utils.js'; const reassignable_assignments = new Set(['$state', '$state.raw', 'regular']); export class ClassAnalysis { - // TODO: Probably need to include property definitions here too /** @type {Map} */ - property_assignments = new Map(); + #public_assignments = new Map(); + + /** @type {Map} */ + #private_assignments = new Map(); /** * Determines if the node is a valid assignment to a class property, and if so, @@ -30,30 +32,46 @@ export class ClassAnalysis { let name; /** @type {PropertyAssignmentType} */ let type; + /** @type {boolean} */ + let is_private; if (node.type === 'AssignmentExpression') { if (!this.is_class_property_assignment_at_constructor_root(node, context.path)) { return; } - name = node.left.property.name; + + let maybe_name = get_name_for_identifier(node.left.property); + if (!maybe_name) { + return; + } + + name = maybe_name; type = this.#get_assignment_type(node, context); + is_private = node.left.property.type === 'PrivateIdentifier'; - this.#check_for_conflicts(node, name, type); + this.#check_for_conflicts(node, name, type, is_private); } else { if (!this.#is_assigned_property(node)) { return; } - name = node.key.name; + let maybe_name = get_name_for_identifier(node.key); + if (!maybe_name) { + return; + } + + name = maybe_name; type = this.#get_assignment_type(node, context); + is_private = node.key.type === 'PrivateIdentifier'; // we don't need to check for conflicts here because they're not possible yet } // we don't have to validate anything other than conflicts here, because the rune placement rules // catch all of the other weirdness. - if (!this.property_assignments.has(name)) { - this.property_assignments.set(name, { type, node }); + const map = is_private ? this.#private_assignments : this.#public_assignments; + if (!map.has(name)) { + map.set(name, { type, node }); } } @@ -61,7 +79,7 @@ export class ClassAnalysis { * @template {AST.SvelteNode} T * @param {AST.SvelteNode} node * @param {T[]} path - * @returns {node is AssignmentExpression & { left: { type: 'MemberExpression' } & { object: { type: 'ThisExpression' }; property: { type: 'Identifier' | 'PrivateIdentifier' } } }} + * @returns {node is AssignmentExpression & { left: { type: 'MemberExpression' } & { object: { type: 'ThisExpression' }; property: { type: 'Identifier' | 'PrivateIdentifier' | 'Literal' } } }} */ is_class_property_assignment_at_constructor_root(node, path) { if ( @@ -71,7 +89,8 @@ export class ClassAnalysis { node.left.type === 'MemberExpression' && node.left.object.type === 'ThisExpression' && (node.left.property.type === 'Identifier' || - node.left.property.type === 'PrivateIdentifier') + node.left.property.type === 'PrivateIdentifier' || + node.left.property.type === 'Literal') ) ) { return false; @@ -89,11 +108,13 @@ export class ClassAnalysis { * We only care about properties that have values assigned to them -- if they don't, * they can't be a conflict for state declared in the constructor. * @param {PropertyDefinition} node - * @returns {node is PropertyDefinition & { key: { type: 'PrivateIdentifier' | 'Identifier' }; value: Expression; static: false; computed: false }} + * @returns {node is PropertyDefinition & { key: { type: 'PrivateIdentifier' | 'Identifier' | 'Literal' }; value: Expression; static: false; computed: false }} */ #is_assigned_property(node) { return ( - (node.key.type === 'PrivateIdentifier' || node.key.type === 'Identifier') && + (node.key.type === 'PrivateIdentifier' || + node.key.type === 'Identifier' || + node.key.type === 'Literal') && Boolean(node.value) && !node.static && !node.computed @@ -107,9 +128,10 @@ export class ClassAnalysis { * @param {AssignmentExpression} node * @param {string} name * @param {PropertyAssignmentType} type + * @param {boolean} is_private */ - #check_for_conflicts(node, name, type) { - const existing = this.property_assignments.get(name); + #check_for_conflicts(node, name, type, is_private) { + const existing = (is_private ? this.#private_assignments : this.#public_assignments).get(name); if (!existing) { return; } @@ -140,3 +162,11 @@ export class ClassAnalysis { return 'regular'; } } + +/** + * + * @param {PrivateIdentifier | Identifier | Literal} node + */ +function get_name_for_identifier(node) { + return node.type === 'Literal' ? node.value?.toString() : node.name; +} diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js index ff8d2b5ce12c..34d80d388ca1 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js @@ -17,7 +17,7 @@ import { validate_mutation } from './shared/utils.js'; * @param {Context} context */ export function AssignmentExpression(node, context) { - const stripped_node = context.state.class_analysis?.register_assignment(node, context); + const stripped_node = context.state.class_analysis?.generate_assignment(node, context); if (stripped_node) { return stripped_node; } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/client-class-analysis.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/client-class-analysis.js index 70dd3646765b..778fea32e5c2 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/client-class-analysis.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/client-class-analysis.js @@ -56,7 +56,10 @@ export function create_client_class_analysis(body) { left: { ...node.left, // ...swap out the assignment to go directly against the private field - property: field.id + property: field.id, + // this could be a transformation from `this.[1]` to `this.#_` (the private field we generated) + // -- private fields are never computed + computed: false }, // ...and swap out the assignment's value for the state field init right: build_init_value(field.kind, node.right.arguments[0], context) @@ -67,15 +70,14 @@ export function create_client_class_analysis(body) { } /** - * * @param {StateCreationRuneName} kind * @param {Expression | SpreadElement} arg * @param {Context} context */ function build_init_value(kind, arg, context) { - const init = /** @type {Expression} **/ ( - context.visit(arg, { ...context.state, in_constructor: false }) - ); + const init = arg + ? /** @type {Expression} **/ (context.visit(arg, { ...context.state, in_constructor: false })) + : b.void0; switch (kind) { case '$state': diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/AssignmentExpression.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/AssignmentExpression.js index 9977c979db3f..ae84f0878228 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/AssignmentExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/AssignmentExpression.js @@ -10,7 +10,7 @@ import { visit_assignment_expression } from '../../shared/assignments.js'; * @param {Context} context */ export function AssignmentExpression(node, context) { - const stripped_node = context.state.class_analysis?.register_assignment(node, context); + const stripped_node = context.state.class_analysis?.generate_assignment(node, context); if (stripped_node) { return stripped_node; } diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/server-class-analysis.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/server-class-analysis.js index 44a641508766..73390d0e38ad 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/server-class-analysis.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/server-class-analysis.js @@ -23,11 +23,9 @@ export function create_server_class_analysis(body) { // value can be assigned in the constructor. value = null; } else if (field.kind !== '$derived' && field.kind !== '$derived.by') { - return [/** @type {PropertyDefinition} */ (context.visit(node, context.state))]; + return [/** @type {PropertyDefinition} */ (context.visit(node))]; } else { - const init = /** @type {Expression} **/ ( - context.visit(node.value.arguments[0], context.state) - ); + const init = /** @type {Expression} **/ (context.visit(node.value.arguments[0])); value = field.kind === '$derived.by' ? b.call('$.once', init) : b.call('$.once', b.thunk(init)); } @@ -73,7 +71,8 @@ export function create_server_class_analysis(body) { left: { ...node.left, // ...swap out the assignment to go directly against the private field - property: field.id + property: field.id, + computed: false }, // ...and swap out the assignment's value for the state field init right: build_init_value(field.kind, node.right.arguments[0], context) @@ -90,7 +89,7 @@ export function create_server_class_analysis(body) { * @param {Context} context */ function build_init_value(kind, arg, context) { - const init = /** @type {Expression} **/ (context.visit(arg, context.state)); + const init = arg ? /** @type {Expression} **/ (context.visit(arg)) : b.void0; switch (kind) { case '$state': diff --git a/packages/svelte/src/compiler/phases/3-transform/shared/class_analysis.js b/packages/svelte/src/compiler/phases/3-transform/shared/class_analysis.js index e482f10d49dc..20d9e75eca18 100644 --- a/packages/svelte/src/compiler/phases/3-transform/shared/class_analysis.js +++ b/packages/svelte/src/compiler/phases/3-transform/shared/class_analysis.js @@ -19,13 +19,25 @@ import { get_rune } from '../../scope.js'; * @returns {ClassAnalysis} */ export function create_class_analysis(body, build_state_field, build_assignment) { - /** @type {Map} */ + /** + * Public, stateful fields. + * @type {Map} + */ const public_fields = new Map(); - /** @type {Map} */ + /** + * Private, stateful fields. These are namespaced separately because + * public and private fields can have the same name in the AST -- ex. + * `count` and `#count` are both named `count` -- and because it's useful + * in a couple of cases to be able to check for only one or the other. + * @type {Map} + */ const private_fields = new Map(); - /** @type {Array} */ + /** + * Accumulates nodes for the new class body. + * @type {Array} + */ const new_body = []; /** @@ -57,6 +69,8 @@ export function create_class_analysis(body, build_state_field, build_assignment) const replacers = []; /** + * Get a state field by name. + * * @param {string} name * @param {boolean} is_private * @param {ReadonlyArray} [kinds] @@ -69,20 +83,30 @@ export function create_class_analysis(body, build_state_field, build_assignment) } /** + * Create a child context that makes sense for passing to the child analyzers. * @param {TContext} context * @returns {TContext} */ function create_child_context(context) { + const state = { + ...context.state, + class_analysis + }; + // @ts-expect-error - I can't find a way to make TypeScript happy with these + const visit = (node, state_override) => context.visit(node, { ...state, ...state_override }); + // @ts-expect-error - I can't find a way to make TypeScript happy with these + const next = (state_override) => context.next({ ...state, ...state_override }); return { ...context, - state: { - ...context.state, - class_analysis - } + state, + visit, + next }; } /** + * Generate a new body for the class. Ensure there is a visitor for AssignmentExpression that + * calls `generate_assignment` to capture any stateful fields declared in the constructor. * @param {TContext} context */ function generate_body(context) { @@ -107,11 +131,16 @@ export function create_class_analysis(body, build_state_field, build_assignment) } /** + * Given an assignment expression, check to see if that assignment expression declares + * a stateful field. If it does, register that field and then return the processed + * assignment expression. If an assignment expression is returned from this function, + * it should be considered _fully processed_ and should replace the existing assignment + * expression node. * @param {AssignmentExpression} node * @param {TContext} context * @returns {AssignmentExpression | null} The node, if `register_assignment` handled its transformation. */ - function register_assignment(node, context) { + function generate_assignment(node, context) { const child_context = create_child_context(context); if ( !( @@ -119,7 +148,8 @@ export function create_class_analysis(body, build_state_field, build_assignment) node.left.type === 'MemberExpression' && node.left.object.type === 'ThisExpression' && (node.left.property.type === 'Identifier' || - node.left.property.type === 'PrivateIdentifier') + node.left.property.type === 'PrivateIdentifier' || + node.left.property.type === 'Literal') ) ) { return null; @@ -177,6 +207,8 @@ export function create_class_analysis(body, build_state_field, build_assignment) } /** + * Register a class body definition. + * * @param {PropertyDefinition | MethodDefinition | StaticBlock} node * @param {TContext} child_context * @returns {boolean} if this node is stateful and was registered @@ -325,7 +357,7 @@ export function create_class_analysis(body, build_state_field, build_assignment) const class_analysis = { get_field, generate_body, - register_assignment + generate_assignment }; return class_analysis; diff --git a/packages/svelte/src/compiler/phases/3-transform/shared/types.d.ts b/packages/svelte/src/compiler/phases/3-transform/shared/types.d.ts index 96fe27ca5dcc..2d02e705c317 100644 --- a/packages/svelte/src/compiler/phases/3-transform/shared/types.d.ts +++ b/packages/svelte/src/compiler/phases/3-transform/shared/types.d.ts @@ -1,4 +1,14 @@ -import type { AssignmentExpression, CallExpression, Identifier, MemberExpression, PropertyDefinition, MethodDefinition, PrivateIdentifier, ThisExpression } from 'estree'; +import type { + AssignmentExpression, + CallExpression, + Identifier, + MemberExpression, + PropertyDefinition, + MethodDefinition, + PrivateIdentifier, + ThisExpression, + Literal +} from 'estree'; import type { StateField } from '../types'; import type { Context as ServerContext } from '../server/types'; import type { Context as ClientContext } from '../client/types'; @@ -7,13 +17,13 @@ import type { StateCreationRuneName } from '../../../../utils'; export type StatefulAssignment = AssignmentExpression & { left: MemberExpression & { object: ThisExpression; - property: Identifier | PrivateIdentifier + property: Identifier | PrivateIdentifier | Literal; }; right: CallExpression; }; export type StatefulPropertyDefinition = PropertyDefinition & { - key: Identifier | PrivateIdentifier; + key: Identifier | PrivateIdentifier | Literal; value: CallExpression; }; @@ -34,7 +44,9 @@ export type AssignmentBuilderParams = (params: AssignmentBuilderParams) => AssignmentExpression; +export type AssignmentBuilder = ( + params: AssignmentBuilderParams +) => AssignmentExpression; export type ClassAnalysis = { /** @@ -43,7 +55,11 @@ export type ClassAnalysis = { * @param kinds - What kinds of state creation runes you're looking for, eg. only '$derived.by'. * @returns The field if it exists and matches the given criteria, or null. */ - get_field: (name: string, is_private: boolean, kinds?: Array) => StateField | undefined; + get_field: ( + name: string, + is_private: boolean, + kinds?: Array + ) => StateField | undefined; /** * Given the body of a class, generate a new body with stateful fields. @@ -59,5 +75,8 @@ export type ClassAnalysis = { * a state field on the class. If it is, it registers that state field and modifies the * assignment expression. */ - register_assignment: (node: AssignmentExpression, context: TContext) => AssignmentExpression | null; -} + generate_assignment: ( + node: AssignmentExpression, + context: TContext + ) => AssignmentExpression | null; +}; diff --git a/packages/svelte/src/compiler/phases/3-transform/types.d.ts b/packages/svelte/src/compiler/phases/3-transform/types.d.ts index 8999f096142e..c610110cef9d 100644 --- a/packages/svelte/src/compiler/phases/3-transform/types.d.ts +++ b/packages/svelte/src/compiler/phases/3-transform/types.d.ts @@ -14,4 +14,4 @@ export interface TransformState { export interface StateField { kind: StateCreationRuneName; id: PrivateIdentifier; -} \ No newline at end of file +} diff --git a/packages/svelte/src/compiler/phases/nodes.js b/packages/svelte/src/compiler/phases/nodes.js index 003c3a2c4945..29fb8c5c83c2 100644 --- a/packages/svelte/src/compiler/phases/nodes.js +++ b/packages/svelte/src/compiler/phases/nodes.js @@ -1,4 +1,5 @@ /** @import { AST, ExpressionMetadata } from '#compiler' */ + /** * All nodes that can appear elsewhere than the top level, have attributes and can contain children */ diff --git a/packages/svelte/tests/runtime-runes/samples/class-state-constructor-conflicting-get-name/_config.js b/packages/svelte/tests/runtime-runes/samples/class-state-constructor-conflicting-get-name/_config.js new file mode 100644 index 000000000000..f47bee71df87 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/class-state-constructor-conflicting-get-name/_config.js @@ -0,0 +1,3 @@ +import { test } from '../../test'; + +export default test({}); diff --git a/packages/svelte/tests/runtime-runes/samples/class-state-constructor-conflicting-get-name/main.svelte b/packages/svelte/tests/runtime-runes/samples/class-state-constructor-conflicting-get-name/main.svelte new file mode 100644 index 000000000000..e2c4f302b397 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/class-state-constructor-conflicting-get-name/main.svelte @@ -0,0 +1,9 @@ + \ No newline at end of file diff --git a/packages/svelte/tests/validator/samples/class-state-constructor-5/errors.json b/packages/svelte/tests/validator/samples/class-state-constructor-5/errors.json new file mode 100644 index 000000000000..359274d15661 --- /dev/null +++ b/packages/svelte/tests/validator/samples/class-state-constructor-5/errors.json @@ -0,0 +1,14 @@ +[ + { + "code": "constructor_state_reassignment", + "message": "Cannot redeclare stateful field `count` in the constructor. The field was originally declared here: `(unknown):2:1`", + "start": { + "line": 4, + "column": 2 + }, + "end": { + "line": 4, + "column": 27 + } + } +] diff --git a/packages/svelte/tests/validator/samples/class-state-constructor-5/input.svelte.js b/packages/svelte/tests/validator/samples/class-state-constructor-5/input.svelte.js new file mode 100644 index 000000000000..bc3d19a14fae --- /dev/null +++ b/packages/svelte/tests/validator/samples/class-state-constructor-5/input.svelte.js @@ -0,0 +1,7 @@ +export class Counter { + // prettier-ignore + 'count' = $state(0); + constructor() { + this['count'] = $state(0); + } +} diff --git a/packages/svelte/tests/validator/samples/class-state-constructor-6/errors.json b/packages/svelte/tests/validator/samples/class-state-constructor-6/errors.json new file mode 100644 index 000000000000..359274d15661 --- /dev/null +++ b/packages/svelte/tests/validator/samples/class-state-constructor-6/errors.json @@ -0,0 +1,14 @@ +[ + { + "code": "constructor_state_reassignment", + "message": "Cannot redeclare stateful field `count` in the constructor. The field was originally declared here: `(unknown):2:1`", + "start": { + "line": 4, + "column": 2 + }, + "end": { + "line": 4, + "column": 27 + } + } +] diff --git a/packages/svelte/tests/validator/samples/class-state-constructor-6/input.svelte.js b/packages/svelte/tests/validator/samples/class-state-constructor-6/input.svelte.js new file mode 100644 index 000000000000..2ebe52e685ed --- /dev/null +++ b/packages/svelte/tests/validator/samples/class-state-constructor-6/input.svelte.js @@ -0,0 +1,6 @@ +export class Counter { + count = $state(0); + constructor() { + this['count'] = $state(0); + } +} From b44eed9a091c360a31d3b0529c7fbd555dc1fffe Mon Sep 17 00:00:00 2001 From: "S. Elliott Johnson" Date: Tue, 29 Apr 2025 16:49:00 -0400 Subject: [PATCH 09/64] tests --- .../validator/samples/class-state-constructor-5/errors.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/svelte/tests/validator/samples/class-state-constructor-5/errors.json b/packages/svelte/tests/validator/samples/class-state-constructor-5/errors.json index 359274d15661..7942b016cb2b 100644 --- a/packages/svelte/tests/validator/samples/class-state-constructor-5/errors.json +++ b/packages/svelte/tests/validator/samples/class-state-constructor-5/errors.json @@ -1,13 +1,13 @@ [ { "code": "constructor_state_reassignment", - "message": "Cannot redeclare stateful field `count` in the constructor. The field was originally declared here: `(unknown):2:1`", + "message": "Cannot redeclare stateful field `count` in the constructor. The field was originally declared here: `(unknown):3:1`", "start": { - "line": 4, + "line": 5, "column": 2 }, "end": { - "line": 4, + "line": 5, "column": 27 } } From 12a02b7eed5c3b01bc4be6ea2f060ecd3a8d5e4d Mon Sep 17 00:00:00 2001 From: "S. Elliott Johnson" Date: Tue, 29 Apr 2025 17:00:13 -0400 Subject: [PATCH 10/64] test for better types --- .../_config.js | 20 +++++++++++++++++++ .../main.svelte | 12 +++++++++++ 2 files changed, 32 insertions(+) create mode 100644 packages/svelte/tests/runtime-runes/samples/class-state-constructor-predeclared-field/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/class-state-constructor-predeclared-field/main.svelte diff --git a/packages/svelte/tests/runtime-runes/samples/class-state-constructor-predeclared-field/_config.js b/packages/svelte/tests/runtime-runes/samples/class-state-constructor-predeclared-field/_config.js new file mode 100644 index 000000000000..02cf36d900cc --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/class-state-constructor-predeclared-field/_config.js @@ -0,0 +1,20 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + html: ``, + + test({ assert, target }) { + const btn = target.querySelector('button'); + + flushSync(() => { + btn?.click(); + }); + assert.htmlEqual(target.innerHTML, ``); + + flushSync(() => { + btn?.click(); + }); + assert.htmlEqual(target.innerHTML, ``); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/class-state-constructor-predeclared-field/main.svelte b/packages/svelte/tests/runtime-runes/samples/class-state-constructor-predeclared-field/main.svelte new file mode 100644 index 000000000000..5dbbb10afd35 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/class-state-constructor-predeclared-field/main.svelte @@ -0,0 +1,12 @@ + + + From 50adbfbd69093a0d8949f1bf9bc54f6ad82bad01 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 15 May 2025 12:25:27 -0400 Subject: [PATCH 11/64] changeset --- .changeset/mean-squids-scream.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/mean-squids-scream.md diff --git a/.changeset/mean-squids-scream.md b/.changeset/mean-squids-scream.md new file mode 100644 index 000000000000..2157ea85a64f --- /dev/null +++ b/.changeset/mean-squids-scream.md @@ -0,0 +1,5 @@ +--- +'svelte': minor +--- + +feat: allow state fields to be declared inside class constructors From 682b0e6970edad88ee1f8d1ec9d44d0b93dca68f Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 15 May 2025 13:05:14 -0400 Subject: [PATCH 12/64] rename functions (the function doesn't test call-expression-ness) --- .../compiler/phases/2-analyze/visitors/CallExpression.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 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 2abf8a88b1d5..f8310ef64777 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js @@ -117,8 +117,8 @@ export function CallExpression(node, context) { case '$derived.by': if ( !( - call_expression_is_variable_declaration(parent, context) || - call_expression_is_class_property_definition(parent) || + is_variable_declaration(parent, context) || + is_class_property_definition(parent) || context.state.class_state?.is_class_property_assignment_at_constructor_root( parent, context.path.slice(0, -1) @@ -281,7 +281,7 @@ function get_function_label(nodes) { * @param {AST.SvelteNode} parent * @param {Context} context */ -function call_expression_is_variable_declaration(parent, context) { +function is_variable_declaration(parent, context) { return parent.type === 'VariableDeclarator' && get_parent(context.path, -3).type !== 'ConstTag'; } @@ -289,6 +289,6 @@ function call_expression_is_variable_declaration(parent, context) { * * @param {AST.SvelteNode} parent */ -function call_expression_is_class_property_definition(parent) { +function is_class_property_definition(parent) { return parent.type === 'PropertyDefinition' && !parent.static && !parent.computed; } From 76b07e5b57d5d270e3b8eecff4100ff83a025112 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 15 May 2025 14:09:49 -0400 Subject: [PATCH 13/64] small readability tweak --- .../2-analyze/visitors/CallExpression.js | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 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 f8310ef64777..58c7447f2cd3 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js @@ -114,17 +114,16 @@ export function CallExpression(node, context) { case '$state': case '$state.raw': case '$derived': - case '$derived.by': - if ( - !( - is_variable_declaration(parent, context) || - is_class_property_definition(parent) || - context.state.class_state?.is_class_property_assignment_at_constructor_root( - parent, - context.path.slice(0, -1) - ) - ) - ) { + case '$derived.by': { + const valid = + is_variable_declaration(parent, context) || + is_class_property_definition(parent) || + context.state.class_state?.is_class_property_assignment_at_constructor_root( + parent, + context.path.slice(0, -1) + ); + + if (!valid) { e.state_invalid_placement(node, rune); } @@ -135,6 +134,7 @@ export function CallExpression(node, context) { } break; + } case '$effect': case '$effect.pre': From 63950858d6a4def871f1dc1876ea028b64d8ed5e Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 15 May 2025 14:09:55 -0400 Subject: [PATCH 14/64] failing test --- .../_config.js | 13 +++++++++++++ .../main.svelte | 12 ++++++++++++ 2 files changed, 25 insertions(+) create mode 100644 packages/svelte/tests/runtime-runes/samples/class-state-constructor-closure-private-3/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/class-state-constructor-closure-private-3/main.svelte diff --git a/packages/svelte/tests/runtime-runes/samples/class-state-constructor-closure-private-3/_config.js b/packages/svelte/tests/runtime-runes/samples/class-state-constructor-closure-private-3/_config.js new file mode 100644 index 000000000000..dd847ce2f2a6 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/class-state-constructor-closure-private-3/_config.js @@ -0,0 +1,13 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + html: ``, + ssrHtml: ``, + + async test({ assert, target }) { + flushSync(); + + assert.htmlEqual(target.innerHTML, ``); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/class-state-constructor-closure-private-3/main.svelte b/packages/svelte/tests/runtime-runes/samples/class-state-constructor-closure-private-3/main.svelte new file mode 100644 index 000000000000..47b8c901eb95 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/class-state-constructor-closure-private-3/main.svelte @@ -0,0 +1,12 @@ + + + From 0024e1ed3ed62739161b96d8eae46580040d5907 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 15 May 2025 14:14:40 -0400 Subject: [PATCH 15/64] fix --- .../svelte/src/compiler/phases/2-analyze/index.js | 2 ++ .../phases/2-analyze/visitors/ClassBody.js | 14 ++++++++++++++ .../phases/2-analyze/visitors/ClassDeclaration.js | 6 +----- 3 files changed, 17 insertions(+), 5 deletions(-) create mode 100644 packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index af3945d43564..3c586179c832 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -23,6 +23,7 @@ import { Attribute } from './visitors/Attribute.js'; import { AwaitBlock } from './visitors/AwaitBlock.js'; import { BindDirective } from './visitors/BindDirective.js'; import { CallExpression } from './visitors/CallExpression.js'; +import { ClassBody } from './visitors/ClassBody.js'; import { ClassDeclaration } from './visitors/ClassDeclaration.js'; import { ClassDirective } from './visitors/ClassDirective.js'; import { Component } from './visitors/Component.js'; @@ -139,6 +140,7 @@ const visitors = { AwaitBlock, BindDirective, CallExpression, + ClassBody, ClassDeclaration, ClassDirective, Component, diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js new file mode 100644 index 000000000000..30364a14f10f --- /dev/null +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js @@ -0,0 +1,14 @@ +/** @import { ClassBody } from 'estree' */ +/** @import { Context } from '../types' */ +import { ClassAnalysis } from './shared/class-analysis.js'; + +/** + * @param {ClassBody} node + * @param {Context} context + */ +export function ClassBody(node, context) { + context.next({ + ...context.state, + class_state: context.state.analysis.runes ? new ClassAnalysis() : null + }); +} diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassDeclaration.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassDeclaration.js index b49ad099e59c..20925a65b737 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassDeclaration.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassDeclaration.js @@ -1,7 +1,6 @@ /** @import { ClassDeclaration } from 'estree' */ /** @import { Context } from '../types' */ import * as w from '../../../warnings.js'; -import { ClassAnalysis } from './shared/class-analysis.js'; import { validate_identifier_name } from './shared/utils.js'; /** @@ -22,8 +21,5 @@ export function ClassDeclaration(node, context) { w.perf_avoid_nested_class(node); } - context.next({ - ...context.state, - class_state: context.state.analysis.runes ? new ClassAnalysis() : null - }); + context.next(); } From 8c7ad3c5a394f6efb0fcb981177a7d8a37105632 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 15 May 2025 14:30:23 -0400 Subject: [PATCH 16/64] disallow computed state fields --- .../2-analyze/visitors/shared/class-analysis.js | 2 +- .../samples/class-state-constructor-7/errors.json | 14 ++++++++++++++ .../class-state-constructor-7/input.svelte.js | 7 +++++++ 3 files changed, 22 insertions(+), 1 deletion(-) create mode 100644 packages/svelte/tests/validator/samples/class-state-constructor-7/errors.json create mode 100644 packages/svelte/tests/validator/samples/class-state-constructor-7/input.svelte.js diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/class-analysis.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/class-analysis.js index 02b585fd4dba..27be9261f885 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/class-analysis.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/class-analysis.js @@ -88,7 +88,7 @@ export class ClassAnalysis { node.operator === '=' && node.left.type === 'MemberExpression' && node.left.object.type === 'ThisExpression' && - (node.left.property.type === 'Identifier' || + ((node.left.property.type === 'Identifier' && !node.left.computed) || node.left.property.type === 'PrivateIdentifier' || node.left.property.type === 'Literal') ) diff --git a/packages/svelte/tests/validator/samples/class-state-constructor-7/errors.json b/packages/svelte/tests/validator/samples/class-state-constructor-7/errors.json new file mode 100644 index 000000000000..64e56f8d5c4e --- /dev/null +++ b/packages/svelte/tests/validator/samples/class-state-constructor-7/errors.json @@ -0,0 +1,14 @@ +[ + { + "code": "state_invalid_placement", + "message": "`$state(...)` 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.", + "start": { + "line": 5, + "column": 16 + }, + "end": { + "line": 5, + "column": 25 + } + } +] diff --git a/packages/svelte/tests/validator/samples/class-state-constructor-7/input.svelte.js b/packages/svelte/tests/validator/samples/class-state-constructor-7/input.svelte.js new file mode 100644 index 000000000000..50c855983700 --- /dev/null +++ b/packages/svelte/tests/validator/samples/class-state-constructor-7/input.svelte.js @@ -0,0 +1,7 @@ +const count = 'count'; + +export class Counter { + constructor() { + this[count] = $state(0); + } +} From 15c7b143f9e2918ecc1a5bc16dd3c94d0350b802 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 15 May 2025 15:01:47 -0400 Subject: [PATCH 17/64] tweak message to better accommodate the case in which state is declared after a regular property --- .../docs/98-reference/.generated/compile-errors.md | 4 ++-- packages/svelte/messages/compile-errors/script.md | 4 ++-- packages/svelte/src/compiler/errors.js | 8 +++----- .../samples/class-state-constructor-1/errors.json | 2 +- .../samples/class-state-constructor-2/errors.json | 2 +- .../samples/class-state-constructor-3/errors.json | 2 +- .../samples/class-state-constructor-5/errors.json | 2 +- .../samples/class-state-constructor-6/errors.json | 2 +- .../samples/class-state-constructor-8/errors.json | 14 ++++++++++++++ .../class-state-constructor-8/input.svelte.js | 6 ++++++ 10 files changed, 32 insertions(+), 14 deletions(-) create mode 100644 packages/svelte/tests/validator/samples/class-state-constructor-8/errors.json create mode 100644 packages/svelte/tests/validator/samples/class-state-constructor-8/input.svelte.js diff --git a/documentation/docs/98-reference/.generated/compile-errors.md b/documentation/docs/98-reference/.generated/compile-errors.md index fb0591b884a5..281186168060 100644 --- a/documentation/docs/98-reference/.generated/compile-errors.md +++ b/documentation/docs/98-reference/.generated/compile-errors.md @@ -211,10 +211,10 @@ Cannot bind to %thing% ### constructor_state_reassignment ``` -Cannot redeclare stateful field `%name%` in the constructor. The field was originally declared here: `%original_location%` +A state field declaration in a constructor must be the first assignment, and the only one that uses a rune ``` -To create stateful class fields in the constructor, the rune assignment must be the _first_ assignment to the class field. +[State fields]($state#Classes) can be declared as normal class fields or inside the constructor, in which case the declaration must be the _first_ assignment. Assignments thereafter must not use the rune. ```ts diff --git a/packages/svelte/messages/compile-errors/script.md b/packages/svelte/messages/compile-errors/script.md index 780c9f4d2863..7831be3a42eb 100644 --- a/packages/svelte/messages/compile-errors/script.md +++ b/packages/svelte/messages/compile-errors/script.md @@ -12,9 +12,9 @@ ## constructor_state_reassignment -> Cannot redeclare stateful field `%name%` in the constructor. The field was originally declared here: `%original_location%` +> A state field declaration in a constructor must be the first assignment, and the only one that uses a rune -To create stateful class fields in the constructor, the rune assignment must be the _first_ assignment to the class field. +[State fields]($state#Classes) can be declared as normal class fields or inside the constructor, in which case the declaration must be the _first_ assignment. Assignments thereafter must not use the rune. ```ts diff --git a/packages/svelte/src/compiler/errors.js b/packages/svelte/src/compiler/errors.js index fc814a16d31a..2913100f1665 100644 --- a/packages/svelte/src/compiler/errors.js +++ b/packages/svelte/src/compiler/errors.js @@ -105,14 +105,12 @@ export function constant_binding(node, thing) { } /** - * Cannot redeclare stateful field `%name%` in the constructor. The field was originally declared here: `%original_location%` + * A state field declaration in a constructor must be the first assignment, and the only one that uses a rune * @param {null | number | NodeLike} node - * @param {string} name - * @param {string} original_location * @returns {never} */ -export function constructor_state_reassignment(node, name, original_location) { - e(node, 'constructor_state_reassignment', `Cannot redeclare stateful field \`${name}\` in the constructor. The field was originally declared here: \`${original_location}\`\nhttps://svelte.dev/e/constructor_state_reassignment`); +export function constructor_state_reassignment(node) { + e(node, 'constructor_state_reassignment', `A state field declaration in a constructor must be the first assignment, and the only one that uses a rune\nhttps://svelte.dev/e/constructor_state_reassignment`); } /** diff --git a/packages/svelte/tests/validator/samples/class-state-constructor-1/errors.json b/packages/svelte/tests/validator/samples/class-state-constructor-1/errors.json index eaff12b89645..3eaacfe85013 100644 --- a/packages/svelte/tests/validator/samples/class-state-constructor-1/errors.json +++ b/packages/svelte/tests/validator/samples/class-state-constructor-1/errors.json @@ -1,7 +1,7 @@ [ { "code": "constructor_state_reassignment", - "message": "Cannot redeclare stateful field `count` in the constructor. The field was originally declared here: `(unknown):2:1`", + "message": "A state field declaration in a constructor must be the first assignment, and the only one that uses a rune", "start": { "line": 5, "column": 2 diff --git a/packages/svelte/tests/validator/samples/class-state-constructor-2/errors.json b/packages/svelte/tests/validator/samples/class-state-constructor-2/errors.json index a27d7411d1f6..3eaacfe85013 100644 --- a/packages/svelte/tests/validator/samples/class-state-constructor-2/errors.json +++ b/packages/svelte/tests/validator/samples/class-state-constructor-2/errors.json @@ -1,7 +1,7 @@ [ { "code": "constructor_state_reassignment", - "message": "Cannot redeclare stateful field `count` in the constructor. The field was originally declared here: `(unknown):3:2`", + "message": "A state field declaration in a constructor must be the first assignment, and the only one that uses a rune", "start": { "line": 5, "column": 2 diff --git a/packages/svelte/tests/validator/samples/class-state-constructor-3/errors.json b/packages/svelte/tests/validator/samples/class-state-constructor-3/errors.json index 8017794a679d..a0f12ba4a6e6 100644 --- a/packages/svelte/tests/validator/samples/class-state-constructor-3/errors.json +++ b/packages/svelte/tests/validator/samples/class-state-constructor-3/errors.json @@ -1,7 +1,7 @@ [ { "code": "constructor_state_reassignment", - "message": "Cannot redeclare stateful field `count` in the constructor. The field was originally declared here: `(unknown):3:2`", + "message": "A state field declaration in a constructor must be the first assignment, and the only one that uses a rune", "start": { "line": 5, "column": 2 diff --git a/packages/svelte/tests/validator/samples/class-state-constructor-5/errors.json b/packages/svelte/tests/validator/samples/class-state-constructor-5/errors.json index 7942b016cb2b..82209f6a8ccf 100644 --- a/packages/svelte/tests/validator/samples/class-state-constructor-5/errors.json +++ b/packages/svelte/tests/validator/samples/class-state-constructor-5/errors.json @@ -1,7 +1,7 @@ [ { "code": "constructor_state_reassignment", - "message": "Cannot redeclare stateful field `count` in the constructor. The field was originally declared here: `(unknown):3:1`", + "message": "A state field declaration in a constructor must be the first assignment, and the only one that uses a rune", "start": { "line": 5, "column": 2 diff --git a/packages/svelte/tests/validator/samples/class-state-constructor-6/errors.json b/packages/svelte/tests/validator/samples/class-state-constructor-6/errors.json index 359274d15661..7dd90d4837e6 100644 --- a/packages/svelte/tests/validator/samples/class-state-constructor-6/errors.json +++ b/packages/svelte/tests/validator/samples/class-state-constructor-6/errors.json @@ -1,7 +1,7 @@ [ { "code": "constructor_state_reassignment", - "message": "Cannot redeclare stateful field `count` in the constructor. The field was originally declared here: `(unknown):2:1`", + "message": "A state field declaration in a constructor must be the first assignment, and the only one that uses a rune", "start": { "line": 4, "column": 2 diff --git a/packages/svelte/tests/validator/samples/class-state-constructor-8/errors.json b/packages/svelte/tests/validator/samples/class-state-constructor-8/errors.json new file mode 100644 index 000000000000..ae0caee6169d --- /dev/null +++ b/packages/svelte/tests/validator/samples/class-state-constructor-8/errors.json @@ -0,0 +1,14 @@ +[ + { + "code": "constructor_state_reassignment", + "message": "A state field declaration in a constructor must be the first assignment, and the only one that uses a rune", + "start": { + "line": 4, + "column": 2 + }, + "end": { + "line": 4, + "column": 24 + } + } +] diff --git a/packages/svelte/tests/validator/samples/class-state-constructor-8/input.svelte.js b/packages/svelte/tests/validator/samples/class-state-constructor-8/input.svelte.js new file mode 100644 index 000000000000..0a76c6fec90f --- /dev/null +++ b/packages/svelte/tests/validator/samples/class-state-constructor-8/input.svelte.js @@ -0,0 +1,6 @@ +export class Counter { + constructor() { + this.count = -1; + this.count = $state(0); + } +} From 0ac22c19b213822a0de90c516e523dc22164ca13 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 15 May 2025 15:02:58 -0400 Subject: [PATCH 18/64] failing test --- .../samples/class-state-constructor-9/errors.json | 14 ++++++++++++++ .../class-state-constructor-9/input.svelte.js | 9 +++++++++ 2 files changed, 23 insertions(+) create mode 100644 packages/svelte/tests/validator/samples/class-state-constructor-9/errors.json create mode 100644 packages/svelte/tests/validator/samples/class-state-constructor-9/input.svelte.js diff --git a/packages/svelte/tests/validator/samples/class-state-constructor-9/errors.json b/packages/svelte/tests/validator/samples/class-state-constructor-9/errors.json new file mode 100644 index 000000000000..6cdeb4a14e59 --- /dev/null +++ b/packages/svelte/tests/validator/samples/class-state-constructor-9/errors.json @@ -0,0 +1,14 @@ +[ + { + "code": "constructor_state_reassignment", + "message": "A state field declaration in a constructor must be the first assignment, and the only one that uses a rune", + "start": { + "line": 7, + "column": 2 + }, + "end": { + "line": 7, + "column": 24 + } + } +] diff --git a/packages/svelte/tests/validator/samples/class-state-constructor-9/input.svelte.js b/packages/svelte/tests/validator/samples/class-state-constructor-9/input.svelte.js new file mode 100644 index 000000000000..e5ad562727c7 --- /dev/null +++ b/packages/svelte/tests/validator/samples/class-state-constructor-9/input.svelte.js @@ -0,0 +1,9 @@ +export class Counter { + constructor() { + if (true) { + this.count = -1; + } + + this.count = $state(0); + } +} From 4edebbcfb128e49d51e9d4cd4cfdf4235a8bfa5e Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 15 May 2025 15:57:30 -0400 Subject: [PATCH 19/64] wildly confusing to have so many things called 'class analysis' - rename the transform-phrase ones to transformers --- .../src/compiler/phases/3-transform/client/types.d.ts | 4 ++-- .../phases/3-transform/client/visitors/ClassBody.js | 6 +++--- ...t-class-analysis.js => client-class-transformer.js} | 10 +++++----- .../src/compiler/phases/3-transform/server/types.d.ts | 4 ++-- .../phases/3-transform/server/visitors/ClassBody.js | 6 +++--- ...r-class-analysis.js => server-class-transformer.js} | 10 +++++----- .../shared/{class_analysis.js => class_transformer.js} | 6 +++--- .../src/compiler/phases/3-transform/shared/types.d.ts | 2 +- 8 files changed, 24 insertions(+), 24 deletions(-) rename packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/{client-class-analysis.js => client-class-transformer.js} (88%) rename packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/{server-class-analysis.js => server-class-transformer.js} (90%) rename packages/svelte/src/compiler/phases/3-transform/shared/{class_analysis.js => class_transformer.js} (97%) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts b/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts index 3e7ef24aee58..ce201958da2c 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts +++ b/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts @@ -12,10 +12,10 @@ import type { AST, Namespace, ValidatedCompileOptions } from '#compiler'; import type { TransformState } from '../types.js'; import type { ComponentAnalysis } from '../../types.js'; import type { SourceLocation } from '#shared'; -import type { ClassAnalysis } from '../shared/types.js'; +import type { ClassTransformer } from '../shared/types.js'; export interface ClientTransformState extends TransformState { - readonly class_analysis: ClassAnalysis | null; + readonly class_analysis: ClassTransformer | null; /** * `true` if the current lexical scope belongs to a class constructor. this allows diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js index a9ccad03770c..75f2b35cca62 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js @@ -1,6 +1,6 @@ /** @import { ClassBody } from 'estree' */ /** @import { Context } from '../types' */ -import { create_client_class_analysis } from './shared/client-class-analysis.js'; +import { create_client_class_transformer } from './shared/client-class-transformer.js'; /** * @param {ClassBody} node @@ -12,8 +12,8 @@ export function ClassBody(node, context) { return; } - const class_analysis = create_client_class_analysis(node.body); - const body = class_analysis.generate_body(context); + const class_transformer = create_client_class_transformer(node.body); + const body = class_transformer.generate_body(context); return { ...node, body }; } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/client-class-analysis.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/client-class-transformer.js similarity index 88% rename from packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/client-class-analysis.js rename to packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/client-class-transformer.js index 778fea32e5c2..767a99ea6952 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/client-class-analysis.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/client-class-transformer.js @@ -1,16 +1,16 @@ /** @import { Context } from '../../types.js' */ /** @import { MethodDefinition, PropertyDefinition, Expression, StaticBlock, SpreadElement } from 'estree' */ /** @import { StateCreationRuneName } from '../../../../../../utils.js' */ -/** @import { AssignmentBuilder, ClassAnalysis, StateFieldBuilder } from '../../../shared/types.js' */ +/** @import { AssignmentBuilder, ClassTransformer, StateFieldBuilder } from '../../../shared/types.js' */ import * as b from '#compiler/builders'; -import { create_class_analysis } from '../../../shared/class_analysis.js'; +import { create_class_transformer } from '../../../shared/class_transformer.js'; import { should_proxy } from '../../utils.js'; /** * @param {Array} body - * @returns {ClassAnalysis} + * @returns {ClassTransformer} */ -export function create_client_class_analysis(body) { +export function create_client_class_transformer(body) { /** @type {StateFieldBuilder} */ function build_state_field({ is_private, field, node, context }) { let original_id = node.type === 'AssignmentExpression' ? node.left.property : node.key; @@ -66,7 +66,7 @@ export function create_client_class_analysis(body) { }; } - return create_class_analysis(body, build_state_field, build_assignment); + return create_class_transformer(body, build_state_field, build_assignment); } /** diff --git a/packages/svelte/src/compiler/phases/3-transform/server/types.d.ts b/packages/svelte/src/compiler/phases/3-transform/server/types.d.ts index 0f1b2647a288..9d2d3164a9ef 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/types.d.ts +++ b/packages/svelte/src/compiler/phases/3-transform/server/types.d.ts @@ -2,12 +2,12 @@ import type { Expression, Statement, ModuleDeclaration, LabeledStatement } from import type { AST, Namespace, ValidatedCompileOptions } from '#compiler'; import type { TransformState } from '../types.js'; import type { ComponentAnalysis } from '../../types.js'; -import type { ClassAnalysis } from '../shared/types.js'; +import type { ClassTransformer } from '../shared/types.js'; export interface ServerTransformState extends TransformState { /** The $: calls, which will be ordered in the end */ readonly legacy_reactive_statements: Map; - readonly class_analysis: ClassAnalysis | null; + readonly class_analysis: ClassTransformer | null; } export interface ComponentServerTransformState extends ServerTransformState { diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/ClassBody.js index 8d67b3836155..3bab89cf9bee 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/ClassBody.js @@ -1,6 +1,6 @@ /** @import { ClassBody } from 'estree' */ /** @import { Context } from '../types.js' */ -import { create_server_class_analysis } from './shared/server-class-analysis.js'; +import { create_server_class_transformer } from './shared/server-class-transformer.js'; /** * @param {ClassBody} node @@ -12,8 +12,8 @@ export function ClassBody(node, context) { return; } - const class_analysis = create_server_class_analysis(node.body); - const body = class_analysis.generate_body(context); + const class_transformer = create_server_class_transformer(node.body); + const body = class_transformer.generate_body(context); return { ...node, body }; } diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/server-class-analysis.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/server-class-transformer.js similarity index 90% rename from packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/server-class-analysis.js rename to packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/server-class-transformer.js index 73390d0e38ad..7a52f1473106 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/server-class-analysis.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/server-class-transformer.js @@ -1,18 +1,18 @@ /** @import { Expression, MethodDefinition, StaticBlock, PropertyDefinition, SpreadElement } from 'estree' */ /** @import { Context } from '../../types.js' */ /** @import { AssignmentBuilder, StateFieldBuilder } from '../../../shared/types.js' */ -/** @import { ClassAnalysis } from '../../../shared/types.js' */ +/** @import { ClassTransformer } from '../../../shared/types.js' */ /** @import { StateCreationRuneName } from '../../../../../../utils.js' */ import * as b from '#compiler/builders'; import { dev } from '../../../../../state.js'; -import { create_class_analysis } from '../../../shared/class_analysis.js'; +import { create_class_transformer } from '../../../shared/class_transformer.js'; /** * @param {Array} body - * @returns {ClassAnalysis} + * @returns {ClassTransformer} */ -export function create_server_class_analysis(body) { +export function create_server_class_transformer(body) { /** @type {StateFieldBuilder} */ function build_state_field({ is_private, field, node, context }) { let original_id = node.type === 'AssignmentExpression' ? node.left.property : node.key; @@ -79,7 +79,7 @@ export function create_server_class_analysis(body) { }; } - return create_class_analysis(body, build_state_field, build_assignment); + return create_class_transformer(body, build_state_field, build_assignment); } /** diff --git a/packages/svelte/src/compiler/phases/3-transform/shared/class_analysis.js b/packages/svelte/src/compiler/phases/3-transform/shared/class_transformer.js similarity index 97% rename from packages/svelte/src/compiler/phases/3-transform/shared/class_analysis.js rename to packages/svelte/src/compiler/phases/3-transform/shared/class_transformer.js index 20d9e75eca18..5abd98c207d9 100644 --- a/packages/svelte/src/compiler/phases/3-transform/shared/class_analysis.js +++ b/packages/svelte/src/compiler/phases/3-transform/shared/class_transformer.js @@ -3,7 +3,7 @@ /** @import { Context as ClientContext } from '../client/types.js' */ /** @import { Context as ServerContext } from '../server/types.js' */ /** @import { StateCreationRuneName } from '../../../../utils.js' */ -/** @import { AssignmentBuilder, ClassAnalysis, StateFieldBuilder, StatefulAssignment, StatefulPropertyDefinition } from './types.js' */ +/** @import { AssignmentBuilder, ClassTransformer, StateFieldBuilder, StatefulAssignment, StatefulPropertyDefinition } from './types.js' */ /** @import { Scope } from '../../scope.js' */ import * as b from '#compiler/builders'; import { once } from '../../../../internal/server/index.js'; @@ -16,9 +16,9 @@ import { get_rune } from '../../scope.js'; * @param {Array} body * @param {StateFieldBuilder} build_state_field * @param {AssignmentBuilder} build_assignment - * @returns {ClassAnalysis} + * @returns {ClassTransformer} */ -export function create_class_analysis(body, build_state_field, build_assignment) { +export function create_class_transformer(body, build_state_field, build_assignment) { /** * Public, stateful fields. * @type {Map} diff --git a/packages/svelte/src/compiler/phases/3-transform/shared/types.d.ts b/packages/svelte/src/compiler/phases/3-transform/shared/types.d.ts index 2d02e705c317..7c03be2235ae 100644 --- a/packages/svelte/src/compiler/phases/3-transform/shared/types.d.ts +++ b/packages/svelte/src/compiler/phases/3-transform/shared/types.d.ts @@ -48,7 +48,7 @@ export type AssignmentBuilder = params: AssignmentBuilderParams ) => AssignmentExpression; -export type ClassAnalysis = { +export type ClassTransformer = { /** * @param name - The name of the field. * @param is_private - Whether the field is private (whether its name starts with '#'). From 1e0f423cd2b68320c44beb03e28aeacf82ec3314 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 15 May 2025 16:03:32 -0400 Subject: [PATCH 20/64] missed a spot --- .../compiler/phases/3-transform/client/transform-client.js | 4 ++-- .../svelte/src/compiler/phases/3-transform/client/types.d.ts | 2 +- .../3-transform/client/visitors/AssignmentExpression.js | 4 ++-- .../phases/3-transform/client/visitors/MemberExpression.js | 2 +- .../phases/3-transform/client/visitors/UpdateExpression.js | 2 +- .../compiler/phases/3-transform/server/transform-server.js | 4 ++-- .../svelte/src/compiler/phases/3-transform/server/types.d.ts | 2 +- .../3-transform/server/visitors/AssignmentExpression.js | 2 +- .../phases/3-transform/server/visitors/MemberExpression.js | 2 +- 9 files changed, 12 insertions(+), 12 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js index 0282a71f6de1..740bcae479c0 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js @@ -163,7 +163,7 @@ export function client_component(analysis, options) { }, events: new Set(), preserve_whitespace: options.preserveWhitespace, - class_analysis: null, + class_transformer: null, transform: {}, in_constructor: false, instance_level_snippets: [], @@ -672,7 +672,7 @@ export function client_module(analysis, options) { scopes: analysis.module.scopes, transform: {}, in_constructor: false, - class_analysis: null + class_transformer: null }; const module = /** @type {ESTree.Program} */ ( diff --git a/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts b/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts index ce201958da2c..c2292292da2e 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts +++ b/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts @@ -15,7 +15,7 @@ import type { SourceLocation } from '#shared'; import type { ClassTransformer } from '../shared/types.js'; export interface ClientTransformState extends TransformState { - readonly class_analysis: ClassTransformer | null; + readonly class_transformer: ClassTransformer | null; /** * `true` if the current lexical scope belongs to a class constructor. this allows diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js index 34d80d388ca1..d11938af9778 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js @@ -17,7 +17,7 @@ import { validate_mutation } from './shared/utils.js'; * @param {Context} context */ export function AssignmentExpression(node, context) { - const stripped_node = context.state.class_analysis?.generate_assignment(node, context); + const stripped_node = context.state.class_transformer?.generate_assignment(node, context); if (stripped_node) { return stripped_node; } @@ -61,7 +61,7 @@ function build_assignment(operator, left, right, context) { left.type === 'MemberExpression' && left.property.type === 'PrivateIdentifier' ) { - const private_state = context.state.class_analysis?.get_field(left.property.name, true); + const private_state = context.state.class_transformer?.get_field(left.property.name, true); if (private_state !== undefined) { let value = /** @type {Expression} */ ( diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/MemberExpression.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/MemberExpression.js index 87a886fd3b78..02052942f565 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/MemberExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/MemberExpression.js @@ -9,7 +9,7 @@ import * as b from '#compiler/builders'; export function MemberExpression(node, context) { // rewrite `this.#foo` as `this.#foo.v` inside a constructor if (node.property.type === 'PrivateIdentifier') { - const field = context.state.class_analysis?.get_field(node.property.name, true); + const field = context.state.class_transformer?.get_field(node.property.name, true); if (field) { return context.state.in_constructor && (field.kind === '$state.raw' || field.kind === '$state') diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/UpdateExpression.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/UpdateExpression.js index 2e6f9123ddc9..ca81fa14931f 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/UpdateExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/UpdateExpression.js @@ -15,7 +15,7 @@ export function UpdateExpression(node, context) { argument.type === 'MemberExpression' && argument.object.type === 'ThisExpression' && argument.property.type === 'PrivateIdentifier' && - context.state.class_analysis?.get_field(argument.property.name, true) + context.state.class_transformer?.get_field(argument.property.name, true) ) { let fn = '$.update'; if (node.prefix) fn += '_pre'; diff --git a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js index e159e2ff4e7a..661b73d65956 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js @@ -99,7 +99,7 @@ export function server_component(analysis, options) { template: /** @type {any} */ (null), namespace: options.namespace, preserve_whitespace: options.preserveWhitespace, - class_analysis: null, + class_transformer: null, skip_hydration_boundaries: false }; @@ -395,7 +395,7 @@ export function server_module(analysis, options) { // to be present for `javascript_visitors_legacy` and so is included in module // transform state as well as component transform state legacy_reactive_statements: new Map(), - class_analysis: null + class_transformer: null }; const module = /** @type {Program} */ ( diff --git a/packages/svelte/src/compiler/phases/3-transform/server/types.d.ts b/packages/svelte/src/compiler/phases/3-transform/server/types.d.ts index 9d2d3164a9ef..24b35d37612b 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/types.d.ts +++ b/packages/svelte/src/compiler/phases/3-transform/server/types.d.ts @@ -7,7 +7,7 @@ import type { ClassTransformer } from '../shared/types.js'; export interface ServerTransformState extends TransformState { /** The $: calls, which will be ordered in the end */ readonly legacy_reactive_statements: Map; - readonly class_analysis: ClassTransformer | null; + readonly class_transformer: ClassTransformer | null; } export interface ComponentServerTransformState extends ServerTransformState { diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/AssignmentExpression.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/AssignmentExpression.js index ae84f0878228..939dd45ca28c 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/AssignmentExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/AssignmentExpression.js @@ -10,7 +10,7 @@ import { visit_assignment_expression } from '../../shared/assignments.js'; * @param {Context} context */ export function AssignmentExpression(node, context) { - const stripped_node = context.state.class_analysis?.generate_assignment(node, context); + const stripped_node = context.state.class_transformer?.generate_assignment(node, context); if (stripped_node) { return stripped_node; } diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/MemberExpression.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/MemberExpression.js index 2f72327191d0..2a90307293fb 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/MemberExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/MemberExpression.js @@ -12,7 +12,7 @@ export function MemberExpression(node, context) { node.object.type === 'ThisExpression' && node.property.type === 'PrivateIdentifier' ) { - const field = context.state.class_analysis?.get_field(node.property.name, true, [ + const field = context.state.class_transformer?.get_field(node.property.name, true, [ '$derived', '$derived.by' ]); From f81405cb87e2389f68e34f754b580e6c9a595f20 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 15 May 2025 16:04:36 -0400 Subject: [PATCH 21/64] and another --- .../compiler/phases/3-transform/shared/class_transformer.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/shared/class_transformer.js b/packages/svelte/src/compiler/phases/3-transform/shared/class_transformer.js index 5abd98c207d9..469a178cc295 100644 --- a/packages/svelte/src/compiler/phases/3-transform/shared/class_transformer.js +++ b/packages/svelte/src/compiler/phases/3-transform/shared/class_transformer.js @@ -90,7 +90,7 @@ export function create_class_transformer(body, build_state_field, build_assignme function create_child_context(context) { const state = { ...context.state, - class_analysis + class_transformer }; // @ts-expect-error - I can't find a way to make TypeScript happy with these const visit = (node, state_override) => context.visit(node, { ...state, ...state_override }); @@ -354,13 +354,13 @@ export function create_class_transformer(body, build_state_field, build_assignme } } - const class_analysis = { + const class_transformer = { get_field, generate_body, generate_assignment }; - return class_analysis; + return class_transformer; } /** From 41e8ade5628381a4fc5ddf1c4e5dd7f3723b13bf Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 15 May 2025 16:12:00 -0400 Subject: [PATCH 22/64] store analysis for use during transformation --- packages/svelte/src/compiler/phases/2-analyze/index.js | 9 ++++++--- packages/svelte/src/compiler/phases/2-analyze/types.d.ts | 5 ++++- .../phases/2-analyze/visitors/AssignmentExpression.js | 2 +- .../compiler/phases/2-analyze/visitors/CallExpression.js | 2 +- .../src/compiler/phases/2-analyze/visitors/ClassBody.js | 2 +- .../phases/2-analyze/visitors/PropertyDefinition.js | 2 +- .../phases/2-analyze/visitors/shared/class-analysis.js | 2 +- 7 files changed, 15 insertions(+), 9 deletions(-) diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index 3c586179c832..63cfc7406409 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -267,7 +267,8 @@ export function analyze_module(ast, options) { scope, scopes, analysis: /** @type {ComponentAnalysis} */ (analysis), - class_state: null, + classes: new Map(), + class: null, // TODO the following are not needed for modules, but we have to pass them in order to avoid type error, // and reducing the type would result in a lot of tedious type casts elsewhere - find a good solution one day ast_type: /** @type {any} */ (null), @@ -626,7 +627,8 @@ export function analyze_component(root, source, options) { has_props_rune: false, component_slots: new Set(), expression: null, - class_state: null, + classes: new Map(), + class: null, function_depth: scope.function_depth, reactive_statement: null }; @@ -693,7 +695,8 @@ export function analyze_component(root, source, options) { reactive_statement: null, component_slots: new Set(), expression: null, - class_state: null, + classes: new Map(), + class: null, function_depth: scope.function_depth }; diff --git a/packages/svelte/src/compiler/phases/2-analyze/types.d.ts b/packages/svelte/src/compiler/phases/2-analyze/types.d.ts index 5f9887e98980..c01a6fceb21c 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/types.d.ts +++ b/packages/svelte/src/compiler/phases/2-analyze/types.d.ts @@ -2,6 +2,7 @@ import type { Scope } from '../scope.js'; import type { ComponentAnalysis, ReactiveStatement } from '../types.js'; import type { AST, ExpressionMetadata, ValidatedCompileOptions } from '#compiler'; import type { ClassAnalysis } from './visitors/shared/class-analysis.js'; +import type { ClassBody } from 'estree'; export interface AnalysisState { scope: Scope; @@ -21,7 +22,9 @@ export interface AnalysisState { expression: ExpressionMetadata | null; /** Used to analyze class state. */ - class_state: ClassAnalysis | null; + classes: Map; + class: ClassAnalysis | null; + function_depth: number; // legacy stuff diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/AssignmentExpression.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/AssignmentExpression.js index 9a94fc668ed9..d69ef71db191 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/AssignmentExpression.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/AssignmentExpression.js @@ -23,6 +23,6 @@ export function AssignmentExpression(node, context) { } } - context.state.class_state?.register?.(node, context); + context.state.class?.register?.(node, context); context.next(); } 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 58c7447f2cd3..009f688e08ab 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js @@ -118,7 +118,7 @@ export function CallExpression(node, context) { const valid = is_variable_declaration(parent, context) || is_class_property_definition(parent) || - context.state.class_state?.is_class_property_assignment_at_constructor_root( + context.state.class?.is_class_property_assignment_at_constructor_root( parent, context.path.slice(0, -1) ); diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js index 30364a14f10f..a64e2fd4f9a0 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js @@ -9,6 +9,6 @@ import { ClassAnalysis } from './shared/class-analysis.js'; export function ClassBody(node, context) { context.next({ ...context.state, - class_state: context.state.analysis.runes ? new ClassAnalysis() : null + class: context.state.analysis.runes ? new ClassAnalysis() : null }); } diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/PropertyDefinition.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/PropertyDefinition.js index aed7ff7fe8e8..ea072d168d00 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/PropertyDefinition.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/PropertyDefinition.js @@ -7,6 +7,6 @@ * @param {Context} context */ export function PropertyDefinition(node, context) { - context.state.class_state?.register(node, context); + context.state.class?.register(node, context); context.next(); } diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/class-analysis.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/class-analysis.js index 27be9261f885..283e7d9edfb5 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/class-analysis.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/class-analysis.js @@ -140,7 +140,7 @@ export class ClassAnalysis { return; } - e.constructor_state_reassignment(node, name, locate_node(existing.node)); + e.constructor_state_reassignment(node); } /** From 1d1f0eb1b51dfa64427a3dad3b66543a4b7d7a9e Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 15 May 2025 16:16:21 -0400 Subject: [PATCH 23/64] move code to where it is used --- .../phases/2-analyze/visitors/ClassBody.js | 180 +++++++++++++++++- .../visitors/shared/class-analysis.js | 172 ----------------- 2 files changed, 177 insertions(+), 175 deletions(-) delete mode 100644 packages/svelte/src/compiler/phases/2-analyze/visitors/shared/class-analysis.js diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js index a64e2fd4f9a0..2d9b4018a644 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js @@ -1,14 +1,188 @@ -/** @import { ClassBody } from 'estree' */ +/** @import { AssignmentExpression, ClassBody, PropertyDefinition, Expression, Identifier, PrivateIdentifier, Literal } from 'estree' */ +/** @import { AST } from '#compiler' */ /** @import { Context } from '../types' */ -import { ClassAnalysis } from './shared/class-analysis.js'; +/** @import { StateCreationRuneName } from '../../../../utils.js' */ +import { get_parent } from '../../../utils/ast.js'; +import { get_rune } from '../../scope.js'; +import * as e from '../../../errors.js'; +import { is_state_creation_rune } from '../../../../utils.js'; /** * @param {ClassBody} node * @param {Context} context */ export function ClassBody(node, context) { + if (!context.state.analysis.runes) { + context.next(); + return; + } + + const analyzed = new ClassAnalysis(); + context.next({ ...context.state, - class: context.state.analysis.runes ? new ClassAnalysis() : null + class: analyzed }); } + +/** @typedef { StateCreationRuneName | 'regular'} PropertyAssignmentType */ +/** @typedef {{ type: PropertyAssignmentType; node: AssignmentExpression | PropertyDefinition; }} PropertyAssignmentDetails */ + +const reassignable_assignments = new Set(['$state', '$state.raw', 'regular']); + +class ClassAnalysis { + /** @type {Map} */ + #public_assignments = new Map(); + + /** @type {Map} */ + #private_assignments = new Map(); + + /** + * Determines if the node is a valid assignment to a class property, and if so, + * registers the assignment. + * @param {AssignmentExpression | PropertyDefinition} node + * @param {Context} context + */ + register(node, context) { + /** @type {string} */ + let name; + /** @type {PropertyAssignmentType} */ + let type; + /** @type {boolean} */ + let is_private; + + if (node.type === 'AssignmentExpression') { + if (!this.is_class_property_assignment_at_constructor_root(node, context.path)) { + return; + } + + let maybe_name = get_name_for_identifier(node.left.property); + if (!maybe_name) { + return; + } + + name = maybe_name; + type = this.#get_assignment_type(node, context); + is_private = node.left.property.type === 'PrivateIdentifier'; + + this.#check_for_conflicts(node, name, type, is_private); + } else { + if (!this.#is_assigned_property(node)) { + return; + } + + let maybe_name = get_name_for_identifier(node.key); + if (!maybe_name) { + return; + } + + name = maybe_name; + type = this.#get_assignment_type(node, context); + is_private = node.key.type === 'PrivateIdentifier'; + + // we don't need to check for conflicts here because they're not possible yet + } + + // we don't have to validate anything other than conflicts here, because the rune placement rules + // catch all of the other weirdness. + const map = is_private ? this.#private_assignments : this.#public_assignments; + if (!map.has(name)) { + map.set(name, { type, node }); + } + } + + /** + * @template {AST.SvelteNode} T + * @param {AST.SvelteNode} node + * @param {T[]} path + * @returns {node is AssignmentExpression & { left: { type: 'MemberExpression' } & { object: { type: 'ThisExpression' }; property: { type: 'Identifier' | 'PrivateIdentifier' | 'Literal' } } }} + */ + is_class_property_assignment_at_constructor_root(node, path) { + if ( + !( + node.type === 'AssignmentExpression' && + node.operator === '=' && + node.left.type === 'MemberExpression' && + node.left.object.type === 'ThisExpression' && + ((node.left.property.type === 'Identifier' && !node.left.computed) || + node.left.property.type === 'PrivateIdentifier' || + node.left.property.type === 'Literal') + ) + ) { + return false; + } + // AssignmentExpression (here) -> ExpressionStatement (-1) -> BlockStatement (-2) -> FunctionExpression (-3) -> MethodDefinition (-4) + const maybe_constructor = get_parent(path, -4); + return ( + maybe_constructor && + maybe_constructor.type === 'MethodDefinition' && + maybe_constructor.kind === 'constructor' + ); + } + + /** + * We only care about properties that have values assigned to them -- if they don't, + * they can't be a conflict for state declared in the constructor. + * @param {PropertyDefinition} node + * @returns {node is PropertyDefinition & { key: { type: 'PrivateIdentifier' | 'Identifier' | 'Literal' }; value: Expression; static: false; computed: false }} + */ + #is_assigned_property(node) { + return ( + (node.key.type === 'PrivateIdentifier' || + node.key.type === 'Identifier' || + node.key.type === 'Literal') && + Boolean(node.value) && + !node.static && + !node.computed + ); + } + + /** + * Checks for conflicts with existing assignments. A conflict occurs if: + * - The original assignment used `$derived` or `$derived.by` (these can never be reassigned) + * - The original assignment used `$state`, `$state.raw`, or `regular` and is being assigned to with any type other than `regular` + * @param {AssignmentExpression} node + * @param {string} name + * @param {PropertyAssignmentType} type + * @param {boolean} is_private + */ + #check_for_conflicts(node, name, type, is_private) { + const existing = (is_private ? this.#private_assignments : this.#public_assignments).get(name); + if (!existing) { + return; + } + + if (reassignable_assignments.has(existing.type) && type === 'regular') { + return; + } + + e.constructor_state_reassignment(node); + } + + /** + * @param {AssignmentExpression | PropertyDefinition} node + * @param {Context} context + * @returns {PropertyAssignmentType} + */ + #get_assignment_type(node, context) { + const value = node.type === 'AssignmentExpression' ? node.right : node.value; + const rune = get_rune(value, context.state.scope); + if (rune === null) { + return 'regular'; + } + if (is_state_creation_rune(rune)) { + return rune; + } + // this does mean we return `regular` for some other runes (like `$trace` or `$state.raw`) + // -- this is ok because the rune placement rules will throw if they're invalid. + return 'regular'; + } +} + +/** + * + * @param {PrivateIdentifier | Identifier | Literal} node + */ +function get_name_for_identifier(node) { + return node.type === 'Literal' ? node.value?.toString() : node.name; +} diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/class-analysis.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/class-analysis.js deleted file mode 100644 index 283e7d9edfb5..000000000000 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/class-analysis.js +++ /dev/null @@ -1,172 +0,0 @@ -/** @import { AssignmentExpression, PropertyDefinition, Expression, Identifier, PrivateIdentifier, Literal, MethodDefinition } from 'estree' */ -/** @import { AST } from '#compiler' */ -/** @import { Context } from '../../types' */ -/** @import { StateCreationRuneName } from '../../../../../utils.js' */ - -import { get_parent } from '../../../../utils/ast.js'; -import { get_rune } from '../../../scope.js'; -import * as e from '../../../../errors.js'; -import { locate_node } from '../../../../state.js'; -import { is_state_creation_rune } from '../../../../../utils.js'; - -/** @typedef { StateCreationRuneName | 'regular'} PropertyAssignmentType */ -/** @typedef {{ type: PropertyAssignmentType; node: AssignmentExpression | PropertyDefinition; }} PropertyAssignmentDetails */ - -const reassignable_assignments = new Set(['$state', '$state.raw', 'regular']); - -export class ClassAnalysis { - /** @type {Map} */ - #public_assignments = new Map(); - - /** @type {Map} */ - #private_assignments = new Map(); - - /** - * Determines if the node is a valid assignment to a class property, and if so, - * registers the assignment. - * @param {AssignmentExpression | PropertyDefinition} node - * @param {Context} context - */ - register(node, context) { - /** @type {string} */ - let name; - /** @type {PropertyAssignmentType} */ - let type; - /** @type {boolean} */ - let is_private; - - if (node.type === 'AssignmentExpression') { - if (!this.is_class_property_assignment_at_constructor_root(node, context.path)) { - return; - } - - let maybe_name = get_name_for_identifier(node.left.property); - if (!maybe_name) { - return; - } - - name = maybe_name; - type = this.#get_assignment_type(node, context); - is_private = node.left.property.type === 'PrivateIdentifier'; - - this.#check_for_conflicts(node, name, type, is_private); - } else { - if (!this.#is_assigned_property(node)) { - return; - } - - let maybe_name = get_name_for_identifier(node.key); - if (!maybe_name) { - return; - } - - name = maybe_name; - type = this.#get_assignment_type(node, context); - is_private = node.key.type === 'PrivateIdentifier'; - - // we don't need to check for conflicts here because they're not possible yet - } - - // we don't have to validate anything other than conflicts here, because the rune placement rules - // catch all of the other weirdness. - const map = is_private ? this.#private_assignments : this.#public_assignments; - if (!map.has(name)) { - map.set(name, { type, node }); - } - } - - /** - * @template {AST.SvelteNode} T - * @param {AST.SvelteNode} node - * @param {T[]} path - * @returns {node is AssignmentExpression & { left: { type: 'MemberExpression' } & { object: { type: 'ThisExpression' }; property: { type: 'Identifier' | 'PrivateIdentifier' | 'Literal' } } }} - */ - is_class_property_assignment_at_constructor_root(node, path) { - if ( - !( - node.type === 'AssignmentExpression' && - node.operator === '=' && - node.left.type === 'MemberExpression' && - node.left.object.type === 'ThisExpression' && - ((node.left.property.type === 'Identifier' && !node.left.computed) || - node.left.property.type === 'PrivateIdentifier' || - node.left.property.type === 'Literal') - ) - ) { - return false; - } - // AssignmentExpression (here) -> ExpressionStatement (-1) -> BlockStatement (-2) -> FunctionExpression (-3) -> MethodDefinition (-4) - const maybe_constructor = get_parent(path, -4); - return ( - maybe_constructor && - maybe_constructor.type === 'MethodDefinition' && - maybe_constructor.kind === 'constructor' - ); - } - - /** - * We only care about properties that have values assigned to them -- if they don't, - * they can't be a conflict for state declared in the constructor. - * @param {PropertyDefinition} node - * @returns {node is PropertyDefinition & { key: { type: 'PrivateIdentifier' | 'Identifier' | 'Literal' }; value: Expression; static: false; computed: false }} - */ - #is_assigned_property(node) { - return ( - (node.key.type === 'PrivateIdentifier' || - node.key.type === 'Identifier' || - node.key.type === 'Literal') && - Boolean(node.value) && - !node.static && - !node.computed - ); - } - - /** - * Checks for conflicts with existing assignments. A conflict occurs if: - * - The original assignment used `$derived` or `$derived.by` (these can never be reassigned) - * - The original assignment used `$state`, `$state.raw`, or `regular` and is being assigned to with any type other than `regular` - * @param {AssignmentExpression} node - * @param {string} name - * @param {PropertyAssignmentType} type - * @param {boolean} is_private - */ - #check_for_conflicts(node, name, type, is_private) { - const existing = (is_private ? this.#private_assignments : this.#public_assignments).get(name); - if (!existing) { - return; - } - - if (reassignable_assignments.has(existing.type) && type === 'regular') { - return; - } - - e.constructor_state_reassignment(node); - } - - /** - * @param {AssignmentExpression | PropertyDefinition} node - * @param {Context} context - * @returns {PropertyAssignmentType} - */ - #get_assignment_type(node, context) { - const value = node.type === 'AssignmentExpression' ? node.right : node.value; - const rune = get_rune(value, context.state.scope); - if (rune === null) { - return 'regular'; - } - if (is_state_creation_rune(rune)) { - return rune; - } - // this does mean we return `regular` for some other runes (like `$trace` or `$state.raw`) - // -- this is ok because the rune placement rules will throw if they're invalid. - return 'regular'; - } -} - -/** - * - * @param {PrivateIdentifier | Identifier | Literal} node - */ -function get_name_for_identifier(node) { - return node.type === 'Literal' ? node.value?.toString() : node.name; -} From 823e66f06f65d12bf12a3cc8715c05662ca24115 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 15 May 2025 17:29:17 -0400 Subject: [PATCH 24/64] do the analysis upfront, it's way simpler --- .../src/compiler/phases/2-analyze/index.js | 2 - .../visitors/AssignmentExpression.js | 1 - .../phases/2-analyze/visitors/ClassBody.js | 194 +++++++----------- .../2-analyze/visitors/PropertyDefinition.js | 12 -- 4 files changed, 70 insertions(+), 139 deletions(-) delete mode 100644 packages/svelte/src/compiler/phases/2-analyze/visitors/PropertyDefinition.js diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index 63cfc7406409..511bc50646c7 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -48,7 +48,6 @@ import { Literal } from './visitors/Literal.js'; import { MemberExpression } from './visitors/MemberExpression.js'; import { NewExpression } from './visitors/NewExpression.js'; import { OnDirective } from './visitors/OnDirective.js'; -import { PropertyDefinition } from './visitors/PropertyDefinition.js'; import { RegularElement } from './visitors/RegularElement.js'; import { RenderTag } from './visitors/RenderTag.js'; import { SlotElement } from './visitors/SlotElement.js'; @@ -165,7 +164,6 @@ const visitors = { MemberExpression, NewExpression, OnDirective, - PropertyDefinition, RegularElement, RenderTag, SlotElement, diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/AssignmentExpression.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/AssignmentExpression.js index d69ef71db191..a64c89cd88f1 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/AssignmentExpression.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/AssignmentExpression.js @@ -23,6 +23,5 @@ export function AssignmentExpression(node, context) { } } - context.state.class?.register?.(node, context); context.next(); } diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js index 2d9b4018a644..233223de69ec 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js @@ -1,4 +1,4 @@ -/** @import { AssignmentExpression, ClassBody, PropertyDefinition, Expression, Identifier, PrivateIdentifier, Literal } from 'estree' */ +/** @import { AssignmentExpression, ClassBody, PropertyDefinition, Expression, Identifier, PrivateIdentifier, Literal, MethodDefinition } from 'estree' */ /** @import { AST } from '#compiler' */ /** @import { Context } from '../types' */ /** @import { StateCreationRuneName } from '../../../../utils.js' */ @@ -19,78 +19,90 @@ export function ClassBody(node, context) { const analyzed = new ClassAnalysis(); - context.next({ - ...context.state, - class: analyzed - }); -} + /** @type {string[]} */ + const seen = []; -/** @typedef { StateCreationRuneName | 'regular'} PropertyAssignmentType */ -/** @typedef {{ type: PropertyAssignmentType; node: AssignmentExpression | PropertyDefinition; }} PropertyAssignmentDetails */ - -const reassignable_assignments = new Set(['$state', '$state.raw', 'regular']); - -class ClassAnalysis { - /** @type {Map} */ - #public_assignments = new Map(); - - /** @type {Map} */ - #private_assignments = new Map(); + /** @type {MethodDefinition | null} */ + let constructor = null; /** - * Determines if the node is a valid assignment to a class property, and if so, - * registers the assignment. - * @param {AssignmentExpression | PropertyDefinition} node - * @param {Context} context + * @param {PropertyDefinition | AssignmentExpression} node + * @param {Expression | PrivateIdentifier} key + * @param {Expression | null | undefined} value */ - register(node, context) { - /** @type {string} */ - let name; - /** @type {PropertyAssignmentType} */ - let type; - /** @type {boolean} */ - let is_private; - - if (node.type === 'AssignmentExpression') { - if (!this.is_class_property_assignment_at_constructor_root(node, context.path)) { - return; - } - - let maybe_name = get_name_for_identifier(node.left.property); - if (!maybe_name) { - return; - } + function handle(node, key, value) { + const name = get_name(key); + if (!name) return; - name = maybe_name; - type = this.#get_assignment_type(node, context); - is_private = node.left.property.type === 'PrivateIdentifier'; + const rune = get_rune(value, context.state.scope); - this.#check_for_conflicts(node, name, type, is_private); - } else { - if (!this.#is_assigned_property(node)) { - return; + if (rune && is_state_creation_rune(rune)) { + if (seen.includes(name)) { + e.constructor_state_reassignment(node); // TODO the same thing applies to duplicate fields, so the code/message needs to change } - let maybe_name = get_name_for_identifier(node.key); - if (!maybe_name) { - return; - } + analyzed.fields[name] = { + node, + type: rune + }; + } - name = maybe_name; - type = this.#get_assignment_type(node, context); - is_private = node.key.type === 'PrivateIdentifier'; + if (value) { + seen.push(name); + } + } + + for (const child of node.body) { + if (child.type === 'PropertyDefinition' && !child.computed) { + handle(child, child.key, child.value); + } - // we don't need to check for conflicts here because they're not possible yet + if ( + child.type === 'MethodDefinition' && + child.key.type === 'Identifier' && + child.key.name === 'constructor' + ) { + constructor = child; } + } + + if (constructor) { + for (const statement of constructor.value.body.body) { + if (statement.type !== 'ExpressionStatement') continue; + if (statement.expression.type !== 'AssignmentExpression') continue; - // we don't have to validate anything other than conflicts here, because the rune placement rules - // catch all of the other weirdness. - const map = is_private ? this.#private_assignments : this.#public_assignments; - if (!map.has(name)) { - map.set(name, { type, node }); + const { left, right } = statement.expression; + + if (left.type !== 'MemberExpression') continue; + if (left.object.type !== 'ThisExpression') continue; + if (left.computed && left.property.type !== 'Literal') continue; + + handle(statement.expression, left.property, right); } } + context.next({ + ...context.state, + class: analyzed + }); +} + +/** @param {Expression | PrivateIdentifier} node */ +function get_name(node) { + if (node.type === 'Literal') return String(node.value); + if (node.type === 'PrivateIdentifier') return '#' + node.name; + if (node.type === 'Identifier') return node.name; + + return null; +} + +/** @typedef { StateCreationRuneName | 'regular'} PropertyAssignmentType */ +/** @typedef {{ type: PropertyAssignmentType; node: AssignmentExpression | PropertyDefinition; }} PropertyAssignmentDetails */ + +class ClassAnalysis { + /** @type {Record} */ + fields = {}; + /** * @template {AST.SvelteNode} T * @param {AST.SvelteNode} node @@ -119,70 +131,4 @@ class ClassAnalysis { maybe_constructor.kind === 'constructor' ); } - - /** - * We only care about properties that have values assigned to them -- if they don't, - * they can't be a conflict for state declared in the constructor. - * @param {PropertyDefinition} node - * @returns {node is PropertyDefinition & { key: { type: 'PrivateIdentifier' | 'Identifier' | 'Literal' }; value: Expression; static: false; computed: false }} - */ - #is_assigned_property(node) { - return ( - (node.key.type === 'PrivateIdentifier' || - node.key.type === 'Identifier' || - node.key.type === 'Literal') && - Boolean(node.value) && - !node.static && - !node.computed - ); - } - - /** - * Checks for conflicts with existing assignments. A conflict occurs if: - * - The original assignment used `$derived` or `$derived.by` (these can never be reassigned) - * - The original assignment used `$state`, `$state.raw`, or `regular` and is being assigned to with any type other than `regular` - * @param {AssignmentExpression} node - * @param {string} name - * @param {PropertyAssignmentType} type - * @param {boolean} is_private - */ - #check_for_conflicts(node, name, type, is_private) { - const existing = (is_private ? this.#private_assignments : this.#public_assignments).get(name); - if (!existing) { - return; - } - - if (reassignable_assignments.has(existing.type) && type === 'regular') { - return; - } - - e.constructor_state_reassignment(node); - } - - /** - * @param {AssignmentExpression | PropertyDefinition} node - * @param {Context} context - * @returns {PropertyAssignmentType} - */ - #get_assignment_type(node, context) { - const value = node.type === 'AssignmentExpression' ? node.right : node.value; - const rune = get_rune(value, context.state.scope); - if (rune === null) { - return 'regular'; - } - if (is_state_creation_rune(rune)) { - return rune; - } - // this does mean we return `regular` for some other runes (like `$trace` or `$state.raw`) - // -- this is ok because the rune placement rules will throw if they're invalid. - return 'regular'; - } -} - -/** - * - * @param {PrivateIdentifier | Identifier | Literal} node - */ -function get_name_for_identifier(node) { - return node.type === 'Literal' ? node.value?.toString() : node.name; } diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/PropertyDefinition.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/PropertyDefinition.js deleted file mode 100644 index ea072d168d00..000000000000 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/PropertyDefinition.js +++ /dev/null @@ -1,12 +0,0 @@ -/** @import { PropertyDefinition } from 'estree' */ -/** @import { Context } from '../types' */ - -/** - * - * @param {PropertyDefinition} node - * @param {Context} context - */ -export function PropertyDefinition(node, context) { - context.state.class?.register(node, context); - context.next(); -} From 0dcd3cd2c1ab1c7a7eb64bc0a82707031b4955dd Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 15 May 2025 17:30:47 -0400 Subject: [PATCH 25/64] skip failing test for now --- .../validator/samples/class-state-constructor-9/_config.js | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 packages/svelte/tests/validator/samples/class-state-constructor-9/_config.js diff --git a/packages/svelte/tests/validator/samples/class-state-constructor-9/_config.js b/packages/svelte/tests/validator/samples/class-state-constructor-9/_config.js new file mode 100644 index 000000000000..18cc8bd6d146 --- /dev/null +++ b/packages/svelte/tests/validator/samples/class-state-constructor-9/_config.js @@ -0,0 +1,5 @@ +import { test } from '../../test'; + +export default test({ + skip: true // TODO delete this file so the test runs +}); From 7341f403afb1eb6e39223979c3155b51108c9a2a Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 15 May 2025 17:34:52 -0400 Subject: [PATCH 26/64] simplify --- .../2-analyze/visitors/CallExpression.js | 34 ++++++++++++++++--- .../phases/2-analyze/visitors/ClassBody.js | 33 +----------------- 2 files changed, 31 insertions(+), 36 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 009f688e08ab..ba690eb661bc 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js @@ -118,10 +118,7 @@ export function CallExpression(node, context) { const valid = is_variable_declaration(parent, context) || is_class_property_definition(parent) || - context.state.class?.is_class_property_assignment_at_constructor_root( - parent, - context.path.slice(0, -1) - ); + is_class_property_assignment_at_constructor_root(parent, context); if (!valid) { e.state_invalid_placement(node, rune); @@ -292,3 +289,32 @@ function is_variable_declaration(parent, context) { function is_class_property_definition(parent) { return parent.type === 'PropertyDefinition' && !parent.static && !parent.computed; } + +/** + * @param {AST.SvelteNode} node + * @param {Context} context + * @returns {node is AssignmentExpression & { left: { type: 'MemberExpression' } & { object: { type: 'ThisExpression' }; property: { type: 'Identifier' | 'PrivateIdentifier' | 'Literal' } } }} + */ +function is_class_property_assignment_at_constructor_root(node, context) { + if ( + !( + node.type === 'AssignmentExpression' && + node.operator === '=' && + node.left.type === 'MemberExpression' && + node.left.object.type === 'ThisExpression' && + ((node.left.property.type === 'Identifier' && !node.left.computed) || + node.left.property.type === 'PrivateIdentifier' || + node.left.property.type === 'Literal') + ) + ) { + return false; + } + + // AssignmentExpression (here) -> ExpressionStatement (-1) -> BlockStatement (-2) -> FunctionExpression (-3) -> MethodDefinition (-4) + const maybe_constructor = get_parent(context.path, -5); + return ( + maybe_constructor && + maybe_constructor.type === 'MethodDefinition' && + maybe_constructor.kind === 'constructor' + ); +} diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js index 233223de69ec..9392c298d80e 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js @@ -1,8 +1,6 @@ -/** @import { AssignmentExpression, ClassBody, PropertyDefinition, Expression, Identifier, PrivateIdentifier, Literal, MethodDefinition } from 'estree' */ -/** @import { AST } from '#compiler' */ +/** @import { AssignmentExpression, ClassBody, PropertyDefinition, Expression, PrivateIdentifier, MethodDefinition } from 'estree' */ /** @import { Context } from '../types' */ /** @import { StateCreationRuneName } from '../../../../utils.js' */ -import { get_parent } from '../../../utils/ast.js'; import { get_rune } from '../../scope.js'; import * as e from '../../../errors.js'; import { is_state_creation_rune } from '../../../../utils.js'; @@ -102,33 +100,4 @@ function get_name(node) { class ClassAnalysis { /** @type {Record} */ fields = {}; - - /** - * @template {AST.SvelteNode} T - * @param {AST.SvelteNode} node - * @param {T[]} path - * @returns {node is AssignmentExpression & { left: { type: 'MemberExpression' } & { object: { type: 'ThisExpression' }; property: { type: 'Identifier' | 'PrivateIdentifier' | 'Literal' } } }} - */ - is_class_property_assignment_at_constructor_root(node, path) { - if ( - !( - node.type === 'AssignmentExpression' && - node.operator === '=' && - node.left.type === 'MemberExpression' && - node.left.object.type === 'ThisExpression' && - ((node.left.property.type === 'Identifier' && !node.left.computed) || - node.left.property.type === 'PrivateIdentifier' || - node.left.property.type === 'Literal') - ) - ) { - return false; - } - // AssignmentExpression (here) -> ExpressionStatement (-1) -> BlockStatement (-2) -> FunctionExpression (-3) -> MethodDefinition (-4) - const maybe_constructor = get_parent(path, -4); - return ( - maybe_constructor && - maybe_constructor.type === 'MethodDefinition' && - maybe_constructor.kind === 'constructor' - ); - } } From 75680a9a5eec4f243dc95b4c317e067b1484ccf3 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 15 May 2025 17:44:05 -0400 Subject: [PATCH 27/64] get rid of the class --- .../src/compiler/phases/2-analyze/index.js | 6 +++--- .../src/compiler/phases/2-analyze/types.d.ts | 7 +++---- .../phases/2-analyze/visitors/ClassBody.js | 17 +++++------------ packages/svelte/src/compiler/phases/types.d.ts | 3 ++- packages/svelte/src/compiler/types/index.d.ts | 9 +++++++++ 5 files changed, 22 insertions(+), 20 deletions(-) diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index 511bc50646c7..c29ddd9146a5 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -266,7 +266,7 @@ export function analyze_module(ast, options) { scopes, analysis: /** @type {ComponentAnalysis} */ (analysis), classes: new Map(), - class: null, + state_fields: null, // TODO the following are not needed for modules, but we have to pass them in order to avoid type error, // and reducing the type would result in a lot of tedious type casts elsewhere - find a good solution one day ast_type: /** @type {any} */ (null), @@ -626,7 +626,7 @@ export function analyze_component(root, source, options) { component_slots: new Set(), expression: null, classes: new Map(), - class: null, + state_fields: null, function_depth: scope.function_depth, reactive_statement: null }; @@ -694,7 +694,7 @@ export function analyze_component(root, source, options) { component_slots: new Set(), expression: null, classes: new Map(), - class: null, + state_fields: null, function_depth: scope.function_depth }; diff --git a/packages/svelte/src/compiler/phases/2-analyze/types.d.ts b/packages/svelte/src/compiler/phases/2-analyze/types.d.ts index c01a6fceb21c..97b6029f7958 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/types.d.ts +++ b/packages/svelte/src/compiler/phases/2-analyze/types.d.ts @@ -1,7 +1,6 @@ import type { Scope } from '../scope.js'; import type { ComponentAnalysis, ReactiveStatement } from '../types.js'; -import type { AST, ExpressionMetadata, ValidatedCompileOptions } from '#compiler'; -import type { ClassAnalysis } from './visitors/shared/class-analysis.js'; +import type { AST, ExpressionMetadata, StateFields, ValidatedCompileOptions } from '#compiler'; import type { ClassBody } from 'estree'; export interface AnalysisState { @@ -22,8 +21,8 @@ export interface AnalysisState { expression: ExpressionMetadata | null; /** Used to analyze class state. */ - classes: Map; - class: ClassAnalysis | null; + classes: Map; + state_fields: StateFields | null; function_depth: number; diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js index 9392c298d80e..6c5055bc17cb 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js @@ -1,6 +1,6 @@ /** @import { AssignmentExpression, ClassBody, PropertyDefinition, Expression, PrivateIdentifier, MethodDefinition } from 'estree' */ +/** @import { StateFields } from '#compiler' */ /** @import { Context } from '../types' */ -/** @import { StateCreationRuneName } from '../../../../utils.js' */ import { get_rune } from '../../scope.js'; import * as e from '../../../errors.js'; import { is_state_creation_rune } from '../../../../utils.js'; @@ -15,7 +15,8 @@ export function ClassBody(node, context) { return; } - const analyzed = new ClassAnalysis(); + /** @type {StateFields} */ + const state_fields = {}; /** @type {string[]} */ const seen = []; @@ -39,7 +40,7 @@ export function ClassBody(node, context) { e.constructor_state_reassignment(node); // TODO the same thing applies to duplicate fields, so the code/message needs to change } - analyzed.fields[name] = { + state_fields[name] = { node, type: rune }; @@ -81,7 +82,7 @@ export function ClassBody(node, context) { context.next({ ...context.state, - class: analyzed + state_fields }); } @@ -93,11 +94,3 @@ function get_name(node) { return null; } - -/** @typedef { StateCreationRuneName | 'regular'} PropertyAssignmentType */ -/** @typedef {{ type: PropertyAssignmentType; node: AssignmentExpression | PropertyDefinition; }} PropertyAssignmentDetails */ - -class ClassAnalysis { - /** @type {Record} */ - fields = {}; -} diff --git a/packages/svelte/src/compiler/phases/types.d.ts b/packages/svelte/src/compiler/phases/types.d.ts index f98cbe141567..22e749c97b81 100644 --- a/packages/svelte/src/compiler/phases/types.d.ts +++ b/packages/svelte/src/compiler/phases/types.d.ts @@ -1,6 +1,7 @@ import type { AST, Binding } from '#compiler'; -import type { Identifier, LabeledStatement, Node, Program } from 'estree'; +import type { AssignmentExpression, Identifier, LabeledStatement, Node, Program } from 'estree'; import type { Scope, ScopeRoot } from './scope.js'; +import type { StateCreationRuneName } from '../../utils.js'; export interface Js { ast: Program; diff --git a/packages/svelte/src/compiler/types/index.d.ts b/packages/svelte/src/compiler/types/index.d.ts index 616c346ad35a..20fe2c5eec51 100644 --- a/packages/svelte/src/compiler/types/index.d.ts +++ b/packages/svelte/src/compiler/types/index.d.ts @@ -2,6 +2,8 @@ import type { SourceMap } from 'magic-string'; import type { Binding } from '../phases/scope.js'; import type { AST, Namespace } from './template.js'; import type { ICompileDiagnostic } from '../utils/compile_diagnostic.js'; +import type { StateCreationRuneName } from '../../utils.js'; +import type { AssignmentExpression, PropertyDefinition } from 'estree'; /** The return value of `compile` from `svelte/compiler` */ export interface CompileResult { @@ -269,6 +271,13 @@ export interface ExpressionMetadata { has_call: boolean; } +export interface StateFields { + [name: string]: { + type: StateCreationRuneName; + node: PropertyDefinition | AssignmentExpression; + }; +} + export * from './template.js'; export { Binding, Scope } from '../phases/scope.js'; From 2ffb863b5d77c6b8a477427fc18469ed21644536 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 15 May 2025 17:59:17 -0400 Subject: [PATCH 28/64] on second thoughts --- packages/svelte/src/compiler/phases/2-analyze/types.d.ts | 6 +++--- .../src/compiler/phases/2-analyze/visitors/ClassBody.js | 4 ++-- packages/svelte/src/compiler/types/index.d.ts | 8 +++----- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/packages/svelte/src/compiler/phases/2-analyze/types.d.ts b/packages/svelte/src/compiler/phases/2-analyze/types.d.ts index 97b6029f7958..3bbab7d067d9 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/types.d.ts +++ b/packages/svelte/src/compiler/phases/2-analyze/types.d.ts @@ -1,6 +1,6 @@ import type { Scope } from '../scope.js'; import type { ComponentAnalysis, ReactiveStatement } from '../types.js'; -import type { AST, ExpressionMetadata, StateFields, ValidatedCompileOptions } from '#compiler'; +import type { AST, ExpressionMetadata, StateField, ValidatedCompileOptions } from '#compiler'; import type { ClassBody } from 'estree'; export interface AnalysisState { @@ -21,8 +21,8 @@ export interface AnalysisState { expression: ExpressionMetadata | null; /** Used to analyze class state. */ - classes: Map; - state_fields: StateFields | null; + classes: Map>; + state_fields: Record | null; function_depth: number; diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js index 6c5055bc17cb..6d6390b650f4 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js @@ -1,5 +1,5 @@ /** @import { AssignmentExpression, ClassBody, PropertyDefinition, Expression, PrivateIdentifier, MethodDefinition } from 'estree' */ -/** @import { StateFields } from '#compiler' */ +/** @import { StateField } from '#compiler' */ /** @import { Context } from '../types' */ import { get_rune } from '../../scope.js'; import * as e from '../../../errors.js'; @@ -15,7 +15,7 @@ export function ClassBody(node, context) { return; } - /** @type {StateFields} */ + /** @type {Record} */ const state_fields = {}; /** @type {string[]} */ diff --git a/packages/svelte/src/compiler/types/index.d.ts b/packages/svelte/src/compiler/types/index.d.ts index 20fe2c5eec51..8f96b3308f58 100644 --- a/packages/svelte/src/compiler/types/index.d.ts +++ b/packages/svelte/src/compiler/types/index.d.ts @@ -271,11 +271,9 @@ export interface ExpressionMetadata { has_call: boolean; } -export interface StateFields { - [name: string]: { - type: StateCreationRuneName; - node: PropertyDefinition | AssignmentExpression; - }; +export interface StateField { + type: StateCreationRuneName; + node: PropertyDefinition | AssignmentExpression; } export * from './template.js'; From b1e095af52b7dc3a0d38580858cd998ec2bd20f1 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 15 May 2025 18:01:21 -0400 Subject: [PATCH 29/64] reduce indirection --- .../phases/2-analyze/visitors/ClassBody.js | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js index 6d6390b650f4..829a9293b774 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js @@ -30,7 +30,11 @@ export function ClassBody(node, context) { * @param {Expression | null | undefined} value */ function handle(node, key, value) { - const name = get_name(key); + const name = + (key.type === 'Literal' && String(key.value)) || + (key.type === 'PrivateIdentifier' && '#' + key.name) || + (key.type === 'Identifier' && key.name); + if (!name) return; const rune = get_rune(value, context.state.scope); @@ -85,12 +89,3 @@ export function ClassBody(node, context) { state_fields }); } - -/** @param {Expression | PrivateIdentifier} node */ -function get_name(node) { - if (node.type === 'Literal') return String(node.value); - if (node.type === 'PrivateIdentifier') return '#' + node.name; - if (node.type === 'Identifier') return node.name; - - return null; -} From c407dc09de51174c1ac18ff839f91228f3938cc2 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 15 May 2025 18:14:27 -0400 Subject: [PATCH 30/64] make analysis available at transform time --- .../svelte/src/compiler/phases/2-analyze/index.js | 7 +++---- .../src/compiler/phases/2-analyze/types.d.ts | 1 - .../phases/2-analyze/visitors/ClassBody.js | 2 ++ packages/svelte/src/compiler/phases/types.d.ts | 14 ++++++++++++-- 4 files changed, 17 insertions(+), 7 deletions(-) diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index c29ddd9146a5..51180ca7cc6a 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -256,7 +256,8 @@ export function analyze_module(ast, options) { accessors: false, runes: true, immutable: true, - tracing: false + tracing: false, + classes: new Map() }; walk( @@ -265,7 +266,6 @@ export function analyze_module(ast, options) { scope, scopes, analysis: /** @type {ComponentAnalysis} */ (analysis), - classes: new Map(), state_fields: null, // TODO the following are not needed for modules, but we have to pass them in order to avoid type error, // and reducing the type would result in a lot of tedious type casts elsewhere - find a good solution one day @@ -430,6 +430,7 @@ export function analyze_component(root, source, options) { elements: [], runes, tracing: false, + classes: new Map(), immutable: runes || options.immutable, exports: [], uses_props: false, @@ -625,7 +626,6 @@ export function analyze_component(root, source, options) { has_props_rune: false, component_slots: new Set(), expression: null, - classes: new Map(), state_fields: null, function_depth: scope.function_depth, reactive_statement: null @@ -693,7 +693,6 @@ export function analyze_component(root, source, options) { reactive_statement: null, component_slots: new Set(), expression: null, - classes: new Map(), state_fields: null, function_depth: scope.function_depth }; diff --git a/packages/svelte/src/compiler/phases/2-analyze/types.d.ts b/packages/svelte/src/compiler/phases/2-analyze/types.d.ts index 3bbab7d067d9..328dc4deb168 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/types.d.ts +++ b/packages/svelte/src/compiler/phases/2-analyze/types.d.ts @@ -21,7 +21,6 @@ export interface AnalysisState { expression: ExpressionMetadata | null; /** Used to analyze class state. */ - classes: Map>; state_fields: Record | null; function_depth: number; diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js index 829a9293b774..46e1fc1a1771 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js @@ -18,6 +18,8 @@ export function ClassBody(node, context) { /** @type {Record} */ const state_fields = {}; + context.state.analysis.classes.set(node, state_fields); + /** @type {string[]} */ const seen = []; diff --git a/packages/svelte/src/compiler/phases/types.d.ts b/packages/svelte/src/compiler/phases/types.d.ts index 22e749c97b81..6e3aa9e02650 100644 --- a/packages/svelte/src/compiler/phases/types.d.ts +++ b/packages/svelte/src/compiler/phases/types.d.ts @@ -1,7 +1,15 @@ -import type { AST, Binding } from '#compiler'; -import type { AssignmentExpression, Identifier, LabeledStatement, Node, Program } from 'estree'; +import type { AST, Binding, StateField } from '#compiler'; +import type { + AssignmentExpression, + ClassBody, + Identifier, + LabeledStatement, + Node, + Program +} from 'estree'; import type { Scope, ScopeRoot } from './scope.js'; import type { StateCreationRuneName } from '../../utils.js'; +import type { AnalysisState } from './2-analyze/types.js'; export interface Js { ast: Program; @@ -30,6 +38,8 @@ export interface Analysis { immutable: boolean; tracing: boolean; + classes: Map>; + // TODO figure out if we can move this to ComponentAnalysis accessors: boolean; } From 2efb766f2360d095ceda3c09b527dd51f077f398 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 16 May 2025 16:25:16 -0400 Subject: [PATCH 31/64] WIP --- .../phases/2-analyze/visitors/ClassBody.js | 5 +- .../3-transform/client/transform-client.js | 8 +- .../phases/3-transform/client/types.d.ts | 9 +- .../client/visitors/AssignmentExpression.js | 66 ++++-- .../client/visitors/CallExpression.js | 23 +- .../3-transform/client/visitors/ClassBody.js | 211 +++++++++++++++++- .../client/visitors/MemberExpression.js | 4 +- .../client/visitors/UpdateExpression.js | 2 +- .../3-transform/server/transform-server.js | 4 +- .../phases/3-transform/server/types.d.ts | 4 +- .../server/visitors/AssignmentExpression.js | 31 ++- .../server/visitors/CallExpression.js | 9 + .../3-transform/server/visitors/ClassBody.js | 120 +++++++++- .../server/visitors/MemberExpression.js | 5 +- .../compiler/phases/3-transform/types.d.ts | 7 - packages/svelte/src/compiler/phases/nodes.js | 12 + packages/svelte/src/compiler/types/index.d.ts | 3 +- 17 files changed, 458 insertions(+), 65 deletions(-) diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js index 46e1fc1a1771..1fe22128f4d5 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js @@ -1,4 +1,4 @@ -/** @import { AssignmentExpression, ClassBody, PropertyDefinition, Expression, PrivateIdentifier, MethodDefinition } from 'estree' */ +/** @import { AssignmentExpression, CallExpression, ClassBody, PropertyDefinition, Expression, PrivateIdentifier, MethodDefinition } from 'estree' */ /** @import { StateField } from '#compiler' */ /** @import { Context } from '../types' */ import { get_rune } from '../../scope.js'; @@ -48,7 +48,8 @@ export function ClassBody(node, context) { state_fields[name] = { node, - type: rune + type: rune, + value: /** @type {CallExpression} */ (value) }; } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js index 740bcae479c0..9a2f4dd34c70 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js @@ -163,7 +163,8 @@ export function client_component(analysis, options) { }, events: new Set(), preserve_whitespace: options.preserveWhitespace, - class_transformer: null, + public_state: new Map(), + private_state: new Map(), transform: {}, in_constructor: false, instance_level_snippets: [], @@ -670,9 +671,10 @@ export function client_module(analysis, options) { options, scope: analysis.module.scope, scopes: analysis.module.scopes, + public_state: new Map(), + private_state: new Map(), transform: {}, - in_constructor: false, - class_transformer: null + in_constructor: false }; const module = /** @type {ESTree.Program} */ ( diff --git a/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts b/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts index c2292292da2e..040105342564 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts +++ b/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts @@ -3,6 +3,7 @@ import type { Statement, LabeledStatement, Identifier, + PrivateIdentifier, Expression, AssignmentExpression, UpdateExpression, @@ -12,10 +13,10 @@ import type { AST, Namespace, ValidatedCompileOptions } from '#compiler'; import type { TransformState } from '../types.js'; import type { ComponentAnalysis } from '../../types.js'; import type { SourceLocation } from '#shared'; -import type { ClassTransformer } from '../shared/types.js'; export interface ClientTransformState extends TransformState { - readonly class_transformer: ClassTransformer | null; + readonly private_state: Map; + readonly public_state: Map; /** * `true` if the current lexical scope belongs to a class constructor. this allows @@ -93,6 +94,10 @@ export interface ComponentClientTransformState extends ClientTransformState { readonly module_level_snippets: VariableDeclaration[]; } +export interface StateField { + type: '$state' | '$state.raw' | '$derived' | '$derived.by'; +} + export type Context = import('zimmerframe').Context; export type Visitors = import('zimmerframe').Visitors; diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js index d11938af9778..d23ecb26dbb8 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js @@ -1,4 +1,4 @@ -/** @import { AssignmentExpression, AssignmentOperator, Expression, Identifier, Pattern, MemberExpression, ThisExpression, PrivateIdentifier, CallExpression } from 'estree' */ +/** @import { AssignmentExpression, AssignmentOperator, Expression, Identifier, Pattern } from 'estree' */ /** @import { AST } from '#compiler' */ /** @import { Context } from '../types.js' */ import * as b from '#compiler/builders'; @@ -11,17 +11,14 @@ import { dev, locate_node } from '../../../../state.js'; import { should_proxy } from '../utils.js'; import { visit_assignment_expression } from '../../shared/assignments.js'; import { validate_mutation } from './shared/utils.js'; +import { get_rune } from '../../../scope.js'; +import { get_name } from '../../../nodes.js'; /** * @param {AssignmentExpression} node * @param {Context} context */ export function AssignmentExpression(node, context) { - const stripped_node = context.state.class_transformer?.generate_assignment(node, context); - if (stripped_node) { - return stripped_node; - } - const expression = /** @type {Expression} */ ( visit_assignment_expression(node, context, build_assignment) ?? context.next() ); @@ -55,25 +52,50 @@ const callees = { * @returns {Expression | null} */ function build_assignment(operator, left, right, context) { - // Handle class private/public state assignment cases - if ( - context.state.analysis.runes && - left.type === 'MemberExpression' && - left.property.type === 'PrivateIdentifier' - ) { - const private_state = context.state.class_transformer?.get_field(left.property.name, true); + if (context.state.analysis.runes && left.type === 'MemberExpression') { + // special case — state declaration in class constructor + const ancestor = context.path.at(-4); + + if (ancestor?.type === 'MethodDefinition' && ancestor.kind === 'constructor') { + const rune = get_rune(right, context.state.scope); + + if (rune) { + const name = get_name(left.property); + + const child_state = { + ...context.state, + in_constructor: rune !== '$derived' && rune !== '$derived.by' + }; - if (private_state !== undefined) { - let value = /** @type {Expression} */ ( - context.visit(build_assignment_value(operator, left, right)) - ); + const l = b.member( + b.this, + left.property.type === 'PrivateIdentifier' + ? left.property + : context.state.backing_fields[name] + ); - const needs_proxy = - private_state?.kind === '$state' && - is_non_coercive_operator(operator) && - should_proxy(value, context.state.scope); + const r = /** @type {Expression} */ (context.visit(right, child_state)); - return b.call('$.set', left, value, needs_proxy && b.true); + return b.assignment(operator, l, r); + } + } + + // special case — assignment to private state field + if (left.property.type === 'PrivateIdentifier') { + const private_state = context.state.private_state.get(left.property.name); + + if (private_state !== undefined) { + let value = /** @type {Expression} */ ( + context.visit(build_assignment_value(operator, left, right)) + ); + + const needs_proxy = + private_state.type === '$state' && + is_non_coercive_operator(operator) && + should_proxy(value, context.state.scope); + + return b.call('$.set', left, value, needs_proxy && b.true); + } } } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/CallExpression.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/CallExpression.js index b110f8eae82c..dd336e397de6 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/CallExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/CallExpression.js @@ -10,13 +10,34 @@ import { transform_inspect_rune } from '../../utils.js'; * @param {Context} context */ export function CallExpression(node, context) { - switch (get_rune(node, context.state.scope)) { + const rune = get_rune(node, context.state.scope); + + switch (rune) { case '$host': return b.id('$$props.$$host'); case '$effect.tracking': return b.call('$.effect_tracking'); + case '$state': + case '$state.raw': { + let should_proxy = rune === '$state' && true; // TODO + + return b.call( + '$.state', + node.arguments[0] && /** @type {Expression} */ (context.visit(node.arguments[0])), + should_proxy && b.true + ); + } + + case '$derived': + case '$derived.by': { + let fn = /** @type {Expression} */ (context.visit(node.arguments[0])); + if (rune === '$derived') fn = b.thunk(fn); + + return b.call('$.derived', fn); + } + case '$state.snapshot': return b.call( '$.snapshot', diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js index 75f2b35cca62..5501e5f72d5e 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js @@ -1,19 +1,218 @@ -/** @import { ClassBody } from 'estree' */ -/** @import { Context } from '../types' */ -import { create_client_class_transformer } from './shared/client-class-transformer.js'; +/** @import { CallExpression, ClassBody, Expression, Identifier, Literal, MethodDefinition, PrivateIdentifier, PropertyDefinition, StaticBlock } from 'estree' */ +/** @import { Context, StateField } from '../types' */ +import * as b from '#compiler/builders'; +import { get_name } from '../../../nodes.js'; +import { regex_invalid_identifier_chars } from '../../../patterns.js'; +import { get_rune } from '../../../scope.js'; +import { should_proxy } from '../utils.js'; /** * @param {ClassBody} node * @param {Context} context */ export function ClassBody(node, context) { - if (!context.state.analysis.runes) { + const state_fields = context.state.analysis.classes.get(node); + + if (!state_fields) { + // in legacy mode, do nothing context.next(); return; } - const class_transformer = create_client_class_transformer(node.body); - const body = class_transformer.generate_body(context); + /** @type {string[]} */ + const private_ids = []; + + for (const prop of node.body) { + if ( + (prop.type === 'MethodDefinition' || prop.type === 'PropertyDefinition') && + prop.key.type === 'PrivateIdentifier' + ) { + private_ids.push(prop.key.name); + } + } + + const private_state = new Map(); + + /** + * each `foo = $state()` needs a backing `#foo` field + * @type {Record} + */ + const backing_fields = {}; + + for (const name in state_fields) { + if (name[0] === '#') { + private_state.set(name.slice(1), state_fields[name]); + continue; + } + + let deconflicted = name.replace(regex_invalid_identifier_chars, '_'); + while (private_ids.includes(deconflicted)) { + deconflicted = '_' + deconflicted; + } + + private_ids.push(deconflicted); + backing_fields[name] = b.private_id(deconflicted); + } + + /** @type {Array} */ + const body = []; + + const child_state = { ...context.state, state_fields, backing_fields, private_state }; // TODO populate private_state + + for (const name in state_fields) { + if (name[0] === '#') { + continue; + } + + const field = state_fields[name]; + + // insert backing fields for stuff declared in the constructor + if (field.node.type === 'AssignmentExpression') { + const backing = backing_fields[name]; + const member = b.member(b.this, backing); + + const should_proxy = field.type === '$state' && true; // TODO + + const key = b.key(name); + + body.push( + b.prop_def(backing, null), + + b.method('get', key, [], [b.return(b.call('$.get', member))]), + + b.method( + 'set', + key, + [b.id('value')], + [b.stmt(b.call('$.set', member, b.id('value'), should_proxy && b.true))] + ) + ); + } + } + + // Replace parts of the class body + for (const definition of node.body) { + if (definition.type === 'MethodDefinition' || definition.type === 'StaticBlock') { + body.push( + /** @type {MethodDefinition | StaticBlock} */ (context.visit(definition, child_state)) + ); + continue; + } + + const name = get_name(definition.key); + if (name === null || !Object.hasOwn(state_fields, name)) { + body.push(/** @type {PropertyDefinition} */ (context.visit(definition, child_state))); + continue; + } + + const field = state_fields[name]; + + if (name[0] === '#') { + body.push(/** @type {PropertyDefinition} */ (context.visit(definition, child_state))); + } else { + const backing = backing_fields[name]; + const member = b.member(b.this, backing); + + const should_proxy = field.type === '$state' && true; // TODO + + body.push( + b.prop_def( + backing, + /** @type {CallExpression} */ ( + context.visit(definition.value ?? field.value, child_state) + ) + ), + + b.method('get', definition.key, [], [b.return(b.call('$.get', member))]), + + b.method( + 'set', + definition.key, + [b.id('value')], + [b.stmt(b.call('$.set', member, b.id('value'), should_proxy && b.true))] + ) + ); + } + + // if (definition.type === 'PropertyDefinition') { + // const original_name = get_name(definition.key); + // if (original_name === null) continue; + + // const name = definition_names[original_name]; + + // const is_private = definition.key.type === 'PrivateIdentifier'; + // const field = (is_private ? private_state : public_state).get(name); + + // if (definition.value?.type === 'CallExpression' && field !== undefined) { + // let value = null; + + // if (definition.value.arguments.length > 0) { + // const init = /** @type {Expression} **/ ( + // context.visit(definition.value.arguments[0], child_state) + // ); + + // value = + // field.kind === 'state' + // ? b.call( + // '$.state', + // should_proxy(init, context.state.scope) ? b.call('$.proxy', init) : init + // ) + // : field.kind === 'raw_state' + // ? b.call('$.state', init) + // : field.kind === 'derived_by' + // ? b.call('$.derived', init) + // : b.call('$.derived', b.thunk(init)); + // } else { + // // if no arguments, we know it's state as `$derived()` is a compile error + // value = b.call('$.state'); + // } + + // if (is_private) { + // body.push(b.prop_def(field.id, value)); + // } else { + // // #foo; + // const member = b.member(b.this, field.id); + // body.push(b.prop_def(field.id, value)); + + // // get foo() { return this.#foo; } + // body.push(b.method('get', definition.key, [], [b.return(b.call('$.get', member))])); + + // // set foo(value) { this.#foo = value; } + // const val = b.id('value'); + + // body.push( + // b.method( + // 'set', + // definition.key, + // [val], + // [b.stmt(b.call('$.set', member, val, field.kind === 'state' && b.true))] + // ) + // ); + // } + // continue; + // } + // } + + // body.push(/** @type {MethodDefinition} **/ (context.visit(definition, child_state))); + } return { ...node, body }; } + +/** + * @param {string} name + * @param {Map} public_state + */ +function get_deconflicted_name(name, public_state) { + name = name.replace(regex_invalid_identifier_chars, '_'); + + // the above could generate conflicts because it has to generate a valid identifier + // so stuff like `0` and `1` or `state%` and `state^` will result in the same string + // so we have to de-conflict. We can only check `public_state` because private state + // can't have literal keys + while (name && public_state.has(name)) { + name = '_' + name; + } + + return name; +} diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/MemberExpression.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/MemberExpression.js index 02052942f565..bc9d26367002 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/MemberExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/MemberExpression.js @@ -9,10 +9,10 @@ import * as b from '#compiler/builders'; export function MemberExpression(node, context) { // rewrite `this.#foo` as `this.#foo.v` inside a constructor if (node.property.type === 'PrivateIdentifier') { - const field = context.state.class_transformer?.get_field(node.property.name, true); + const field = context.state.private_state.get(node.property.name); if (field) { return context.state.in_constructor && - (field.kind === '$state.raw' || field.kind === '$state') + (field.type === '$state.raw' || field.type === '$state') ? b.member(node, 'v') : b.call('$.get', node); } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/UpdateExpression.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/UpdateExpression.js index ca81fa14931f..96be119b840a 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/UpdateExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/UpdateExpression.js @@ -15,7 +15,7 @@ export function UpdateExpression(node, context) { argument.type === 'MemberExpression' && argument.object.type === 'ThisExpression' && argument.property.type === 'PrivateIdentifier' && - context.state.class_transformer?.get_field(argument.property.name, true) + context.state.private_state.has(argument.property.name) ) { let fn = '$.update'; if (node.prefix) fn += '_pre'; diff --git a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js index 661b73d65956..e7896991d98f 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js @@ -99,7 +99,7 @@ export function server_component(analysis, options) { template: /** @type {any} */ (null), namespace: options.namespace, preserve_whitespace: options.preserveWhitespace, - class_transformer: null, + private_derived: new Map(), skip_hydration_boundaries: false }; @@ -395,7 +395,7 @@ export function server_module(analysis, options) { // to be present for `javascript_visitors_legacy` and so is included in module // transform state as well as component transform state legacy_reactive_statements: new Map(), - class_transformer: null + private_derived: new Map() }; const module = /** @type {Program} */ ( diff --git a/packages/svelte/src/compiler/phases/3-transform/server/types.d.ts b/packages/svelte/src/compiler/phases/3-transform/server/types.d.ts index 24b35d37612b..971271642cfc 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/types.d.ts +++ b/packages/svelte/src/compiler/phases/3-transform/server/types.d.ts @@ -2,12 +2,12 @@ import type { Expression, Statement, ModuleDeclaration, LabeledStatement } from import type { AST, Namespace, ValidatedCompileOptions } from '#compiler'; import type { TransformState } from '../types.js'; import type { ComponentAnalysis } from '../../types.js'; -import type { ClassTransformer } from '../shared/types.js'; +import type { StateField } from '../client/types.js'; export interface ServerTransformState extends TransformState { /** The $: calls, which will be ordered in the end */ readonly legacy_reactive_statements: Map; - readonly class_transformer: ClassTransformer | null; + readonly private_derived: Map; } export interface ComponentServerTransformState extends ServerTransformState { diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/AssignmentExpression.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/AssignmentExpression.js index 939dd45ca28c..c0dd77c64eaa 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/AssignmentExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/AssignmentExpression.js @@ -3,6 +3,8 @@ /** @import { Context, ServerTransformState } from '../types.js' */ import * as b from '#compiler/builders'; import { build_assignment_value } from '../../../../utils/ast.js'; +import { get_name } from '../../../nodes.js'; +import { get_rune } from '../../../scope.js'; import { visit_assignment_expression } from '../../shared/assignments.js'; /** @@ -10,11 +12,6 @@ import { visit_assignment_expression } from '../../shared/assignments.js'; * @param {Context} context */ export function AssignmentExpression(node, context) { - const stripped_node = context.state.class_transformer?.generate_assignment(node, context); - if (stripped_node) { - return stripped_node; - } - return visit_assignment_expression(node, context, build_assignment) ?? context.next(); } @@ -27,6 +24,30 @@ export function AssignmentExpression(node, context) { * @returns {Expression | null} */ function build_assignment(operator, left, right, context) { + if (context.state.analysis.runes && left.type === 'MemberExpression') { + // special case — state declaration in class constructor + const ancestor = context.path.at(-4); + + if (ancestor?.type === 'MethodDefinition' && ancestor.kind === 'constructor') { + const rune = get_rune(right, context.state.scope); + + if (rune) { + const name = get_name(left.property); + + const l = b.member( + b.this, + left.property.type === 'PrivateIdentifier' || rune === '$state' || rune === '$state.raw' + ? left.property + : context.state.backing_fields[name] + ); + + const r = /** @type {Expression} */ (context.visit(right)); + + return b.assignment(operator, l, r); + } + } + } + let object = left; while (object.type === 'MemberExpression') { diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/CallExpression.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/CallExpression.js index 5bcbdee9fbfe..e36dc820b3ef 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/CallExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/CallExpression.js @@ -25,6 +25,15 @@ export function CallExpression(node, context) { return b.arrow([], b.block([])); } + if (rune === '$state' || rune === '$state.raw') { + return node.arguments[0] ? context.visit(node.arguments[0]) : b.void0; + } + + if (rune === '$derived' || rune === '$derived.by') { + const fn = /** @type {Expression} */ (context.visit(node.arguments[0])); + return b.call('$.once', rune === '$derived' ? b.thunk(fn) : fn); + } + if (rune === '$state.snapshot') { return b.call( '$.snapshot', diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/ClassBody.js index 3bab89cf9bee..a59dea5cc113 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/ClassBody.js @@ -1,19 +1,129 @@ -/** @import { ClassBody } from 'estree' */ +/** @import { CallExpression, ClassBody, Expression, MethodDefinition, PrivateIdentifier, PropertyDefinition, StaticBlock } from 'estree' */ /** @import { Context } from '../types.js' */ -import { create_server_class_transformer } from './shared/server-class-transformer.js'; +/** @import { StateField } from '../../client/types.js' */ +import { dev } from '../../../../state.js'; +import * as b from '#compiler/builders'; +import { get_rune } from '../../../scope.js'; +import { regex_invalid_identifier_chars } from '../../../patterns.js'; +import { get_name } from '../../../nodes.js'; /** * @param {ClassBody} node * @param {Context} context */ export function ClassBody(node, context) { - if (!context.state.analysis.runes) { + const state_fields = context.state.analysis.classes.get(node); + + if (!state_fields) { + // in legacy mode, do nothing context.next(); return; } - const class_transformer = create_server_class_transformer(node.body); - const body = class_transformer.generate_body(context); + /** @type {string[]} */ + const private_ids = []; + + for (const prop of node.body) { + if ( + (prop.type === 'MethodDefinition' || prop.type === 'PropertyDefinition') && + prop.key.type === 'PrivateIdentifier' + ) { + private_ids.push(prop.key.name); + } + } + + const private_state = new Map(); + + /** + * each `foo = $state()` needs a backing `#foo` field + * @type {Record} + */ + const backing_fields = {}; + + for (const name in state_fields) { + if (name[0] === '#') { + private_state.set(name.slice(1), state_fields[name]); + continue; + } + + let deconflicted = name.replace(regex_invalid_identifier_chars, '_'); + while (private_ids.includes(deconflicted)) { + deconflicted = '_' + deconflicted; + } + + private_ids.push(deconflicted); + backing_fields[name] = b.private_id(deconflicted); + } + + /** @type {Array} */ + const body = []; + + const child_state = { ...context.state, state_fields, backing_fields, private_state }; // TODO populate private_state + + for (const name in state_fields) { + if (name[0] === '#') { + continue; + } + + const field = state_fields[name]; + + // insert backing fields for stuff declared in the constructor + if ( + field.node.type === 'AssignmentExpression' && + (field.type === '$derived' || field.type === '$derived.by') + ) { + const backing = backing_fields[name]; + const member = b.member(b.this, backing); + + const should_proxy = field.type === '$state' && true; // TODO + + const key = b.key(name); + + body.push( + b.prop_def(backing, null), + + b.method('get', key, [], [b.return(b.call(member))]) + ); + } + } + + // Replace parts of the class body + for (const definition of node.body) { + if (definition.type === 'MethodDefinition' || definition.type === 'StaticBlock') { + body.push( + /** @type {MethodDefinition | StaticBlock} */ (context.visit(definition, child_state)) + ); + continue; + } + + const name = get_name(definition.key); + if (name === null || !Object.hasOwn(state_fields, name)) { + body.push(/** @type {PropertyDefinition} */ (context.visit(definition, child_state))); + continue; + } + + const field = state_fields[name]; + + if (name[0] === '#' || field.type === '$state' || field.type === '$state.raw') { + body.push(/** @type {PropertyDefinition} */ (context.visit(definition, child_state))); + } else { + const backing = backing_fields[name]; + const member = b.member(b.this, backing); + + const should_proxy = field.type === '$state' && true; // TODO + + body.push( + b.prop_def( + backing, + /** @type {CallExpression} */ ( + context.visit(definition.value ?? field.value, child_state) + ) + ), + + b.method('get', definition.key, [], [b.return(b.call(member))]) + ); + } + } return { ...node, body }; } diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/MemberExpression.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/MemberExpression.js index 2a90307293fb..73631395e66e 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/MemberExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/MemberExpression.js @@ -12,10 +12,7 @@ export function MemberExpression(node, context) { node.object.type === 'ThisExpression' && node.property.type === 'PrivateIdentifier' ) { - const field = context.state.class_transformer?.get_field(node.property.name, true, [ - '$derived', - '$derived.by' - ]); + const field = context.state.private_derived.get(node.property.name); if (field) { return b.call(node); diff --git a/packages/svelte/src/compiler/phases/3-transform/types.d.ts b/packages/svelte/src/compiler/phases/3-transform/types.d.ts index c610110cef9d..5d860207dde6 100644 --- a/packages/svelte/src/compiler/phases/3-transform/types.d.ts +++ b/packages/svelte/src/compiler/phases/3-transform/types.d.ts @@ -1,8 +1,6 @@ import type { Scope } from '../scope.js'; import type { AST, ValidatedModuleCompileOptions } from '#compiler'; import type { Analysis } from '../types.js'; -import type { StateCreationRuneName } from '../../../utils.js'; -import type { PrivateIdentifier } from 'estree'; export interface TransformState { readonly analysis: Analysis; @@ -10,8 +8,3 @@ export interface TransformState { readonly scope: Scope; readonly scopes: Map; } - -export interface StateField { - kind: StateCreationRuneName; - id: PrivateIdentifier; -} diff --git a/packages/svelte/src/compiler/phases/nodes.js b/packages/svelte/src/compiler/phases/nodes.js index 29fb8c5c83c2..2043747ed07e 100644 --- a/packages/svelte/src/compiler/phases/nodes.js +++ b/packages/svelte/src/compiler/phases/nodes.js @@ -1,3 +1,4 @@ +/** @import { Expression, PrivateIdentifier } from 'estree' */ /** @import { AST, ExpressionMetadata } from '#compiler' */ /** @@ -65,3 +66,14 @@ export function create_expression_metadata() { has_call: false }; } + +/** + * @param {Expression | PrivateIdentifier} node + */ +export function get_name(node) { + if (node.type === 'Literal') return String(node.value); + if (node.type === 'PrivateIdentifier') return '#' + node.name; + if (node.type === 'Identifier') return node.name; + + return null; +} diff --git a/packages/svelte/src/compiler/types/index.d.ts b/packages/svelte/src/compiler/types/index.d.ts index 8f96b3308f58..f91bceac8093 100644 --- a/packages/svelte/src/compiler/types/index.d.ts +++ b/packages/svelte/src/compiler/types/index.d.ts @@ -3,7 +3,7 @@ import type { Binding } from '../phases/scope.js'; import type { AST, Namespace } from './template.js'; import type { ICompileDiagnostic } from '../utils/compile_diagnostic.js'; import type { StateCreationRuneName } from '../../utils.js'; -import type { AssignmentExpression, PropertyDefinition } from 'estree'; +import type { AssignmentExpression, CallExpression, PropertyDefinition } from 'estree'; /** The return value of `compile` from `svelte/compiler` */ export interface CompileResult { @@ -274,6 +274,7 @@ export interface ExpressionMetadata { export interface StateField { type: StateCreationRuneName; node: PropertyDefinition | AssignmentExpression; + value: CallExpression; } export * from './template.js'; From 0e9ca232867800f9389651c776fd81e06af5ce60 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 16 May 2025 16:33:02 -0400 Subject: [PATCH 32/64] WIP --- .../3-transform/server/visitors/AssignmentExpression.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/AssignmentExpression.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/AssignmentExpression.js index c0dd77c64eaa..defcf1367277 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/AssignmentExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/AssignmentExpression.js @@ -33,13 +33,12 @@ function build_assignment(operator, left, right, context) { if (rune) { const name = get_name(left.property); - - const l = b.member( - b.this, + const key = left.property.type === 'PrivateIdentifier' || rune === '$state' || rune === '$state.raw' ? left.property - : context.state.backing_fields[name] - ); + : context.state.backing_fields[name]; + + const l = b.member(b.this, key, key.type === 'Literal'); const r = /** @type {Expression} */ (context.visit(right)); From b7bbae8e7ee89648f17e185b408bdf6b81b66490 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 16 May 2025 16:39:37 -0400 Subject: [PATCH 33/64] WIP --- .../compiler/phases/3-transform/client/visitors/ClassBody.js | 4 ++++ .../compiler/phases/3-transform/server/visitors/ClassBody.js | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js index 5501e5f72d5e..69bc151cee90 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js @@ -110,6 +110,10 @@ export function ClassBody(node, context) { if (name[0] === '#') { body.push(/** @type {PropertyDefinition} */ (context.visit(definition, child_state))); } else { + if (field.node.type === 'AssignmentExpression') { + continue; + } + const backing = backing_fields[name]; const member = b.member(b.this, backing); diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/ClassBody.js index a59dea5cc113..ed32cfa954a9 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/ClassBody.js @@ -107,6 +107,10 @@ export function ClassBody(node, context) { if (name[0] === '#' || field.type === '$state' || field.type === '$state.raw') { body.push(/** @type {PropertyDefinition} */ (context.visit(definition, child_state))); } else { + if (field.node.type === 'AssignmentExpression') { + continue; + } + const backing = backing_fields[name]; const member = b.member(b.this, backing); From 737bfb5740b3dacf92a51bfb25d3bb97f9587cd2 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 16 May 2025 16:43:37 -0400 Subject: [PATCH 34/64] fix --- .../3-transform/client/visitors/CallExpression.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/CallExpression.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/CallExpression.js index dd336e397de6..22b715868fc3 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/CallExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/CallExpression.js @@ -23,11 +23,13 @@ export function CallExpression(node, context) { case '$state.raw': { let should_proxy = rune === '$state' && true; // TODO - return b.call( - '$.state', - node.arguments[0] && /** @type {Expression} */ (context.visit(node.arguments[0])), - should_proxy && b.true - ); + let value = node.arguments[0] && /** @type {Expression} */ (context.visit(node.arguments[0])); + + if (value && should_proxy) { + value = b.call('$.proxy', value); + } + + return b.call('$.state', value); } case '$derived': From c536ec6f2be5df8321f2f93e862d5589a5e3c6a4 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 16 May 2025 16:47:08 -0400 Subject: [PATCH 35/64] remove unused stuff --- .../shared/client-class-transformer.js | 95 ----- .../shared/server-class-transformer.js | 103 ----- .../3-transform/shared/class_transformer.js | 392 ------------------ .../phases/3-transform/shared/types.d.ts | 82 ---- 4 files changed, 672 deletions(-) delete mode 100644 packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/client-class-transformer.js delete mode 100644 packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/server-class-transformer.js delete mode 100644 packages/svelte/src/compiler/phases/3-transform/shared/class_transformer.js delete mode 100644 packages/svelte/src/compiler/phases/3-transform/shared/types.d.ts diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/client-class-transformer.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/client-class-transformer.js deleted file mode 100644 index 767a99ea6952..000000000000 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/client-class-transformer.js +++ /dev/null @@ -1,95 +0,0 @@ -/** @import { Context } from '../../types.js' */ -/** @import { MethodDefinition, PropertyDefinition, Expression, StaticBlock, SpreadElement } from 'estree' */ -/** @import { StateCreationRuneName } from '../../../../../../utils.js' */ -/** @import { AssignmentBuilder, ClassTransformer, StateFieldBuilder } from '../../../shared/types.js' */ -import * as b from '#compiler/builders'; -import { create_class_transformer } from '../../../shared/class_transformer.js'; -import { should_proxy } from '../../utils.js'; - -/** - * @param {Array} body - * @returns {ClassTransformer} - */ -export function create_client_class_transformer(body) { - /** @type {StateFieldBuilder} */ - function build_state_field({ is_private, field, node, context }) { - let original_id = node.type === 'AssignmentExpression' ? node.left.property : node.key; - let value; - if (node.type === 'AssignmentExpression') { - // if there's no call expression, this is state that's created in the constructor. - // it's guaranteed to be the very first assignment to this field, so we initialize - // the field but don't assign to it. - value = null; - } else if (node.value.arguments.length > 0) { - value = build_init_value(field.kind, node.value.arguments[0], context); - } else { - // if no arguments, we know it's state as `$derived()` is a compile error - value = b.call('$.state'); - } - - if (is_private) { - return [b.prop_def(field.id, value)]; - } - - const member = b.member(b.this, field.id); - const val = b.id('value'); - - return [ - // #foo; - b.prop_def(field.id, value), - // get foo() { return this.#foo; } - b.method('get', original_id, [], [b.return(b.call('$.get', member))]), - // set foo(value) { this.#foo = value; } - b.method( - 'set', - original_id, - [val], - [b.stmt(b.call('$.set', member, val, field.kind === '$state' && b.true))] - ) - ]; - } - - /** @type {AssignmentBuilder} */ - function build_assignment({ field, node, context }) { - return { - ...node, - left: { - ...node.left, - // ...swap out the assignment to go directly against the private field - property: field.id, - // this could be a transformation from `this.[1]` to `this.#_` (the private field we generated) - // -- private fields are never computed - computed: false - }, - // ...and swap out the assignment's value for the state field init - right: build_init_value(field.kind, node.right.arguments[0], context) - }; - } - - return create_class_transformer(body, build_state_field, build_assignment); -} - -/** - * @param {StateCreationRuneName} kind - * @param {Expression | SpreadElement} arg - * @param {Context} context - */ -function build_init_value(kind, arg, context) { - const init = arg - ? /** @type {Expression} **/ (context.visit(arg, { ...context.state, in_constructor: false })) - : b.void0; - - switch (kind) { - case '$state': - return b.call( - '$.state', - should_proxy(init, context.state.scope) ? b.call('$.proxy', init) : init - ); - case '$state.raw': - return b.call('$.state', init); - case '$derived': - return b.call('$.derived', b.thunk(init)); - case '$derived.by': - return b.call('$.derived', init); - } -} diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/server-class-transformer.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/server-class-transformer.js deleted file mode 100644 index 7a52f1473106..000000000000 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/server-class-transformer.js +++ /dev/null @@ -1,103 +0,0 @@ -/** @import { Expression, MethodDefinition, StaticBlock, PropertyDefinition, SpreadElement } from 'estree' */ -/** @import { Context } from '../../types.js' */ -/** @import { AssignmentBuilder, StateFieldBuilder } from '../../../shared/types.js' */ -/** @import { ClassTransformer } from '../../../shared/types.js' */ -/** @import { StateCreationRuneName } from '../../../../../../utils.js' */ - -import * as b from '#compiler/builders'; -import { dev } from '../../../../../state.js'; -import { create_class_transformer } from '../../../shared/class_transformer.js'; - -/** - * @param {Array} body - * @returns {ClassTransformer} - */ -export function create_server_class_transformer(body) { - /** @type {StateFieldBuilder} */ - function build_state_field({ is_private, field, node, context }) { - let original_id = node.type === 'AssignmentExpression' ? node.left.property : node.key; - let value; - if (node.type === 'AssignmentExpression') { - // This means it's a state assignment in the constructor (this.foo = $state('bar')) - // which means the state field needs to have no default value so that the initial - // value can be assigned in the constructor. - value = null; - } else if (field.kind !== '$derived' && field.kind !== '$derived.by') { - return [/** @type {PropertyDefinition} */ (context.visit(node))]; - } else { - const init = /** @type {Expression} **/ (context.visit(node.value.arguments[0])); - value = - field.kind === '$derived.by' ? b.call('$.once', init) : b.call('$.once', b.thunk(init)); - } - - if (is_private) { - return [b.prop_def(field.id, value)]; - } - // #foo; - const member = b.member(b.this, field.id); - - /** @type {Array} */ - const defs = [ - // #foo; - b.prop_def(field.id, value) - ]; - - // get foo() { return this.#foo; } - if (field.kind === '$state' || field.kind === '$state.raw') { - defs.push(b.method('get', original_id, [], [b.return(member)])); - } else { - defs.push(b.method('get', original_id, [], [b.return(b.call(member))])); - } - - // TODO make this work on server - if (dev) { - defs.push( - b.method( - 'set', - original_id, - [b.id('_')], - [b.throw_error(`Cannot update a derived property ('${name}')`)] - ) - ); - } - - return defs; - } - - /** @type {AssignmentBuilder} */ - function build_assignment({ field, node, context }) { - return { - ...node, - left: { - ...node.left, - // ...swap out the assignment to go directly against the private field - property: field.id, - computed: false - }, - // ...and swap out the assignment's value for the state field init - right: build_init_value(field.kind, node.right.arguments[0], context) - }; - } - - return create_class_transformer(body, build_state_field, build_assignment); -} - -/** - * - * @param {StateCreationRuneName} kind - * @param {Expression | SpreadElement} arg - * @param {Context} context - */ -function build_init_value(kind, arg, context) { - const init = arg ? /** @type {Expression} **/ (context.visit(arg)) : b.void0; - - switch (kind) { - case '$state': - case '$state.raw': - return init; - case '$derived': - return b.call('$.once', b.thunk(init)); - case '$derived.by': - return b.call('$.once', init); - } -} diff --git a/packages/svelte/src/compiler/phases/3-transform/shared/class_transformer.js b/packages/svelte/src/compiler/phases/3-transform/shared/class_transformer.js deleted file mode 100644 index 469a178cc295..000000000000 --- a/packages/svelte/src/compiler/phases/3-transform/shared/class_transformer.js +++ /dev/null @@ -1,392 +0,0 @@ -/** @import { AssignmentExpression, Identifier, Literal, MethodDefinition, PrivateIdentifier, PropertyDefinition, StaticBlock } from 'estree' */ -/** @import { StateField } from '../types.js' */ -/** @import { Context as ClientContext } from '../client/types.js' */ -/** @import { Context as ServerContext } from '../server/types.js' */ -/** @import { StateCreationRuneName } from '../../../../utils.js' */ -/** @import { AssignmentBuilder, ClassTransformer, StateFieldBuilder, StatefulAssignment, StatefulPropertyDefinition } from './types.js' */ -/** @import { Scope } from '../../scope.js' */ -import * as b from '#compiler/builders'; -import { once } from '../../../../internal/server/index.js'; -import { is_state_creation_rune, STATE_CREATION_RUNES } from '../../../../utils.js'; -import { regex_invalid_identifier_chars } from '../../patterns.js'; -import { get_rune } from '../../scope.js'; - -/** - * @template {ClientContext | ServerContext} TContext - * @param {Array} body - * @param {StateFieldBuilder} build_state_field - * @param {AssignmentBuilder} build_assignment - * @returns {ClassTransformer} - */ -export function create_class_transformer(body, build_state_field, build_assignment) { - /** - * Public, stateful fields. - * @type {Map} - */ - const public_fields = new Map(); - - /** - * Private, stateful fields. These are namespaced separately because - * public and private fields can have the same name in the AST -- ex. - * `count` and `#count` are both named `count` -- and because it's useful - * in a couple of cases to be able to check for only one or the other. - * @type {Map} - */ - const private_fields = new Map(); - - /** - * Accumulates nodes for the new class body. - * @type {Array} - */ - const new_body = []; - - /** - * Private identifiers in use by this analysis. - * Factoid: Unlike public class fields, private fields _must_ be declared in the class body - * before use. So the following is actually a JavaScript syntax error, which means we can - * be 100% certain we know all private fields after parsing the class body: - * - * ```ts - * class Example { - * constructor() { - * this.public = 'foo'; // not a problem! - * this.#private = 'bar'; // JavaScript parser error - * } - * } - * ``` - * @type {Set} - */ - const private_ids = new Set(); - - /** - * A registry of functions to call to complete body modifications. - * Replacements may insert more than one node to the body. The original - * body should not be modified -- instead, replacers should push new - * nodes to new_body. - * - * @type {Array<() => void>} - */ - const replacers = []; - - /** - * Get a state field by name. - * - * @param {string} name - * @param {boolean} is_private - * @param {ReadonlyArray} [kinds] - */ - function get_field(name, is_private, kinds = STATE_CREATION_RUNES) { - const value = (is_private ? private_fields : public_fields).get(name); - if (value && kinds.includes(value.kind)) { - return value; - } - } - - /** - * Create a child context that makes sense for passing to the child analyzers. - * @param {TContext} context - * @returns {TContext} - */ - function create_child_context(context) { - const state = { - ...context.state, - class_transformer - }; - // @ts-expect-error - I can't find a way to make TypeScript happy with these - const visit = (node, state_override) => context.visit(node, { ...state, ...state_override }); - // @ts-expect-error - I can't find a way to make TypeScript happy with these - const next = (state_override) => context.next({ ...state, ...state_override }); - return { - ...context, - state, - visit, - next - }; - } - - /** - * Generate a new body for the class. Ensure there is a visitor for AssignmentExpression that - * calls `generate_assignment` to capture any stateful fields declared in the constructor. - * @param {TContext} context - */ - function generate_body(context) { - const child_context = create_child_context(context); - for (const node of body) { - const was_registered = register_body_definition(node, child_context); - if (!was_registered) { - new_body.push( - /** @type {PropertyDefinition | MethodDefinition} */ ( - // @ts-expect-error generics silliness - child_context.visit(node, child_context.state) - ) - ); - } - } - - for (const replacer of replacers) { - replacer(); - } - - return new_body; - } - - /** - * Given an assignment expression, check to see if that assignment expression declares - * a stateful field. If it does, register that field and then return the processed - * assignment expression. If an assignment expression is returned from this function, - * it should be considered _fully processed_ and should replace the existing assignment - * expression node. - * @param {AssignmentExpression} node - * @param {TContext} context - * @returns {AssignmentExpression | null} The node, if `register_assignment` handled its transformation. - */ - function generate_assignment(node, context) { - const child_context = create_child_context(context); - if ( - !( - node.operator === '=' && - node.left.type === 'MemberExpression' && - node.left.object.type === 'ThisExpression' && - (node.left.property.type === 'Identifier' || - node.left.property.type === 'PrivateIdentifier' || - node.left.property.type === 'Literal') - ) - ) { - return null; - } - - const name = get_name(node.left.property); - if (!name) { - return null; - } - - const parsed = parse_stateful_assignment(node, child_context.state.scope); - if (!parsed) { - return null; - } - const { stateful_assignment, rune } = parsed; - - const is_private = stateful_assignment.left.property.type === 'PrivateIdentifier'; - - let field; - if (is_private) { - field = { - kind: rune, - id: /** @type {PrivateIdentifier} */ (stateful_assignment.left.property) - }; - private_fields.set(name, field); - } else { - field = { - kind: rune, - // it's safe to do this upfront now because we're guaranteed to already know about all private - // identifiers (they had to have been declared at the class root, before we visited the constructor) - id: deconflict(name) - }; - public_fields.set(name, field); - } - - const replacer = () => { - const nodes = build_state_field({ - is_private, - field, - node: stateful_assignment, - context: child_context - }); - if (!nodes) { - return; - } - new_body.push(...nodes); - }; - replacers.push(replacer); - - return build_assignment({ - field, - node: stateful_assignment, - context: child_context - }); - } - - /** - * Register a class body definition. - * - * @param {PropertyDefinition | MethodDefinition | StaticBlock} node - * @param {TContext} child_context - * @returns {boolean} if this node is stateful and was registered - */ - function register_body_definition(node, child_context) { - if (node.type === 'MethodDefinition' && node.kind === 'constructor') { - // life is easier to reason about if we've visited the constructor - // and registered its public state field before we start building - // anything else - replacers.unshift(() => { - new_body.push( - /** @type {MethodDefinition} */ ( - // @ts-expect-error generics silliness - child_context.visit(node, child_context.state) - ) - ); - }); - return true; - } - - if ( - !( - (node.type === 'PropertyDefinition' || node.type === 'MethodDefinition') && - (node.key.type === 'Identifier' || - node.key.type === 'PrivateIdentifier' || - node.key.type === 'Literal') - ) - ) { - return false; - } - - /* - * We don't know if the node is stateful yet, but we still need to register some details. - * For example: If the node is a private identifier, we could accidentally conflict with it later - * if we create a private field for public state (as would happen in this example:) - * - * ```ts - * class Foo { - * #count = 0; - * count = $state(0); // would become #count if we didn't know about the private field above - * } - */ - - const name = get_name(node.key); - if (!name) { - return false; - } - - const is_private = node.key.type === 'PrivateIdentifier'; - if (is_private) { - private_ids.add(name); - } - - const parsed = prop_def_is_stateful(node, child_context.state.scope); - if (!parsed) { - // this isn't a stateful field definition, but if could become one in the constructor -- so we register - // it, but conditionally -- so that if it's added as a field in the constructor (which causes us to create) - // a field definition for it), we don't end up with a duplicate definition (this one, plus the one we create) - replacers.push(() => { - if (!get_field(name, is_private)) { - new_body.push( - /** @type {PropertyDefinition | MethodDefinition} */ ( - // @ts-expect-error generics silliness - child_context.visit(node, child_context.state) - ) - ); - } - }); - return true; - } - const { stateful_prop_def, rune } = parsed; - - let field; - if (is_private) { - field = { - kind: rune, - id: /** @type {PrivateIdentifier} */ (stateful_prop_def.key) - }; - private_fields.set(name, field); - } else { - // We can't set the ID until we've identified all of the private state fields, - // otherwise we might conflict with them. After registering all property definitions, - // call `finalize_property_definitions` to populate the IDs. So long as we don't - // access the ID before the end of this loop, we're fine! - const id = once(() => deconflict(name)); - field = { - kind: rune, - get id() { - return id(); - } - }; - public_fields.set(name, field); - } - - const replacer = () => { - const nodes = build_state_field({ - is_private, - field, - node: stateful_prop_def, - context: child_context - }); - if (!nodes) { - return; - } - new_body.push(...nodes); - }; - replacers.push(replacer); - - return true; - } - - /** - * @param {string} name - * @returns {PrivateIdentifier} - */ - function deconflict(name) { - let deconflicted = name; - while (private_ids.has(deconflicted)) { - deconflicted = '_' + deconflicted; - } - - private_ids.add(deconflicted); - return b.private_id(deconflicted); - } - - /** - * @param {Identifier | PrivateIdentifier | Literal} node - */ - function get_name(node) { - if (node.type === 'Literal') { - let name = node.value?.toString().replace(regex_invalid_identifier_chars, '_'); - - // the above could generate conflicts because it has to generate a valid identifier - // so stuff like `0` and `1` or `state%` and `state^` will result in the same string - // so we have to de-conflict. We can only check `public_fields` because private state - // can't have literal keys - while (name && public_fields.has(name)) { - name = '_' + name; - } - return name; - } else { - return node.name; - } - } - - const class_transformer = { - get_field, - generate_body, - generate_assignment - }; - - return class_transformer; -} - -/** - * `get_rune` is really annoying because it really guarantees this already - * we just need this to tell the type system about it - * @param {AssignmentExpression} node - * @param {Scope} scope - * @returns {{ stateful_assignment: StatefulAssignment, rune: StateCreationRuneName } | null} - */ -function parse_stateful_assignment(node, scope) { - const rune = get_rune(node.right, scope); - if (!rune || !is_state_creation_rune(rune)) { - return null; - } - return { stateful_assignment: /** @type {StatefulAssignment} */ (node), rune }; -} - -/** - * @param {PropertyDefinition | MethodDefinition} node - * @param {Scope} scope - * @returns {{ stateful_prop_def: StatefulPropertyDefinition, rune: StateCreationRuneName } | null} - */ -function prop_def_is_stateful(node, scope) { - const rune = get_rune(node.value, scope); - if (!rune || !is_state_creation_rune(rune)) { - return null; - } - return { stateful_prop_def: /** @type {StatefulPropertyDefinition} */ (node), rune }; -} diff --git a/packages/svelte/src/compiler/phases/3-transform/shared/types.d.ts b/packages/svelte/src/compiler/phases/3-transform/shared/types.d.ts deleted file mode 100644 index 7c03be2235ae..000000000000 --- a/packages/svelte/src/compiler/phases/3-transform/shared/types.d.ts +++ /dev/null @@ -1,82 +0,0 @@ -import type { - AssignmentExpression, - CallExpression, - Identifier, - MemberExpression, - PropertyDefinition, - MethodDefinition, - PrivateIdentifier, - ThisExpression, - Literal -} from 'estree'; -import type { StateField } from '../types'; -import type { Context as ServerContext } from '../server/types'; -import type { Context as ClientContext } from '../client/types'; -import type { StateCreationRuneName } from '../../../../utils'; - -export type StatefulAssignment = AssignmentExpression & { - left: MemberExpression & { - object: ThisExpression; - property: Identifier | PrivateIdentifier | Literal; - }; - right: CallExpression; -}; - -export type StatefulPropertyDefinition = PropertyDefinition & { - key: Identifier | PrivateIdentifier | Literal; - value: CallExpression; -}; - -export type StateFieldBuilderParams = { - is_private: boolean; - field: StateField; - node: StatefulAssignment | StatefulPropertyDefinition; - context: TContext; -}; - -export type StateFieldBuilder = ( - params: StateFieldBuilderParams -) => Array; - -export type AssignmentBuilderParams = { - node: StatefulAssignment; - field: StateField; - context: TContext; -}; - -export type AssignmentBuilder = ( - params: AssignmentBuilderParams -) => AssignmentExpression; - -export type ClassTransformer = { - /** - * @param name - The name of the field. - * @param is_private - Whether the field is private (whether its name starts with '#'). - * @param kinds - What kinds of state creation runes you're looking for, eg. only '$derived.by'. - * @returns The field if it exists and matches the given criteria, or null. - */ - get_field: ( - name: string, - is_private: boolean, - kinds?: Array - ) => StateField | undefined; - - /** - * Given the body of a class, generate a new body with stateful fields. - * This assumes that {@link register_assignment} is registered to be called - * for all `AssignmentExpression` nodes in the class body. - * @param context - The context associated with the `ClassBody`. - * @returns The new body. - */ - generate_body: (context: TContext) => Array; - - /** - * Register an assignment expression. This checks to see if the assignment is creating - * a state field on the class. If it is, it registers that state field and modifies the - * assignment expression. - */ - generate_assignment: ( - node: AssignmentExpression, - context: TContext - ) => AssignmentExpression | null; -}; From fbf1b4e9fb4a320aff7002a83c9d89f7e5235e10 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 16 May 2025 16:47:24 -0400 Subject: [PATCH 36/64] revert snapshot tests --- .../_expected/client/index.svelte.js | 10 +++++----- .../_expected/server/index.svelte.js | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/client/index.svelte.js index 620623ebfd7d..21339741761f 100644 --- a/packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/client/index.svelte.js @@ -5,11 +5,6 @@ export default function Class_state_field_constructor_assignment($$anchor, $$pro $.push($$props, true); class Foo { - constructor() { - this.a = 1; - $.set(this.#b, 2); - } - #a = $.state(); get a() { @@ -21,6 +16,11 @@ export default function Class_state_field_constructor_assignment($$anchor, $$pro } #b = $.state(); + + constructor() { + this.a = 1; + $.set(this.#b, 2); + } } $.pop(); diff --git a/packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/server/index.svelte.js index a27bcb8623a7..2a115a498373 100644 --- a/packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/server/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/server/index.svelte.js @@ -4,13 +4,13 @@ export default function Class_state_field_constructor_assignment($$payload, $$pr $.push(); class Foo { + a; + #b; + constructor() { this.a = 1; this.#b = 2; } - - a; - #b; } $.pop(); From de015b04bcdef494182db2f5399f0d3730f17172 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 16 May 2025 16:52:36 -0400 Subject: [PATCH 37/64] unused --- packages/svelte/src/compiler/phases/2-analyze/types.d.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/svelte/src/compiler/phases/2-analyze/types.d.ts b/packages/svelte/src/compiler/phases/2-analyze/types.d.ts index 328dc4deb168..7ccd175ff3c2 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/types.d.ts +++ b/packages/svelte/src/compiler/phases/2-analyze/types.d.ts @@ -1,7 +1,6 @@ import type { Scope } from '../scope.js'; import type { ComponentAnalysis, ReactiveStatement } from '../types.js'; import type { AST, ExpressionMetadata, StateField, ValidatedCompileOptions } from '#compiler'; -import type { ClassBody } from 'estree'; export interface AnalysisState { scope: Scope; @@ -20,7 +19,7 @@ export interface AnalysisState { /** Information about the current expression/directive/block value */ expression: ExpressionMetadata | null; - /** Used to analyze class state. */ + /** Used to analyze class state */ state_fields: Record | null; function_depth: number; From 4653f540104db912627e82fce621336032525bdb Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 16 May 2025 17:03:07 -0400 Subject: [PATCH 38/64] note to self --- .../phases/3-transform/client/visitors/CallExpression.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/CallExpression.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/CallExpression.js index 22b715868fc3..fdab52a49898 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/CallExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/CallExpression.js @@ -19,6 +19,7 @@ export function CallExpression(node, context) { case '$effect.tracking': return b.call('$.effect_tracking'); + // TODO can we reuse this logic for normal declarations, i.e. fall through to this? case '$state': case '$state.raw': { let should_proxy = rune === '$state' && true; // TODO From 23f269be7ca2ed39c05faaa1e3e6bbcd8eab5ae9 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 16 May 2025 17:10:08 -0400 Subject: [PATCH 39/64] fix --- .../compiler/phases/3-transform/client/visitors/ClassBody.js | 2 +- .../compiler/phases/3-transform/server/visitors/ClassBody.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js index 69bc151cee90..b18e4569cba4 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js @@ -92,7 +92,7 @@ export function ClassBody(node, context) { // Replace parts of the class body for (const definition of node.body) { - if (definition.type === 'MethodDefinition' || definition.type === 'StaticBlock') { + if (definition.type !== 'PropertyDefinition') { body.push( /** @type {MethodDefinition | StaticBlock} */ (context.visit(definition, child_state)) ); diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/ClassBody.js index ed32cfa954a9..14bd6204cedb 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/ClassBody.js @@ -89,7 +89,7 @@ export function ClassBody(node, context) { // Replace parts of the class body for (const definition of node.body) { - if (definition.type === 'MethodDefinition' || definition.type === 'StaticBlock') { + if (definition.type !== 'PropertyDefinition') { body.push( /** @type {MethodDefinition | StaticBlock} */ (context.visit(definition, child_state)) ); From d1bb2c9555e89ebcfadef40dd3d11f061ef59bf3 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 16 May 2025 17:10:33 -0400 Subject: [PATCH 40/64] unused --- .../3-transform/client/visitors/ClassBody.js | 79 ------------------- 1 file changed, 79 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js index b18e4569cba4..c11a7aca25cd 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js @@ -137,86 +137,7 @@ export function ClassBody(node, context) { ) ); } - - // if (definition.type === 'PropertyDefinition') { - // const original_name = get_name(definition.key); - // if (original_name === null) continue; - - // const name = definition_names[original_name]; - - // const is_private = definition.key.type === 'PrivateIdentifier'; - // const field = (is_private ? private_state : public_state).get(name); - - // if (definition.value?.type === 'CallExpression' && field !== undefined) { - // let value = null; - - // if (definition.value.arguments.length > 0) { - // const init = /** @type {Expression} **/ ( - // context.visit(definition.value.arguments[0], child_state) - // ); - - // value = - // field.kind === 'state' - // ? b.call( - // '$.state', - // should_proxy(init, context.state.scope) ? b.call('$.proxy', init) : init - // ) - // : field.kind === 'raw_state' - // ? b.call('$.state', init) - // : field.kind === 'derived_by' - // ? b.call('$.derived', init) - // : b.call('$.derived', b.thunk(init)); - // } else { - // // if no arguments, we know it's state as `$derived()` is a compile error - // value = b.call('$.state'); - // } - - // if (is_private) { - // body.push(b.prop_def(field.id, value)); - // } else { - // // #foo; - // const member = b.member(b.this, field.id); - // body.push(b.prop_def(field.id, value)); - - // // get foo() { return this.#foo; } - // body.push(b.method('get', definition.key, [], [b.return(b.call('$.get', member))])); - - // // set foo(value) { this.#foo = value; } - // const val = b.id('value'); - - // body.push( - // b.method( - // 'set', - // definition.key, - // [val], - // [b.stmt(b.call('$.set', member, val, field.kind === 'state' && b.true))] - // ) - // ); - // } - // continue; - // } - // } - - // body.push(/** @type {MethodDefinition} **/ (context.visit(definition, child_state))); } return { ...node, body }; } - -/** - * @param {string} name - * @param {Map} public_state - */ -function get_deconflicted_name(name, public_state) { - name = name.replace(regex_invalid_identifier_chars, '_'); - - // the above could generate conflicts because it has to generate a valid identifier - // so stuff like `0` and `1` or `state%` and `state^` will result in the same string - // so we have to de-conflict. We can only check `public_state` because private state - // can't have literal keys - while (name && public_state.has(name)) { - name = '_' + name; - } - - return name; -} From de77e6962840ffea82b5937833d46d9c7fe13bae Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 16 May 2025 17:11:17 -0400 Subject: [PATCH 41/64] unused --- .../phases/3-transform/client/visitors/ClassBody.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js index c11a7aca25cd..8b141671930b 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js @@ -1,10 +1,8 @@ -/** @import { CallExpression, ClassBody, Expression, Identifier, Literal, MethodDefinition, PrivateIdentifier, PropertyDefinition, StaticBlock } from 'estree' */ -/** @import { Context, StateField } from '../types' */ +/** @import { CallExpression, ClassBody, MethodDefinition, PrivateIdentifier, PropertyDefinition, StaticBlock } from 'estree' */ +/** @import { Context } from '../types' */ import * as b from '#compiler/builders'; import { get_name } from '../../../nodes.js'; import { regex_invalid_identifier_chars } from '../../../patterns.js'; -import { get_rune } from '../../../scope.js'; -import { should_proxy } from '../utils.js'; /** * @param {ClassBody} node From dfb4e13cddd832fb15b40fd66b335fc821d46f74 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 16 May 2025 17:24:19 -0400 Subject: [PATCH 42/64] remove some unused stuff --- .../phases/3-transform/client/transform-client.js | 8 ++++---- .../src/compiler/phases/3-transform/client/types.d.ts | 10 +++------- .../client/visitors/AssignmentExpression.js | 10 +++++----- .../phases/3-transform/client/visitors/ClassBody.js | 5 +---- .../3-transform/client/visitors/MemberExpression.js | 3 ++- .../3-transform/client/visitors/UpdateExpression.js | 2 +- .../phases/3-transform/server/visitors/ClassBody.js | 5 +---- 7 files changed, 17 insertions(+), 26 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js index 9a2f4dd34c70..fe789852ef92 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js @@ -163,8 +163,8 @@ export function client_component(analysis, options) { }, events: new Set(), preserve_whitespace: options.preserveWhitespace, - public_state: new Map(), - private_state: new Map(), + state_fields: {}, + backing_fields: {}, transform: {}, in_constructor: false, instance_level_snippets: [], @@ -671,8 +671,8 @@ export function client_module(analysis, options) { options, scope: analysis.module.scope, scopes: analysis.module.scopes, - public_state: new Map(), - private_state: new Map(), + state_fields: {}, + backing_fields: {}, transform: {}, in_constructor: false }; diff --git a/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts b/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts index 040105342564..0db7be1fe1c0 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts +++ b/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts @@ -9,14 +9,14 @@ import type { UpdateExpression, VariableDeclaration } from 'estree'; -import type { AST, Namespace, ValidatedCompileOptions } from '#compiler'; +import type { AST, Namespace, StateField, ValidatedCompileOptions } from '#compiler'; import type { TransformState } from '../types.js'; import type { ComponentAnalysis } from '../../types.js'; import type { SourceLocation } from '#shared'; export interface ClientTransformState extends TransformState { - readonly private_state: Map; - readonly public_state: Map; + readonly state_fields: Record; + readonly backing_fields: Record; /** * `true` if the current lexical scope belongs to a class constructor. this allows @@ -94,10 +94,6 @@ export interface ComponentClientTransformState extends ClientTransformState { readonly module_level_snippets: VariableDeclaration[]; } -export interface StateField { - type: '$state' | '$state.raw' | '$derived' | '$derived.by'; -} - export type Context = import('zimmerframe').Context; export type Visitors = import('zimmerframe').Visitors; diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js index d23ecb26dbb8..1d2cf585a69f 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js @@ -53,6 +53,8 @@ const callees = { */ function build_assignment(operator, left, right, context) { if (context.state.analysis.runes && left.type === 'MemberExpression') { + const name = get_name(left.property); + // special case — state declaration in class constructor const ancestor = context.path.at(-4); @@ -60,8 +62,6 @@ function build_assignment(operator, left, right, context) { const rune = get_rune(right, context.state.scope); if (rune) { - const name = get_name(left.property); - const child_state = { ...context.state, in_constructor: rune !== '$derived' && rune !== '$derived.by' @@ -82,15 +82,15 @@ function build_assignment(operator, left, right, context) { // special case — assignment to private state field if (left.property.type === 'PrivateIdentifier') { - const private_state = context.state.private_state.get(left.property.name); + const field = context.state.state_fields[name]; - if (private_state !== undefined) { + if (field) { let value = /** @type {Expression} */ ( context.visit(build_assignment_value(operator, left, right)) ); const needs_proxy = - private_state.type === '$state' && + field.type === '$state' && is_non_coercive_operator(operator) && should_proxy(value, context.state.scope); diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js index 8b141671930b..2867c456c5b4 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js @@ -29,8 +29,6 @@ export function ClassBody(node, context) { } } - const private_state = new Map(); - /** * each `foo = $state()` needs a backing `#foo` field * @type {Record} @@ -39,7 +37,6 @@ export function ClassBody(node, context) { for (const name in state_fields) { if (name[0] === '#') { - private_state.set(name.slice(1), state_fields[name]); continue; } @@ -55,7 +52,7 @@ export function ClassBody(node, context) { /** @type {Array} */ const body = []; - const child_state = { ...context.state, state_fields, backing_fields, private_state }; // TODO populate private_state + const child_state = { ...context.state, state_fields, backing_fields }; for (const name in state_fields) { if (name[0] === '#') { diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/MemberExpression.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/MemberExpression.js index bc9d26367002..ded727dc0f4f 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/MemberExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/MemberExpression.js @@ -9,7 +9,8 @@ import * as b from '#compiler/builders'; export function MemberExpression(node, context) { // rewrite `this.#foo` as `this.#foo.v` inside a constructor if (node.property.type === 'PrivateIdentifier') { - const field = context.state.private_state.get(node.property.name); + const field = context.state.state_fields['#' + node.property.name]; + if (field) { return context.state.in_constructor && (field.type === '$state.raw' || field.type === '$state') diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/UpdateExpression.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/UpdateExpression.js index 96be119b840a..c3c9db6270de 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/UpdateExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/UpdateExpression.js @@ -15,7 +15,7 @@ export function UpdateExpression(node, context) { argument.type === 'MemberExpression' && argument.object.type === 'ThisExpression' && argument.property.type === 'PrivateIdentifier' && - context.state.private_state.has(argument.property.name) + Object.hasOwn(context.state.state_fields, '#' + argument.property.name) ) { let fn = '$.update'; if (node.prefix) fn += '_pre'; diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/ClassBody.js index 14bd6204cedb..e3a1fefb6dc1 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/ClassBody.js @@ -32,8 +32,6 @@ export function ClassBody(node, context) { } } - const private_state = new Map(); - /** * each `foo = $state()` needs a backing `#foo` field * @type {Record} @@ -42,7 +40,6 @@ export function ClassBody(node, context) { for (const name in state_fields) { if (name[0] === '#') { - private_state.set(name.slice(1), state_fields[name]); continue; } @@ -58,7 +55,7 @@ export function ClassBody(node, context) { /** @type {Array} */ const body = []; - const child_state = { ...context.state, state_fields, backing_fields, private_state }; // TODO populate private_state + const child_state = { ...context.state, state_fields, backing_fields }; for (const name in state_fields) { if (name[0] === '#') { From 255603111b8ca0a8fe5d9799d18c477636154f14 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 16 May 2025 17:24:40 -0400 Subject: [PATCH 43/64] unused --- .../compiler/phases/3-transform/server/visitors/ClassBody.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/ClassBody.js index e3a1fefb6dc1..09bfd2585f75 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/ClassBody.js @@ -1,9 +1,6 @@ -/** @import { CallExpression, ClassBody, Expression, MethodDefinition, PrivateIdentifier, PropertyDefinition, StaticBlock } from 'estree' */ +/** @import { CallExpression, ClassBody, MethodDefinition, PrivateIdentifier, PropertyDefinition, StaticBlock } from 'estree' */ /** @import { Context } from '../types.js' */ -/** @import { StateField } from '../../client/types.js' */ -import { dev } from '../../../../state.js'; import * as b from '#compiler/builders'; -import { get_rune } from '../../../scope.js'; import { regex_invalid_identifier_chars } from '../../../patterns.js'; import { get_name } from '../../../nodes.js'; From c7e8422cebd4b87e67e6d0595f3c08d4a9052d3f Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 16 May 2025 17:32:55 -0400 Subject: [PATCH 44/64] lint, tidy up --- .../phases/3-transform/client/types.d.ts | 3 - .../client/visitors/AssignmentExpression.js | 68 ++++++++++--------- .../3-transform/server/transform-server.js | 8 +-- .../phases/3-transform/server/types.d.ts | 2 - .../server/visitors/AssignmentExpression.js | 29 ++++---- .../3-transform/server/visitors/ClassBody.js | 9 +-- .../server/visitors/MemberExpression.js | 23 ------- .../compiler/phases/3-transform/types.d.ts | 6 +- 8 files changed, 61 insertions(+), 87 deletions(-) delete mode 100644 packages/svelte/src/compiler/phases/3-transform/server/visitors/MemberExpression.js diff --git a/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts b/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts index 0db7be1fe1c0..763b6773c7ce 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts +++ b/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts @@ -15,9 +15,6 @@ import type { ComponentAnalysis } from '../../types.js'; import type { SourceLocation } from '#shared'; export interface ClientTransformState extends TransformState { - readonly state_fields: Record; - readonly backing_fields: Record; - /** * `true` if the current lexical scope belongs to a class constructor. this allows * us to rewrite `this.foo` as `this.#foo.value` diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js index 1d2cf585a69f..294faa43ddcb 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js @@ -55,46 +55,48 @@ function build_assignment(operator, left, right, context) { if (context.state.analysis.runes && left.type === 'MemberExpression') { const name = get_name(left.property); - // special case — state declaration in class constructor - const ancestor = context.path.at(-4); - - if (ancestor?.type === 'MethodDefinition' && ancestor.kind === 'constructor') { - const rune = get_rune(right, context.state.scope); - - if (rune) { - const child_state = { - ...context.state, - in_constructor: rune !== '$derived' && rune !== '$derived.by' - }; - - const l = b.member( - b.this, - left.property.type === 'PrivateIdentifier' - ? left.property - : context.state.backing_fields[name] - ); + if (name !== null) { + // special case — state declaration in class constructor + const ancestor = context.path.at(-4); + + if (ancestor?.type === 'MethodDefinition' && ancestor.kind === 'constructor') { + const rune = get_rune(right, context.state.scope); + + if (rune) { + const child_state = { + ...context.state, + in_constructor: rune !== '$derived' && rune !== '$derived.by' + }; + + const l = b.member( + b.this, + left.property.type === 'PrivateIdentifier' + ? left.property + : context.state.backing_fields[name] + ); - const r = /** @type {Expression} */ (context.visit(right, child_state)); + const r = /** @type {Expression} */ (context.visit(right, child_state)); - return b.assignment(operator, l, r); + return b.assignment(operator, l, r); + } } - } - // special case — assignment to private state field - if (left.property.type === 'PrivateIdentifier') { - const field = context.state.state_fields[name]; + // special case — assignment to private state field + if (left.property.type === 'PrivateIdentifier') { + const field = context.state.state_fields[name]; - if (field) { - let value = /** @type {Expression} */ ( - context.visit(build_assignment_value(operator, left, right)) - ); + if (field) { + let value = /** @type {Expression} */ ( + context.visit(build_assignment_value(operator, left, right)) + ); - const needs_proxy = - field.type === '$state' && - is_non_coercive_operator(operator) && - should_proxy(value, context.state.scope); + const needs_proxy = + field.type === '$state' && + is_non_coercive_operator(operator) && + should_proxy(value, context.state.scope); - return b.call('$.set', left, value, needs_proxy && b.true); + return b.call('$.set', left, value, needs_proxy && b.true); + } } } } diff --git a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js index e7896991d98f..ef4e1577d316 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js @@ -23,7 +23,6 @@ import { Identifier } from './visitors/Identifier.js'; import { IfBlock } from './visitors/IfBlock.js'; import { KeyBlock } from './visitors/KeyBlock.js'; import { LabeledStatement } from './visitors/LabeledStatement.js'; -import { MemberExpression } from './visitors/MemberExpression.js'; import { PropertyDefinition } from './visitors/PropertyDefinition.js'; import { RegularElement } from './visitors/RegularElement.js'; import { RenderTag } from './visitors/RenderTag.js'; @@ -49,7 +48,6 @@ const global_visitors = { ExpressionStatement, Identifier, LabeledStatement, - MemberExpression, PropertyDefinition, UpdateExpression, VariableDeclaration @@ -99,7 +97,8 @@ export function server_component(analysis, options) { template: /** @type {any} */ (null), namespace: options.namespace, preserve_whitespace: options.preserveWhitespace, - private_derived: new Map(), + state_fields: {}, + backing_fields: {}, skip_hydration_boundaries: false }; @@ -395,7 +394,8 @@ export function server_module(analysis, options) { // to be present for `javascript_visitors_legacy` and so is included in module // transform state as well as component transform state legacy_reactive_statements: new Map(), - private_derived: new Map() + state_fields: {}, + backing_fields: {} }; const module = /** @type {Program} */ ( diff --git a/packages/svelte/src/compiler/phases/3-transform/server/types.d.ts b/packages/svelte/src/compiler/phases/3-transform/server/types.d.ts index 971271642cfc..adde7480cbd1 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/types.d.ts +++ b/packages/svelte/src/compiler/phases/3-transform/server/types.d.ts @@ -2,12 +2,10 @@ import type { Expression, Statement, ModuleDeclaration, LabeledStatement } from import type { AST, Namespace, ValidatedCompileOptions } from '#compiler'; import type { TransformState } from '../types.js'; import type { ComponentAnalysis } from '../../types.js'; -import type { StateField } from '../client/types.js'; export interface ServerTransformState extends TransformState { /** The $: calls, which will be ordered in the end */ readonly legacy_reactive_statements: Map; - readonly private_derived: Map; } export interface ComponentServerTransformState extends ServerTransformState { diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/AssignmentExpression.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/AssignmentExpression.js index defcf1367277..abb073a7d2e4 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/AssignmentExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/AssignmentExpression.js @@ -25,24 +25,27 @@ export function AssignmentExpression(node, context) { */ function build_assignment(operator, left, right, context) { if (context.state.analysis.runes && left.type === 'MemberExpression') { - // special case — state declaration in class constructor - const ancestor = context.path.at(-4); + const name = get_name(left.property); - if (ancestor?.type === 'MethodDefinition' && ancestor.kind === 'constructor') { - const rune = get_rune(right, context.state.scope); + if (name !== null) { + // special case — state declaration in class constructor + const ancestor = context.path.at(-4); - if (rune) { - const name = get_name(left.property); - const key = - left.property.type === 'PrivateIdentifier' || rune === '$state' || rune === '$state.raw' - ? left.property - : context.state.backing_fields[name]; + if (ancestor?.type === 'MethodDefinition' && ancestor.kind === 'constructor') { + const rune = get_rune(right, context.state.scope); - const l = b.member(b.this, key, key.type === 'Literal'); + if (rune) { + const key = + left.property.type === 'PrivateIdentifier' || rune === '$state' || rune === '$state.raw' + ? left.property + : context.state.backing_fields[name]; - const r = /** @type {Expression} */ (context.visit(right)); + const l = b.member(b.this, key, key.type === 'Literal'); - return b.assignment(operator, l, r); + const r = /** @type {Expression} */ (context.visit(right)); + + return b.assignment(operator, l, r); + } } } } diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/ClassBody.js index 09bfd2585f75..03e058ea0f62 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/ClassBody.js @@ -69,14 +69,9 @@ export function ClassBody(node, context) { const backing = backing_fields[name]; const member = b.member(b.this, backing); - const should_proxy = field.type === '$state' && true; // TODO - - const key = b.key(name); - body.push( b.prop_def(backing, null), - - b.method('get', key, [], [b.return(b.call(member))]) + b.method('get', b.key(name), [], [b.return(b.call(member))]) ); } } @@ -108,8 +103,6 @@ export function ClassBody(node, context) { const backing = backing_fields[name]; const member = b.member(b.this, backing); - const should_proxy = field.type === '$state' && true; // TODO - body.push( b.prop_def( backing, diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/MemberExpression.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/MemberExpression.js deleted file mode 100644 index 73631395e66e..000000000000 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/MemberExpression.js +++ /dev/null @@ -1,23 +0,0 @@ -/** @import { MemberExpression } from 'estree' */ -/** @import { Context } from '../types.js' */ -import * as b from '#compiler/builders'; - -/** - * @param {MemberExpression} node - * @param {Context} context - */ -export function MemberExpression(node, context) { - if ( - context.state.analysis.runes && - node.object.type === 'ThisExpression' && - node.property.type === 'PrivateIdentifier' - ) { - const field = context.state.private_derived.get(node.property.name); - - if (field) { - return b.call(node); - } - } - - context.next(); -} diff --git a/packages/svelte/src/compiler/phases/3-transform/types.d.ts b/packages/svelte/src/compiler/phases/3-transform/types.d.ts index 5d860207dde6..cb866abef1d9 100644 --- a/packages/svelte/src/compiler/phases/3-transform/types.d.ts +++ b/packages/svelte/src/compiler/phases/3-transform/types.d.ts @@ -1,10 +1,14 @@ import type { Scope } from '../scope.js'; -import type { AST, ValidatedModuleCompileOptions } from '#compiler'; +import type { AST, StateField, ValidatedModuleCompileOptions } from '#compiler'; import type { Analysis } from '../types.js'; +import type { PrivateIdentifier } from 'estree'; export interface TransformState { readonly analysis: Analysis; readonly options: ValidatedModuleCompileOptions; readonly scope: Scope; readonly scopes: Map; + + readonly state_fields: Record; + readonly backing_fields: Record; } From 1a78e279d68a5d6dbf9a5c3d6cdbdde55266b7bc Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 16 May 2025 17:39:16 -0400 Subject: [PATCH 45/64] reuse helper --- .../src/compiler/phases/2-analyze/visitors/ClassBody.js | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js index 1fe22128f4d5..5f390e001238 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js @@ -4,6 +4,7 @@ import { get_rune } from '../../scope.js'; import * as e from '../../../errors.js'; import { is_state_creation_rune } from '../../../../utils.js'; +import { get_name } from '../../nodes.js'; /** * @param {ClassBody} node @@ -32,12 +33,8 @@ export function ClassBody(node, context) { * @param {Expression | null | undefined} value */ function handle(node, key, value) { - const name = - (key.type === 'Literal' && String(key.value)) || - (key.type === 'PrivateIdentifier' && '#' + key.name) || - (key.type === 'Identifier' && key.name); - - if (!name) return; + const name = get_name(key); + if (name === null) return; const rune = get_rune(value, context.state.scope); From 90e7e0dce5462b1454c9b5e17fa53c812f4e3db9 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 16 May 2025 17:51:30 -0400 Subject: [PATCH 46/64] tweak --- .../2-analyze/visitors/CallExpression.js | 31 +++++++------------ 1 file changed, 11 insertions(+), 20 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 ba690eb661bc..33abb52cac5c 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js @@ -274,7 +274,6 @@ function get_function_label(nodes) { } /** - * * @param {AST.SvelteNode} parent * @param {Context} context */ @@ -283,7 +282,6 @@ function is_variable_declaration(parent, context) { } /** - * * @param {AST.SvelteNode} parent */ function is_class_property_definition(parent) { @@ -293,28 +291,21 @@ function is_class_property_definition(parent) { /** * @param {AST.SvelteNode} node * @param {Context} context - * @returns {node is AssignmentExpression & { left: { type: 'MemberExpression' } & { object: { type: 'ThisExpression' }; property: { type: 'Identifier' | 'PrivateIdentifier' | 'Literal' } } }} */ function is_class_property_assignment_at_constructor_root(node, context) { if ( - !( - node.type === 'AssignmentExpression' && - node.operator === '=' && - node.left.type === 'MemberExpression' && - node.left.object.type === 'ThisExpression' && - ((node.left.property.type === 'Identifier' && !node.left.computed) || - node.left.property.type === 'PrivateIdentifier' || - node.left.property.type === 'Literal') - ) + node.type === 'AssignmentExpression' && + node.operator === '=' && + node.left.type === 'MemberExpression' && + node.left.object.type === 'ThisExpression' && + ((node.left.property.type === 'Identifier' && !node.left.computed) || + node.left.property.type === 'PrivateIdentifier' || + node.left.property.type === 'Literal') ) { - return false; + // MethodDefinition (-5) -> FunctionExpression (-4) -> BlockStatement (-3) -> ExpressionStatement (-2) -> AssignmentExpression (-1) + const parent = get_parent(context.path, -5); + return parent?.type === 'MethodDefinition' && parent.kind === 'constructor'; } - // AssignmentExpression (here) -> ExpressionStatement (-1) -> BlockStatement (-2) -> FunctionExpression (-3) -> MethodDefinition (-4) - const maybe_constructor = get_parent(context.path, -5); - return ( - maybe_constructor && - maybe_constructor.type === 'MethodDefinition' && - maybe_constructor.kind === 'constructor' - ); + return false; } From a6cc07c425c3057dd54715ef354980349510a60b Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 16 May 2025 18:03:21 -0400 Subject: [PATCH 47/64] simplify/DRY --- .../phases/2-analyze/visitors/ClassBody.js | 32 +++++++++++++ .../3-transform/client/transform-client.js | 2 - .../client/visitors/AssignmentExpression.js | 9 ++-- .../3-transform/client/visitors/ClassBody.js | 44 ++--------------- .../3-transform/server/transform-server.js | 4 +- .../server/visitors/AssignmentExpression.js | 4 +- .../3-transform/server/visitors/ClassBody.js | 47 +++---------------- .../compiler/phases/3-transform/types.d.ts | 1 - packages/svelte/src/compiler/types/index.d.ts | 8 +++- 9 files changed, 57 insertions(+), 94 deletions(-) diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js index 5f390e001238..c2635e73f191 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js @@ -1,10 +1,12 @@ /** @import { AssignmentExpression, CallExpression, ClassBody, PropertyDefinition, Expression, PrivateIdentifier, MethodDefinition } from 'estree' */ /** @import { StateField } from '#compiler' */ /** @import { Context } from '../types' */ +import * as b from '#compiler/builders'; import { get_rune } from '../../scope.js'; import * as e from '../../../errors.js'; import { is_state_creation_rune } from '../../../../utils.js'; import { get_name } from '../../nodes.js'; +import { regex_invalid_identifier_chars } from '../../patterns.js'; /** * @param {ClassBody} node @@ -16,6 +18,18 @@ export function ClassBody(node, context) { return; } + /** @type {string[]} */ + const private_ids = []; + + for (const prop of node.body) { + if ( + (prop.type === 'MethodDefinition' || prop.type === 'PropertyDefinition') && + prop.key.type === 'PrivateIdentifier' + ) { + private_ids.push(prop.key.name); + } + } + /** @type {Record} */ const state_fields = {}; @@ -46,6 +60,8 @@ export function ClassBody(node, context) { state_fields[name] = { node, type: rune, + // @ts-expect-error for public state this is filled out in a moment + key: key.type === 'PrivateIdentifier' ? key : null, value: /** @type {CallExpression} */ (value) }; } @@ -84,6 +100,22 @@ export function ClassBody(node, context) { } } + for (const name in state_fields) { + if (name[0] === '#') { + continue; + } + + const field = state_fields[name]; + + let deconflicted = name.replace(regex_invalid_identifier_chars, '_'); + while (private_ids.includes(deconflicted)) { + deconflicted = '_' + deconflicted; + } + + private_ids.push(deconflicted); + field.key = b.private_id(deconflicted); + } + context.next({ ...context.state, state_fields diff --git a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js index fe789852ef92..711c1e64d88f 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js @@ -164,7 +164,6 @@ export function client_component(analysis, options) { events: new Set(), preserve_whitespace: options.preserveWhitespace, state_fields: {}, - backing_fields: {}, transform: {}, in_constructor: false, instance_level_snippets: [], @@ -672,7 +671,6 @@ export function client_module(analysis, options) { scope: analysis.module.scope, scopes: analysis.module.scopes, state_fields: {}, - backing_fields: {}, transform: {}, in_constructor: false }; diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js index 294faa43ddcb..6885be810fe0 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js @@ -68,12 +68,9 @@ function build_assignment(operator, left, right, context) { in_constructor: rune !== '$derived' && rune !== '$derived.by' }; - const l = b.member( - b.this, - left.property.type === 'PrivateIdentifier' - ? left.property - : context.state.backing_fields[name] - ); + const field = context.state.state_fields[name]; + + const l = b.member(b.this, field.key); const r = /** @type {Expression} */ (context.visit(right, child_state)); diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js index 2867c456c5b4..56eae3a763fa 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js @@ -17,42 +17,10 @@ export function ClassBody(node, context) { return; } - /** @type {string[]} */ - const private_ids = []; - - for (const prop of node.body) { - if ( - (prop.type === 'MethodDefinition' || prop.type === 'PropertyDefinition') && - prop.key.type === 'PrivateIdentifier' - ) { - private_ids.push(prop.key.name); - } - } - - /** - * each `foo = $state()` needs a backing `#foo` field - * @type {Record} - */ - const backing_fields = {}; - - for (const name in state_fields) { - if (name[0] === '#') { - continue; - } - - let deconflicted = name.replace(regex_invalid_identifier_chars, '_'); - while (private_ids.includes(deconflicted)) { - deconflicted = '_' + deconflicted; - } - - private_ids.push(deconflicted); - backing_fields[name] = b.private_id(deconflicted); - } - /** @type {Array} */ const body = []; - const child_state = { ...context.state, state_fields, backing_fields }; + const child_state = { ...context.state, state_fields }; for (const name in state_fields) { if (name[0] === '#') { @@ -63,15 +31,14 @@ export function ClassBody(node, context) { // insert backing fields for stuff declared in the constructor if (field.node.type === 'AssignmentExpression') { - const backing = backing_fields[name]; - const member = b.member(b.this, backing); + const member = b.member(b.this, field.key); const should_proxy = field.type === '$state' && true; // TODO const key = b.key(name); body.push( - b.prop_def(backing, null), + b.prop_def(field.key, null), b.method('get', key, [], [b.return(b.call('$.get', member))]), @@ -109,14 +76,13 @@ export function ClassBody(node, context) { continue; } - const backing = backing_fields[name]; - const member = b.member(b.this, backing); + const member = b.member(b.this, field.key); const should_proxy = field.type === '$state' && true; // TODO body.push( b.prop_def( - backing, + field.key, /** @type {CallExpression} */ ( context.visit(definition.value ?? field.value, child_state) ) diff --git a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js index ef4e1577d316..2c677116dcb5 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js @@ -98,7 +98,6 @@ export function server_component(analysis, options) { namespace: options.namespace, preserve_whitespace: options.preserveWhitespace, state_fields: {}, - backing_fields: {}, skip_hydration_boundaries: false }; @@ -394,8 +393,7 @@ export function server_module(analysis, options) { // to be present for `javascript_visitors_legacy` and so is included in module // transform state as well as component transform state legacy_reactive_statements: new Map(), - state_fields: {}, - backing_fields: {} + state_fields: {} }; const module = /** @type {Program} */ ( diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/AssignmentExpression.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/AssignmentExpression.js index abb073a7d2e4..e80187d013f7 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/AssignmentExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/AssignmentExpression.js @@ -35,10 +35,12 @@ function build_assignment(operator, left, right, context) { const rune = get_rune(right, context.state.scope); if (rune) { + const field = context.state.state_fields[name]; + const key = left.property.type === 'PrivateIdentifier' || rune === '$state' || rune === '$state.raw' ? left.property - : context.state.backing_fields[name]; + : field.key; const l = b.member(b.this, key, key.type === 'Literal'); diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/ClassBody.js index 03e058ea0f62..fb3cf89026c5 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/ClassBody.js @@ -1,7 +1,6 @@ -/** @import { CallExpression, ClassBody, MethodDefinition, PrivateIdentifier, PropertyDefinition, StaticBlock } from 'estree' */ +/** @import { CallExpression, ClassBody, MethodDefinition, PropertyDefinition, StaticBlock } from 'estree' */ /** @import { Context } from '../types.js' */ import * as b from '#compiler/builders'; -import { regex_invalid_identifier_chars } from '../../../patterns.js'; import { get_name } from '../../../nodes.js'; /** @@ -17,42 +16,10 @@ export function ClassBody(node, context) { return; } - /** @type {string[]} */ - const private_ids = []; - - for (const prop of node.body) { - if ( - (prop.type === 'MethodDefinition' || prop.type === 'PropertyDefinition') && - prop.key.type === 'PrivateIdentifier' - ) { - private_ids.push(prop.key.name); - } - } - - /** - * each `foo = $state()` needs a backing `#foo` field - * @type {Record} - */ - const backing_fields = {}; - - for (const name in state_fields) { - if (name[0] === '#') { - continue; - } - - let deconflicted = name.replace(regex_invalid_identifier_chars, '_'); - while (private_ids.includes(deconflicted)) { - deconflicted = '_' + deconflicted; - } - - private_ids.push(deconflicted); - backing_fields[name] = b.private_id(deconflicted); - } - /** @type {Array} */ const body = []; - const child_state = { ...context.state, state_fields, backing_fields }; + const child_state = { ...context.state, state_fields }; for (const name in state_fields) { if (name[0] === '#') { @@ -66,11 +33,10 @@ export function ClassBody(node, context) { field.node.type === 'AssignmentExpression' && (field.type === '$derived' || field.type === '$derived.by') ) { - const backing = backing_fields[name]; - const member = b.member(b.this, backing); + const member = b.member(b.this, field.key); body.push( - b.prop_def(backing, null), + b.prop_def(field.key, null), b.method('get', b.key(name), [], [b.return(b.call(member))]) ); } @@ -100,12 +66,11 @@ export function ClassBody(node, context) { continue; } - const backing = backing_fields[name]; - const member = b.member(b.this, backing); + const member = b.member(b.this, field.key); body.push( b.prop_def( - backing, + field.key, /** @type {CallExpression} */ ( context.visit(definition.value ?? field.value, child_state) ) diff --git a/packages/svelte/src/compiler/phases/3-transform/types.d.ts b/packages/svelte/src/compiler/phases/3-transform/types.d.ts index cb866abef1d9..21da2088763f 100644 --- a/packages/svelte/src/compiler/phases/3-transform/types.d.ts +++ b/packages/svelte/src/compiler/phases/3-transform/types.d.ts @@ -10,5 +10,4 @@ export interface TransformState { readonly scopes: Map; readonly state_fields: Record; - readonly backing_fields: Record; } diff --git a/packages/svelte/src/compiler/types/index.d.ts b/packages/svelte/src/compiler/types/index.d.ts index f91bceac8093..ef52552a2e06 100644 --- a/packages/svelte/src/compiler/types/index.d.ts +++ b/packages/svelte/src/compiler/types/index.d.ts @@ -3,7 +3,12 @@ import type { Binding } from '../phases/scope.js'; import type { AST, Namespace } from './template.js'; import type { ICompileDiagnostic } from '../utils/compile_diagnostic.js'; import type { StateCreationRuneName } from '../../utils.js'; -import type { AssignmentExpression, CallExpression, PropertyDefinition } from 'estree'; +import type { + AssignmentExpression, + CallExpression, + PrivateIdentifier, + PropertyDefinition +} from 'estree'; /** The return value of `compile` from `svelte/compiler` */ export interface CompileResult { @@ -274,6 +279,7 @@ export interface ExpressionMetadata { export interface StateField { type: StateCreationRuneName; node: PropertyDefinition | AssignmentExpression; + key: PrivateIdentifier; value: CallExpression; } From 45115dec620a2024cdf749242428eabdd7b2a97a Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 16 May 2025 18:04:03 -0400 Subject: [PATCH 48/64] unused --- .../compiler/phases/3-transform/client/visitors/ClassBody.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js index 56eae3a763fa..1929e84db406 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js @@ -1,8 +1,7 @@ -/** @import { CallExpression, ClassBody, MethodDefinition, PrivateIdentifier, PropertyDefinition, StaticBlock } from 'estree' */ +/** @import { CallExpression, ClassBody, MethodDefinition, PropertyDefinition, StaticBlock } from 'estree' */ /** @import { Context } from '../types' */ import * as b from '#compiler/builders'; import { get_name } from '../../../nodes.js'; -import { regex_invalid_identifier_chars } from '../../../patterns.js'; /** * @param {ClassBody} node From 56bd834c087bc737cc312ef5b1f21df82cd54eab Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 16 May 2025 18:06:09 -0400 Subject: [PATCH 49/64] tweak --- packages/svelte/src/utils.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/svelte/src/utils.js b/packages/svelte/src/utils.js index 654b4bb2d0c1..e2c09f3a9dd1 100644 --- a/packages/svelte/src/utils.js +++ b/packages/svelte/src/utils.js @@ -434,6 +434,7 @@ export const STATE_CREATION_RUNES = /** @type {const} */ ([ '$derived', '$derived.by' ]); + const RUNES = /** @type {const} */ ([ ...STATE_CREATION_RUNES, '$state.snapshot', From 855f209e8efd6e7fa3b1ac277139b938133d1c8c Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 16 May 2025 18:07:10 -0400 Subject: [PATCH 50/64] unused --- packages/svelte/src/compiler/phases/3-transform/types.d.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/types.d.ts b/packages/svelte/src/compiler/phases/3-transform/types.d.ts index 21da2088763f..d7695a60ea1d 100644 --- a/packages/svelte/src/compiler/phases/3-transform/types.d.ts +++ b/packages/svelte/src/compiler/phases/3-transform/types.d.ts @@ -1,7 +1,6 @@ import type { Scope } from '../scope.js'; import type { AST, StateField, ValidatedModuleCompileOptions } from '#compiler'; import type { Analysis } from '../types.js'; -import type { PrivateIdentifier } from 'estree'; export interface TransformState { readonly analysis: Analysis; From d31831be16b54447618be97adb18c535849902a1 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 16 May 2025 18:10:41 -0400 Subject: [PATCH 51/64] more --- .../phases/3-transform/client/visitors/ClassBody.js | 10 ++-------- .../phases/3-transform/server/visitors/ClassBody.js | 10 ++-------- 2 files changed, 4 insertions(+), 16 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js index 1929e84db406..12ed7ccea516 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js @@ -70,11 +70,7 @@ export function ClassBody(node, context) { if (name[0] === '#') { body.push(/** @type {PropertyDefinition} */ (context.visit(definition, child_state))); - } else { - if (field.node.type === 'AssignmentExpression') { - continue; - } - + } else if (field.node === definition) { const member = b.member(b.this, field.key); const should_proxy = field.type === '$state' && true; // TODO @@ -82,9 +78,7 @@ export function ClassBody(node, context) { body.push( b.prop_def( field.key, - /** @type {CallExpression} */ ( - context.visit(definition.value ?? field.value, child_state) - ) + /** @type {CallExpression} */ (context.visit(field.value, child_state)) ), b.method('get', definition.key, [], [b.return(b.call('$.get', member))]), diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/ClassBody.js index fb3cf89026c5..45eddc424c11 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/ClassBody.js @@ -61,19 +61,13 @@ export function ClassBody(node, context) { if (name[0] === '#' || field.type === '$state' || field.type === '$state.raw') { body.push(/** @type {PropertyDefinition} */ (context.visit(definition, child_state))); - } else { - if (field.node.type === 'AssignmentExpression') { - continue; - } - + } else if (field.node === definition) { const member = b.member(b.this, field.key); body.push( b.prop_def( field.key, - /** @type {CallExpression} */ ( - context.visit(definition.value ?? field.value, child_state) - ) + /** @type {CallExpression} */ (context.visit(field.value, child_state)) ), b.method('get', definition.key, [], [b.return(b.call(member))]) From 0cac3d2fddd779b8994c43ae8a6713dee91e6d1f Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 16 May 2025 18:14:47 -0400 Subject: [PATCH 52/64] tweak --- packages/svelte/src/utils.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/svelte/src/utils.js b/packages/svelte/src/utils.js index e2c09f3a9dd1..921eaec57cf5 100644 --- a/packages/svelte/src/utils.js +++ b/packages/svelte/src/utils.js @@ -458,7 +458,7 @@ const RUNES = /** @type {const} */ ([ * @returns {name is RuneName} */ export function is_rune(name) { - return RUNES.includes(/** @type {RUNES[number]} */ (name)); + return RUNES.includes(/** @type {RuneName} */ (name)); } /** @typedef {STATE_CREATION_RUNES[number]} StateCreationRuneName */ @@ -468,9 +468,7 @@ export function is_rune(name) { * @returns {name is StateCreationRuneName} */ export function is_state_creation_rune(name) { - return ( - name === '$state' || name === '$state.raw' || name === '$derived' || name === '$derived.by' - ); + return STATE_CREATION_RUNES.includes(/** @type {StateCreationRuneName} */ (name)); } /** List of elements that require raw contents and should not have SSR comments put in them */ From b1cc424350ae18b6d9c621116a7e5eeefde647a1 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 16 May 2025 18:19:37 -0400 Subject: [PATCH 53/64] tweak --- .../src/compiler/phases/2-analyze/visitors/ClassBody.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js index c2635e73f191..93820edfeb9c 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js @@ -76,11 +76,7 @@ export function ClassBody(node, context) { handle(child, child.key, child.value); } - if ( - child.type === 'MethodDefinition' && - child.key.type === 'Identifier' && - child.key.name === 'constructor' - ) { + if (child.type === 'MethodDefinition' && child.kind === 'constructor') { constructor = child; } } From bbf01914f990b4fdf6939bea20a1266a67bd3022 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 16 May 2025 18:27:00 -0400 Subject: [PATCH 54/64] fix proxying logic --- .../client/visitors/CallExpression.js | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/CallExpression.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/CallExpression.js index fdab52a49898..665be9e23bf4 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/CallExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/CallExpression.js @@ -4,6 +4,7 @@ import { dev, is_ignored } from '../../../../state.js'; import * as b from '#compiler/builders'; import { get_rune } from '../../../scope.js'; import { transform_inspect_rune } from '../../utils.js'; +import { should_proxy } from '../utils.js'; /** * @param {CallExpression} node @@ -19,15 +20,23 @@ export function CallExpression(node, context) { case '$effect.tracking': return b.call('$.effect_tracking'); - // TODO can we reuse this logic for normal declarations, i.e. fall through to this? + // transform state field assignments in constructors case '$state': case '$state.raw': { - let should_proxy = rune === '$state' && true; // TODO + let arg = node.arguments[0]; - let value = node.arguments[0] && /** @type {Expression} */ (context.visit(node.arguments[0])); + /** @type {Expression | undefined} */ + let value = undefined; - if (value && should_proxy) { - value = b.call('$.proxy', value); + if (arg) { + value = /** @type {Expression} */ (context.visit(node.arguments[0])); + + if ( + rune === '$state' && + should_proxy(/** @type {Expression} */ (arg), context.state.scope) + ) { + value = b.call('$.proxy', value); + } } return b.call('$.state', value); From 2b42182085fed912c0e37dc8e6a9f2293f3ed585 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 16 May 2025 18:29:53 -0400 Subject: [PATCH 55/64] tweak --- .../src/compiler/phases/2-analyze/visitors/ClassBody.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js index 93820edfeb9c..fa1c687a2079 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js @@ -112,8 +112,5 @@ export function ClassBody(node, context) { field.key = b.private_id(deconflicted); } - context.next({ - ...context.state, - state_fields - }); + context.next({ ...context.state, state_fields }); } From 63e60c76a3d87a0de0ba5087652409e2f5c43e38 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 16 May 2025 18:33:03 -0400 Subject: [PATCH 56/64] tweak --- .../client/visitors/AssignmentExpression.js | 12 +++++------- .../server/visitors/AssignmentExpression.js | 14 ++++++-------- 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js index 6885be810fe0..2e0d728a0391 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js @@ -68,13 +68,11 @@ function build_assignment(operator, left, right, context) { in_constructor: rune !== '$derived' && rune !== '$derived.by' }; - const field = context.state.state_fields[name]; - - const l = b.member(b.this, field.key); - - const r = /** @type {Expression} */ (context.visit(right, child_state)); - - return b.assignment(operator, l, r); + return b.assignment( + operator, + b.member(b.this, context.state.state_fields[name].key), + /** @type {Expression} */ (context.visit(right, child_state)) + ); } } diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/AssignmentExpression.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/AssignmentExpression.js index e80187d013f7..c10e757d7342 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/AssignmentExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/AssignmentExpression.js @@ -35,18 +35,16 @@ function build_assignment(operator, left, right, context) { const rune = get_rune(right, context.state.scope); if (rune) { - const field = context.state.state_fields[name]; - const key = left.property.type === 'PrivateIdentifier' || rune === '$state' || rune === '$state.raw' ? left.property - : field.key; - - const l = b.member(b.this, key, key.type === 'Literal'); - - const r = /** @type {Expression} */ (context.visit(right)); + : context.state.state_fields[name].key; - return b.assignment(operator, l, r); + return b.assignment( + operator, + b.member(b.this, key, key.type === 'Literal'), + /** @type {Expression} */ (context.visit(right)) + ); } } } From c82ede5cf6ea339ce822401bef044848b2e806ec Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 19 May 2025 09:26:57 -0400 Subject: [PATCH 57/64] adjust message to accommodate more cases --- .vscode/launch.json | 1 + .../98-reference/.generated/compile-errors.md | 57 +++++++++---------- .../svelte/messages/compile-errors/script.md | 53 ++++++++--------- packages/svelte/src/compiler/errors.js | 7 ++- .../phases/2-analyze/visitors/ClassBody.js | 2 +- .../class-state-constructor-1/errors.json | 4 +- .../class-state-constructor-2/errors.json | 4 +- .../class-state-constructor-3/errors.json | 4 +- .../class-state-constructor-5/errors.json | 4 +- .../class-state-constructor-6/errors.json | 4 +- .../class-state-constructor-8/errors.json | 4 +- .../class-state-constructor-9/errors.json | 4 +- 12 files changed, 70 insertions(+), 78 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 142965ada292..6d24f97c2a5a 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -6,6 +6,7 @@ "request": "launch", "name": "Run sandbox", "program": "${workspaceFolder}/playgrounds/sandbox/run.js", + "runtimeExecutable": "node", "env": { "NODE_OPTIONS": "--stack-trace-limit=10000" } diff --git a/documentation/docs/98-reference/.generated/compile-errors.md b/documentation/docs/98-reference/.generated/compile-errors.md index 281186168060..8fdbe87fb049 100644 --- a/documentation/docs/98-reference/.generated/compile-errors.md +++ b/documentation/docs/98-reference/.generated/compile-errors.md @@ -208,37 +208,6 @@ Cannot assign to %thing% Cannot bind to %thing% ``` -### constructor_state_reassignment - -``` -A state field declaration in a constructor must be the first assignment, and the only one that uses a rune -``` - -[State fields]($state#Classes) can be declared as normal class fields or inside the constructor, in which case the declaration must be the _first_ assignment. -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 ``` @@ -877,6 +846,32 @@ Cannot reassign or bind to snippet parameter This snippet is shadowing the prop `%prop%` with the same name ``` +### state_field_duplicate + +``` +`%name%` has already been declared on this class +``` + +An assignment to a class field that uses a `$state` or `$derived` rune is considered a _state field declaration_. The declaration can happen in the class body... + +```js +class Counter { + count = $state(0); +} +``` + +...or inside the constructor... + +```js +class Counter { + constructor() { + this.count = $state(0); + } +} +``` + +...but it can only happen once. + ### state_invalid_export ``` diff --git a/packages/svelte/messages/compile-errors/script.md b/packages/svelte/messages/compile-errors/script.md index 7831be3a42eb..cfd6a2ae00e4 100644 --- a/packages/svelte/messages/compile-errors/script.md +++ b/packages/svelte/messages/compile-errors/script.md @@ -10,35 +10,6 @@ > Cannot bind to %thing% -## constructor_state_reassignment - -> A state field declaration in a constructor must be the first assignment, and the only one that uses a rune - -[State fields]($state#Classes) can be declared as normal class fields or inside the constructor, in which case the declaration must be the _first_ assignment. -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 @@ -241,6 +212,30 @@ It's possible to export a snippet from a `