diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index 63cfc74064..511bc50646 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 d69ef71db1..a64c89cd88 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 2d9b4018a6..233223de69 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 ea072d168d..0000000000 --- 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(); -}