Skip to content

Commit

Permalink
feat: allow state and derived declarations inside class constructors
Browse files Browse the repository at this point in the history
closes #11116
closes #11339
  • Loading branch information
dummdidumm committed May 3, 2024
1 parent f2f71ae commit d45c8f0
Show file tree
Hide file tree
Showing 8 changed files with 211 additions and 54 deletions.
5 changes: 5 additions & 0 deletions .changeset/funny-icons-flash.md
@@ -0,0 +1,5 @@
---
"svelte": patch
---

feat: allow state and derived declarations inside class constructors
3 changes: 2 additions & 1 deletion packages/svelte/src/compiler/phases/2-analyze/validation.js
Expand Up @@ -821,7 +821,8 @@ function validate_call_expression(node, scope, path) {
) {
if (parent.type === 'VariableDeclarator') return;
if (parent.type === 'PropertyDefinition' && !parent.static && !parent.computed) return;
e.state_invalid_placement(node, rune);
// TODO
// e.state_invalid_placement(node, rune);
}

if (rune === '$effect' || rune === '$effect.pre') {
Expand Down
Expand Up @@ -3,7 +3,8 @@ import type {
Statement,
LabeledStatement,
Identifier,
PrivateIdentifier
PrivateIdentifier,
Literal
} from 'estree';
import type { Namespace, SvelteNode, ValidatedCompileOptions } from '#compiler';
import type { TransformState } from '../types.js';
Expand Down Expand Up @@ -66,6 +67,8 @@ export interface ComponentClientTransformState extends ClientTransformState {

export interface StateField {
kind: 'state' | 'frozen_state' | 'derived' | 'derived_call';
public_id: Identifier | Literal | null;
declared_in_constructor: boolean;
id: PrivateIdentifier;
}

Expand Down
26 changes: 26 additions & 0 deletions packages/svelte/src/compiler/phases/3-transform/client/utils.js
Expand Up @@ -11,6 +11,7 @@ import {
PROPS_IS_RUNES,
PROPS_IS_UPDATED
} from '../../../../constants.js';
import { get_rune } from '../../scope.js';

/**
* @template {import('./types').ClientTransformState} State
Expand Down Expand Up @@ -231,6 +232,31 @@ export function serialize_set_binding(node, context, fallback, options) {
state.in_constructor
) {
const public_state = context.state.public_state.get(assignee.property.name);
const rune = get_rune(node.right, context.state.scope);

if (
public_state &&
(rune === '$state' ||
rune === '$state.frozen' ||
rune === '$derived' ||
rune === '$derived.by')
) {
const args = /** @type {import('estree').CallExpression} */ (node.right).arguments;
let value =
args.length === 0
? b.id('undefined')
: /** @type {import('estree').Expression} */ (visit(args[0]));
if (rune === '$state' || rune === '$state.frozen') {
if (should_proxy_or_freeze(value, state.scope)) {
value = b.call(rune === '$state' ? '$.proxy' : '$.freeze', value);
}
value = b.call('$.source', value);
} else {
value = b.call('$.derived', rune === '$derived.by' ? value : b.thunk(value));
}
return b.assignment(node.operator, b.member(b.this, public_state.id), value);
}

const value = get_assignment_value(node, context);
// See if we should wrap value in $.proxy
if (
Expand Down
Expand Up @@ -12,18 +12,26 @@ export const global_visitors = {
return serialize_get_binding(node, state);
}
},
MemberExpression(node, { state, next }) {
MemberExpression(node, { state, next, path }) {
if (node.object.type === 'ThisExpression') {
// rewrite `this.#foo` as `this.#foo.v` inside a constructor
const parent = path.at(-1);
const is_constructor_assignment =
parent?.type === 'AssignmentExpression' && parent.left === node;

// rewrite `this.#foo = ...` as `this.#foo.v = ...` inside a constructor
if (node.property.type === 'PrivateIdentifier') {
const field = state.private_state.get(node.property.name);
if (field) {
return state.in_constructor ? b.member(node, b.id('v')) : b.call('$.get', node);
if (state.in_constructor && is_constructor_assignment) {
return b.member(node, b.id('v'));
} else {
return b.call('$.get', node);
}
}
}

// rewrite `this.foo` as `this.#foo.v` inside a constructor
if (node.property.type === 'Identifier' && !node.computed) {
// rewrite `this.foo = ...` as `this.#foo.v = ...` inside a constructor
if (node.property.type === 'Identifier' && !node.computed && is_constructor_assignment) {
const field = state.public_state.get(node.property.name);

if (field && state.in_constructor) {
Expand Down
Expand Up @@ -50,6 +50,8 @@ export const javascript_visitors_runes = {
: rune === '$derived.by'
? 'derived_call'
: 'derived',
public_id: definition.key.type === 'PrivateIdentifier' ? null : definition.key,
declared_in_constructor: false,
// @ts-expect-error this is set in the next pass
id: is_private ? definition.key : null
};
Expand All @@ -61,6 +63,61 @@ export const javascript_visitors_runes = {
}
}
}
} else if (definition.type === 'MethodDefinition' && definition.kind === 'constructor') {
for (const entry of definition.value.body.body) {
if (
entry.type === 'ExpressionStatement' &&
entry.expression.type === 'AssignmentExpression'
) {
let { left, right } = entry.expression;
if (
left.type !== 'MemberExpression' ||
left.object.type !== 'ThisExpression' ||
(left.property.type !== 'Identifier' && left.property.type !== 'PrivateIdentifier')
) {
continue;
}

const id = left.property;
const name = get_name(id);
if (!name) continue;

const is_private = id.type === 'PrivateIdentifier';
if (is_private) private_ids.push(name);

if (right.type === 'CallExpression') {
const rune = get_rune(right, state.scope);
if (
rune === '$state' ||
rune === '$state.frozen' ||
rune === '$derived' ||
rune === '$derived.by'
) {
/** @type {import('../types.js').StateField} */
const field = {
kind:
rune === '$state'
? 'state'
: rune === '$state.frozen'
? 'frozen_state'
: rune === '$derived.by'
? 'derived_call'
: 'derived',
public_id: id.type === 'PrivateIdentifier' ? null : id,
declared_in_constructor: true,
// @ts-expect-error this is set in the next pass
id: is_private ? id : null
};

if (is_private) {
private_state.set(name, field);
} else {
public_state.set(name, field);
}
}
}
}
}
}
}

Expand All @@ -78,6 +135,53 @@ export const javascript_visitors_runes = {
/** @type {Array<import('estree').MethodDefinition | import('estree').PropertyDefinition>} */
const body = [];

// create getters and setters for public fields
for (const [name, field] of public_state) {
const public_id = /** @type {import('estree').Identifier | import('estree').Literal} */ (
field.public_id
);
const member = b.member(b.this, field.id);

if (field.declared_in_constructor) {
// #foo;
body.push(b.prop_def(field.id, undefined));
}

// get foo() { return this.#foo; }
body.push(b.method('get', public_id, [], [b.return(b.call('$.get', member))]));

if (field.kind === 'state' || field.kind === 'frozen_state') {
// set foo(value) { this.#foo = value; }
const value = b.id('value');
body.push(
b.method(
'set',
public_id,
[value],
[
b.stmt(
b.call(
'$.set',
member,
b.call(field.kind === 'state' ? '$.proxy' : '$.freeze', value)
)
)
]
)
);
} else if (state.options.dev) {
body.push(
b.method(
'set',
public_id,
[b.id('_')],
[b.throw_error(`Cannot update a derived property ('${name}')`)]
)
);
}
}

/** @type {import('../types.js').ComponentClientTransformState} */
const child_state = { ...state, public_state, private_state };

// Replace parts of the class body
Expand Down Expand Up @@ -121,53 +225,8 @@ export const javascript_visitors_runes = {
value = b.call('$.source');
}

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))]));

