final cleanup??

pull/15820/head
S. Elliott Johnson 5 months ago
parent 92940ffbfd
commit ac42ad5580

@ -7,6 +7,6 @@
* @param {Context} context * @param {Context} context
*/ */
export function PropertyDefinition(node, context) { export function PropertyDefinition(node, context) {
context.state.class_state?.register?.(node, context); context.state.class_state?.register(node, context);
context.next(); context.next();
} }

@ -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 { AST } from '#compiler' */
/** @import { Context } from '../../types' */ /** @import { Context } from '../../types' */
/** @import { StateCreationRuneName } from '../../../../../utils.js' */ /** @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']); const reassignable_assignments = new Set(['$state', '$state.raw', 'regular']);
export class ClassAnalysis { export class ClassAnalysis {
// TODO: Probably need to include property definitions here too
/** @type {Map<string, PropertyAssignmentDetails>} */ /** @type {Map<string, PropertyAssignmentDetails>} */
property_assignments = new Map(); #public_assignments = new Map();
/** @type {Map<string, PropertyAssignmentDetails>} */
#private_assignments = new Map();
/** /**
* Determines if the node is a valid assignment to a class property, and if so, * Determines if the node is a valid assignment to a class property, and if so,
@ -30,30 +32,46 @@ export class ClassAnalysis {
let name; let name;
/** @type {PropertyAssignmentType} */ /** @type {PropertyAssignmentType} */
let type; let type;
/** @type {boolean} */
let is_private;
if (node.type === 'AssignmentExpression') { if (node.type === 'AssignmentExpression') {
if (!this.is_class_property_assignment_at_constructor_root(node, context.path)) { if (!this.is_class_property_assignment_at_constructor_root(node, context.path)) {
return; 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); 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 { } else {
if (!this.#is_assigned_property(node)) { if (!this.#is_assigned_property(node)) {
return; 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); 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 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 // we don't have to validate anything other than conflicts here, because the rune placement rules
// catch all of the other weirdness. // catch all of the other weirdness.
if (!this.property_assignments.has(name)) { const map = is_private ? this.#private_assignments : this.#public_assignments;
this.property_assignments.set(name, { type, node }); if (!map.has(name)) {
map.set(name, { type, node });
} }
} }
@ -61,7 +79,7 @@ export class ClassAnalysis {
* @template {AST.SvelteNode} T * @template {AST.SvelteNode} T
* @param {AST.SvelteNode} node * @param {AST.SvelteNode} node
* @param {T[]} path * @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) { is_class_property_assignment_at_constructor_root(node, path) {
if ( if (
@ -71,7 +89,8 @@ export class ClassAnalysis {
node.left.type === 'MemberExpression' && node.left.type === 'MemberExpression' &&
node.left.object.type === 'ThisExpression' && node.left.object.type === 'ThisExpression' &&
(node.left.property.type === 'Identifier' || (node.left.property.type === 'Identifier' ||
node.left.property.type === 'PrivateIdentifier') node.left.property.type === 'PrivateIdentifier' ||
node.left.property.type === 'Literal')
) )
) { ) {
return false; return false;
@ -89,11 +108,13 @@ export class ClassAnalysis {
* We only care about properties that have values assigned to them -- if they don't, * 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. * they can't be a conflict for state declared in the constructor.
* @param {PropertyDefinition} node * @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) { #is_assigned_property(node) {
return ( return (
(node.key.type === 'PrivateIdentifier' || node.key.type === 'Identifier') && (node.key.type === 'PrivateIdentifier' ||
node.key.type === 'Identifier' ||
node.key.type === 'Literal') &&
Boolean(node.value) && Boolean(node.value) &&
!node.static && !node.static &&
!node.computed !node.computed
@ -107,9 +128,10 @@ export class ClassAnalysis {
* @param {AssignmentExpression} node * @param {AssignmentExpression} node
* @param {string} name * @param {string} name
* @param {PropertyAssignmentType} type * @param {PropertyAssignmentType} type
* @param {boolean} is_private
*/ */
#check_for_conflicts(node, name, type) { #check_for_conflicts(node, name, type, is_private) {
const existing = this.property_assignments.get(name); const existing = (is_private ? this.#private_assignments : this.#public_assignments).get(name);
if (!existing) { if (!existing) {
return; return;
} }
@ -140,3 +162,11 @@ export class ClassAnalysis {
return 'regular'; return 'regular';
} }
} }
/**
*
* @param {PrivateIdentifier | Identifier | Literal} node
*/
function get_name_for_identifier(node) {
return node.type === 'Literal' ? node.value?.toString() : node.name;
}

@ -17,7 +17,7 @@ import { validate_mutation } from './shared/utils.js';
* @param {Context} context * @param {Context} context
*/ */
export function AssignmentExpression(node, 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) { if (stripped_node) {
return stripped_node; return stripped_node;
} }

@ -56,7 +56,10 @@ export function create_client_class_analysis(body) {
left: { left: {
...node.left, ...node.left,
// ...swap out the assignment to go directly against the private field // ...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 // ...and swap out the assignment's value for the state field init
right: build_init_value(field.kind, node.right.arguments[0], context) 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 {StateCreationRuneName} kind
* @param {Expression | SpreadElement} arg * @param {Expression | SpreadElement} arg
* @param {Context} context * @param {Context} context
*/ */
function build_init_value(kind, arg, context) { function build_init_value(kind, arg, context) {
const init = /** @type {Expression} **/ ( const init = arg
context.visit(arg, { ...context.state, in_constructor: false }) ? /** @type {Expression} **/ (context.visit(arg, { ...context.state, in_constructor: false }))
); : b.void0;
switch (kind) { switch (kind) {
case '$state': case '$state':

@ -10,7 +10,7 @@ import { visit_assignment_expression } from '../../shared/assignments.js';
* @param {Context} context * @param {Context} context
*/ */
export function AssignmentExpression(node, 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) { if (stripped_node) {
return stripped_node; return stripped_node;
} }

@ -23,11 +23,9 @@ export function create_server_class_analysis(body) {
// value can be assigned in the constructor. // value can be assigned in the constructor.
value = null; value = null;
} else if (field.kind !== '$derived' && field.kind !== '$derived.by') { } else if (field.kind !== '$derived' && field.kind !== '$derived.by') {
return [/** @type {PropertyDefinition} */ (context.visit(node, context.state))]; return [/** @type {PropertyDefinition} */ (context.visit(node))];
} else { } else {
const init = /** @type {Expression} **/ ( const init = /** @type {Expression} **/ (context.visit(node.value.arguments[0]));
context.visit(node.value.arguments[0], context.state)
);
value = value =
field.kind === '$derived.by' ? b.call('$.once', init) : b.call('$.once', b.thunk(init)); 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: { left: {
...node.left, ...node.left,
// ...swap out the assignment to go directly against the private field // ...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 // ...and swap out the assignment's value for the state field init
right: build_init_value(field.kind, node.right.arguments[0], context) 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 * @param {Context} context
*/ */
function build_init_value(kind, arg, 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) { switch (kind) {
case '$state': case '$state':

@ -19,13 +19,25 @@ import { get_rune } from '../../scope.js';
* @returns {ClassAnalysis<TContext>} * @returns {ClassAnalysis<TContext>}
*/ */
export function create_class_analysis(body, build_state_field, build_assignment) { export function create_class_analysis(body, build_state_field, build_assignment) {
/** @type {Map<string, StateField>} */ /**
* Public, stateful fields.
* @type {Map<string, StateField>}
*/
const public_fields = new Map(); const public_fields = new Map();
/** @type {Map<string, StateField>} */ /**
* 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<string, StateField>}
*/
const private_fields = new Map(); const private_fields = new Map();
/** @type {Array<PropertyDefinition | MethodDefinition>} */ /**
* Accumulates nodes for the new class body.
* @type {Array<PropertyDefinition | MethodDefinition>}
*/
const new_body = []; const new_body = [];
/** /**
@ -57,6 +69,8 @@ export function create_class_analysis(body, build_state_field, build_assignment)
const replacers = []; const replacers = [];
/** /**
* Get a state field by name.
*
* @param {string} name * @param {string} name
* @param {boolean} is_private * @param {boolean} is_private
* @param {ReadonlyArray<StateCreationRuneName>} [kinds] * @param {ReadonlyArray<StateCreationRuneName>} [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 * @param {TContext} context
* @returns {TContext} * @returns {TContext}
*/ */
function create_child_context(context) { 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 { return {
...context, ...context,
state: { state,
...context.state, visit,
class_analysis 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 * @param {TContext} context
*/ */
function generate_body(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 {AssignmentExpression} node
* @param {TContext} context * @param {TContext} context
* @returns {AssignmentExpression | null} The node, if `register_assignment` handled its transformation. * @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); const child_context = create_child_context(context);
if ( if (
!( !(
@ -119,7 +148,8 @@ export function create_class_analysis(body, build_state_field, build_assignment)
node.left.type === 'MemberExpression' && node.left.type === 'MemberExpression' &&
node.left.object.type === 'ThisExpression' && node.left.object.type === 'ThisExpression' &&
(node.left.property.type === 'Identifier' || (node.left.property.type === 'Identifier' ||
node.left.property.type === 'PrivateIdentifier') node.left.property.type === 'PrivateIdentifier' ||
node.left.property.type === 'Literal')
) )
) { ) {
return null; 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 {PropertyDefinition | MethodDefinition | StaticBlock} node
* @param {TContext} child_context * @param {TContext} child_context
* @returns {boolean} if this node is stateful and was registered * @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 = { const class_analysis = {
get_field, get_field,
generate_body, generate_body,
register_assignment generate_assignment
}; };
return class_analysis; return class_analysis;

@ -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 { StateField } from '../types';
import type { Context as ServerContext } from '../server/types'; import type { Context as ServerContext } from '../server/types';
import type { Context as ClientContext } from '../client/types'; import type { Context as ClientContext } from '../client/types';
@ -7,13 +17,13 @@ import type { StateCreationRuneName } from '../../../../utils';
export type StatefulAssignment = AssignmentExpression & { export type StatefulAssignment = AssignmentExpression & {
left: MemberExpression & { left: MemberExpression & {
object: ThisExpression; object: ThisExpression;
property: Identifier | PrivateIdentifier property: Identifier | PrivateIdentifier | Literal;
}; };
right: CallExpression; right: CallExpression;
}; };
export type StatefulPropertyDefinition = PropertyDefinition & { export type StatefulPropertyDefinition = PropertyDefinition & {
key: Identifier | PrivateIdentifier; key: Identifier | PrivateIdentifier | Literal;
value: CallExpression; value: CallExpression;
}; };
@ -34,7 +44,9 @@ export type AssignmentBuilderParams<TContext extends ServerContext | ClientConte
context: TContext; context: TContext;
}; };
export type AssignmentBuilder<TContext extends ServerContext | ClientContext> = (params: AssignmentBuilderParams<TContext>) => AssignmentExpression; export type AssignmentBuilder<TContext extends ServerContext | ClientContext> = (
params: AssignmentBuilderParams<TContext>
) => AssignmentExpression;
export type ClassAnalysis<TContext extends ServerContext | ClientContext> = { export type ClassAnalysis<TContext extends ServerContext | ClientContext> = {
/** /**
@ -43,7 +55,11 @@ export type ClassAnalysis<TContext extends ServerContext | ClientContext> = {
* @param kinds - What kinds of state creation runes you're looking for, eg. only '$derived.by'. * @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. * @returns The field if it exists and matches the given criteria, or null.
*/ */
get_field: (name: string, is_private: boolean, kinds?: Array<StateCreationRuneName>) => StateField | undefined; get_field: (
name: string,
is_private: boolean,
kinds?: Array<StateCreationRuneName>
) => StateField | undefined;
/** /**
* Given the body of a class, generate a new body with stateful fields. * Given the body of a class, generate a new body with stateful fields.
@ -59,5 +75,8 @@ export type ClassAnalysis<TContext extends ServerContext | ClientContext> = {
* a state field on the class. If it is, it registers that state field and modifies the * a state field on the class. If it is, it registers that state field and modifies the
* assignment expression. * assignment expression.
*/ */
register_assignment: (node: AssignmentExpression, context: TContext) => AssignmentExpression | null; generate_assignment: (
} node: AssignmentExpression,
context: TContext
) => AssignmentExpression | null;
};

@ -14,4 +14,4 @@ export interface TransformState {
export interface StateField { export interface StateField {
kind: StateCreationRuneName; kind: StateCreationRuneName;
id: PrivateIdentifier; id: PrivateIdentifier;
} }

@ -1,4 +1,5 @@
/** @import { AST, ExpressionMetadata } from '#compiler' */ /** @import { AST, ExpressionMetadata } from '#compiler' */
/** /**
* All nodes that can appear elsewhere than the top level, have attributes and can contain children * All nodes that can appear elsewhere than the top level, have attributes and can contain children
*/ */

@ -0,0 +1,9 @@
<script>
class Test {
0 = $state();
constructor() {
this[1] = $state();
}
}
</script>

@ -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
}
}
]

@ -0,0 +1,7 @@
export class Counter {
// prettier-ignore
'count' = $state(0);
constructor() {
this['count'] = $state(0);
}
}

@ -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
}
}
]

@ -0,0 +1,6 @@
export class Counter {
count = $state(0);
constructor() {
this['count'] = $state(0);
}
}
Loading…
Cancel
Save