diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index 51180ca7cc..fded183b86 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -48,6 +48,7 @@ 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'; @@ -164,6 +165,7 @@ const visitors = { MemberExpression, NewExpression, OnDirective, + PropertyDefinition, RegularElement, RenderTag, SlotElement, @@ -266,7 +268,7 @@ export function analyze_module(ast, options) { scope, scopes, analysis: /** @type {ComponentAnalysis} */ (analysis), - state_fields: null, + state_fields: new Map(), // 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 +628,7 @@ export function analyze_component(root, source, options) { has_props_rune: false, component_slots: new Set(), expression: null, - state_fields: null, + state_fields: new Map(), function_depth: scope.function_depth, reactive_statement: null }; @@ -693,7 +695,7 @@ export function analyze_component(root, source, options) { reactive_statement: null, component_slots: new Set(), expression: null, - state_fields: null, + state_fields: new Map(), 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 7ccd175ff3..080239bac0 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/types.d.ts +++ b/packages/svelte/src/compiler/phases/2-analyze/types.d.ts @@ -20,7 +20,7 @@ export interface AnalysisState { expression: ExpressionMetadata | null; /** Used to analyze class state */ - state_fields: Record | null; + state_fields: Map; 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 224e4445ce..ffc39ac00d 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js @@ -30,14 +30,11 @@ export function ClassBody(node, context) { } } - /** @type {Record} */ - const state_fields = {}; + /** @type {Map} */ + const state_fields = new Map(); context.state.analysis.classes.set(node, state_fields); - /** @type {string[]} */ - const seen = []; - /** @type {MethodDefinition | null} */ let constructor = null; @@ -53,24 +50,22 @@ export function ClassBody(node, context) { const rune = get_rune(value, context.state.scope); if (rune && is_state_creation_rune(rune)) { - if (seen.includes(name)) { + if (state_fields.has(name)) { e.state_field_duplicate(node, name); } - state_fields[name] = { + state_fields.set(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) - }; - - seen.push(name); + }); } } for (const child of node.body) { - if (child.type === 'PropertyDefinition' && !child.computed) { + if (child.type === 'PropertyDefinition' && !child.computed && !child.static) { handle(child, child.key, child.value); } @@ -94,13 +89,11 @@ export function ClassBody(node, context) { } } - for (const name in state_fields) { + for (const [name, field] of 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; diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/PropertyDefinition.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/PropertyDefinition.js new file mode 100644 index 0000000000..99d05cb47c --- /dev/null +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/PropertyDefinition.js @@ -0,0 +1,21 @@ +/** @import { PropertyDefinition } from 'estree' */ +/** @import { Context } from '../types' */ +import * as e from '../../../errors.js'; +import { get_name } from '../../nodes.js'; + +/** + * @param {PropertyDefinition} node + * @param {Context} context + */ +export function PropertyDefinition(node, context) { + const name = get_name(node.key); + const field = name && context.state.state_fields.get(name); + + if (field && node !== field.node && node.value) { + if (/** @type {number} */ (node.start) < /** @type {number} */ (field.node.start)) { + e.state_field_invalid_assignment(node); + } + } + + context.next(); +} diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/utils.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/utils.js index 89921832ae..10468595a4 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/utils.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/utils.js @@ -42,11 +42,7 @@ export function validate_assignment(node, argument, context) { ? null : get_name(argument.property); - const field = - name !== null && - context.state.state_fields && - Object.hasOwn(context.state.state_fields, name) && - context.state.state_fields[name]; + const field = name !== null && context.state.state_fields?.get(name); // check we're not assigning to a state field before its declaration in the constructor if (field && field.node.type === 'AssignmentExpression' && node !== field.node) { 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 711c1e64d8..f2eda3a7d2 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, - state_fields: {}, + state_fields: new Map(), transform: {}, in_constructor: false, instance_level_snippets: [], @@ -670,7 +670,7 @@ export function client_module(analysis, options) { options, scope: analysis.module.scope, scopes: analysis.module.scopes, - state_fields: {}, + state_fields: new Map(), 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 2e0d728a03..a46b318601 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 @@ -54,12 +54,11 @@ const callees = { function build_assignment(operator, left, right, context) { if (context.state.analysis.runes && left.type === 'MemberExpression') { const name = get_name(left.property); + const field = name && context.state.state_fields.get(name); - if (name !== null) { + if (field) { // special case — state declaration in class constructor - const ancestor = context.path.at(-4); - - if (ancestor?.type === 'MethodDefinition' && ancestor.kind === 'constructor') { + if (field.node.type === 'AssignmentExpression' && left === field.node.left) { const rune = get_rune(right, context.state.scope); if (rune) { @@ -70,7 +69,7 @@ function build_assignment(operator, left, right, context) { return b.assignment( operator, - b.member(b.this, context.state.state_fields[name].key), + b.member(b.this, field.key), /** @type {Expression} */ (context.visit(right, child_state)) ); } @@ -78,20 +77,16 @@ function build_assignment(operator, left, right, context) { // 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)) - ); + 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/client/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js index 12ed7ccea5..71a8fc3d05 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,4 +1,5 @@ /** @import { CallExpression, ClassBody, MethodDefinition, PropertyDefinition, StaticBlock } from 'estree' */ +/** @import { StateField } from '#compiler' */ /** @import { Context } from '../types' */ import * as b from '#compiler/builders'; import { get_name } from '../../../nodes.js'; @@ -21,13 +22,11 @@ export function ClassBody(node, context) { const child_state = { ...context.state, state_fields }; - for (const name in state_fields) { + for (const [name, field] of 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 member = b.member(b.this, field.key); @@ -61,13 +60,13 @@ export function ClassBody(node, context) { } const name = get_name(definition.key); - if (name === null || !Object.hasOwn(state_fields, name)) { + const field = name && /** @type {StateField} */ (state_fields.get(name)); + + if (!field) { 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 if (field.node === definition) { 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 ded727dc0f..7e89b0e705 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.state_fields['#' + node.property.name]; + const field = context.state.state_fields.get('#' + node.property.name); if (field) { return context.state.in_constructor && 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 c3c9db6270..66952eb298 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' && - Object.hasOwn(context.state.state_fields, '#' + argument.property.name) + context.state.state_fields.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 2c677116dc..3498b2636b 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 @@ -97,7 +97,7 @@ export function server_component(analysis, options) { template: /** @type {any} */ (null), namespace: options.namespace, preserve_whitespace: options.preserveWhitespace, - state_fields: {}, + state_fields: new Map(), skip_hydration_boundaries: false }; @@ -393,7 +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: {} + state_fields: new Map() }; 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 c10e757d73..280c16dbd2 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 @@ -26,26 +26,23 @@ export function AssignmentExpression(node, context) { function build_assignment(operator, left, right, context) { if (context.state.analysis.runes && left.type === 'MemberExpression') { const name = get_name(left.property); + const field = name && context.state.state_fields.get(name); - if (name !== null) { - // special case — state declaration in class constructor - const ancestor = context.path.at(-4); + // special case — state declaration in class constructor + if (field && field.node.type === 'AssignmentExpression' && left === field.node.left) { + const rune = get_rune(right, context.state.scope); - if (ancestor?.type === 'MethodDefinition' && ancestor.kind === 'constructor') { - const rune = get_rune(right, context.state.scope); + if (rune) { + const key = + left.property.type === 'PrivateIdentifier' || rune === '$state' || rune === '$state.raw' + ? left.property + : field.key; - if (rune) { - const key = - left.property.type === 'PrivateIdentifier' || rune === '$state' || rune === '$state.raw' - ? left.property - : context.state.state_fields[name].key; - - return b.assignment( - operator, - b.member(b.this, key, key.type === 'Literal'), - /** @type {Expression} */ (context.visit(right)) - ); - } + return b.assignment( + operator, + b.member(b.this, key, key.type === 'Literal'), + /** @type {Expression} */ (context.visit(right)) + ); } } } 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 45eddc424c..6797b0beff 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 @@ -21,15 +21,14 @@ export function ClassBody(node, context) { const child_state = { ...context.state, state_fields }; - for (const name in state_fields) { + for (const [name, field] of state_fields) { if (name[0] === '#') { continue; } - const field = state_fields[name]; - // insert backing fields for stuff declared in the constructor if ( + field && field.node.type === 'AssignmentExpression' && (field.type === '$derived' || field.type === '$derived.by') ) { @@ -52,13 +51,13 @@ export function ClassBody(node, context) { } const name = get_name(definition.key); - if (name === null || !Object.hasOwn(state_fields, name)) { + const field = name && state_fields.get(name); + + if (!field) { 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 if (field.node === definition) { 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 d7695a60ea..887ad447b9 100644 --- a/packages/svelte/src/compiler/phases/3-transform/types.d.ts +++ b/packages/svelte/src/compiler/phases/3-transform/types.d.ts @@ -8,5 +8,5 @@ export interface TransformState { readonly scope: Scope; readonly scopes: Map; - readonly state_fields: Record; + readonly state_fields: Map; } diff --git a/packages/svelte/src/compiler/phases/types.d.ts b/packages/svelte/src/compiler/phases/types.d.ts index 6e3aa9e026..67cbd75ff8 100644 --- a/packages/svelte/src/compiler/phases/types.d.ts +++ b/packages/svelte/src/compiler/phases/types.d.ts @@ -38,7 +38,7 @@ export interface Analysis { immutable: boolean; tracing: boolean; - classes: Map>; + classes: Map>; // TODO figure out if we can move this to ComponentAnalysis accessors: boolean; 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 0000000000..b7dd4c8ed4 --- /dev/null +++ b/packages/svelte/tests/validator/samples/class-state-constructor-9/errors.json @@ -0,0 +1,14 @@ +[ + { + "code": "state_field_invalid_assignment", + "message": "Cannot assign to a state field before its declaration", + "start": { + "line": 2, + "column": 1 + }, + "end": { + "line": 2, + "column": 12 + } + } +] 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 0000000000..a8469e13af --- /dev/null +++ b/packages/svelte/tests/validator/samples/class-state-constructor-9/input.svelte.js @@ -0,0 +1,7 @@ +export class Counter { + count = -1; + + constructor() { + this.count = $state(0); + } +}