if (field.kind === 'state') {
// set foo(value) { this.#foo = value; }
const value = b.id('value');
body.push(
b.method(
'set',
definition.key,
[value],
[b.stmt(b.call('$.set', member, b.call('$.proxy', value)))]
)
);
}

if (field.kind === 'frozen_state') {
// set foo(value) { this.#foo = value; }
const value = b.id('value');
body.push(
b.method(
'set',
definition.key,
[value],
[b.stmt(b.call('$.set', member, b.call('$.freeze', value)))]
)
);
}

if ((field.kind === 'derived' || field.kind === 'derived_call') && state.options.dev) {
body.push(
b.method(
'set',
definition.key,
[b.id('_')],
[b.throw_error(`Cannot update a derived property ('${name}')`)]
)
);
}
}
// #foo = $state/$derived();
body.push(b.prop_def(field.id, value));
continue;
}
}
Expand Down
@@ -0,0 +1,31 @@
import { test } from '../../test';

export default test({
solo: true,
html: `
<button>0</button>
<button>0</button>
`,

async test({ assert, target }) {
const [btn1, btn2] = target.querySelectorAll('button');

await btn1?.click();
assert.htmlEqual(
target.innerHTML,
`
<button>1</button>
<button>0</button>
`
);

await btn2?.click();
assert.htmlEqual(
target.innerHTML,
`
<button>2</button>
<button>1</button>
`
);
}
});
@@ -0,0 +1,24 @@
<script>
class Counter {
#count;
get secretCount() {
return this.#count;
}
constructor() {
this.#count = $state(0); // TODO
this.count = $state(0);
this.double = $derived(this.count * 2);
}
increment() {
this.#count++;
this.count++;
}
}
const counter = new Counter();
</script>

<button on:click={() => counter.count++}>{counter.count}</button>
<button on:click={counter.increment}>{counter.secretCount}</button>

0 comments on commit d45c8f0

Please sign in to comment.