Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow for literal property definition with state on classes #11326

Merged
merged 8 commits into from
Apr 29, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/honest-nails-share.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"svelte": minor
---

feat: allow for literal property definition with state on classes
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,44 @@ export const javascript_visitors_runes = {
/** @type {string[]} */
const private_ids = [];

/**
*
* @param {import("estree").PropertyDefinition} definition
* @param {boolean} is_private
* @param {string} name
*/
function create_state_field(definition, is_private, name) {
if (definition.value?.type === 'CallExpression') {
const rune = get_rune(definition.value, 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',
// @ts-expect-error this is set in the next pass
id: is_private ? definition.key : null
};

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

for (const definition of node.body) {
if (
definition.type === 'PropertyDefinition' &&
Expand All @@ -27,35 +65,10 @@ export const javascript_visitors_runes = {
const is_private = type === 'PrivateIdentifier';
if (is_private) private_ids.push(name);

if (definition.value?.type === 'CallExpression') {
const rune = get_rune(definition.value, 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',
// @ts-expect-error this is set in the next pass
id: is_private ? definition.key : null
};

if (is_private) {
private_state.set(name, field);
} else {
public_state.set(name, field);
}
}
}
create_state_field(definition, is_private, name);
} else if (definition.type === 'PropertyDefinition' && definition.key.type === 'Literal') {
const name = definition.key.value?.toString().replace('-', '_');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't think this is enough - there could be all sorts of whacky characters in there, better to use a "non-valid ID identifier" regex (I believe there exists one already)

Copy link
Member Author

@paoloricciuti paoloricciuti Apr 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh great, yesterday I started to think of other invalid characters but nothing come to mind...I'll update soon

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also just realized I need replaceAll

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dummdidumm fixed it in the new commit...should i add more tests for all the same thing that were tested with normal identifiers?

if (name) create_state_field(definition, false, name);
dummdidumm marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand All @@ -75,6 +88,95 @@ export const javascript_visitors_runes = {

const child_state = { ...state, public_state, private_state };

/**
*
* @param {import("estree").PropertyDefinition} definition
* @param {boolean} is_private
* @param {string} name
*/
function replace_class_body(definition, is_private, name) {
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 {import('estree').Expression} **/ (
visit(definition.value.arguments[0], child_state)
);

value =
field.kind === 'state'
? b.call(
'$.source',
should_proxy_or_freeze(init, state.scope) ? b.call('$.proxy', init) : init
)
: field.kind === 'frozen_state'
? b.call(
'$.source',
should_proxy_or_freeze(init, state.scope) ? b.call('$.freeze', init) : init
)
: field.kind === 'derived_call'
? 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('$.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}')`)]
)
);
}
}

return true;
}
return false;
}

// Replace parts of the class body
for (const definition of node.body) {
if (
Expand All @@ -84,83 +186,14 @@ export const javascript_visitors_runes = {
const name = definition.key.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 {import('estree').Expression} **/ (
visit(definition.value.arguments[0], child_state)
);

value =
field.kind === 'state'
? b.call(
'$.source',
should_proxy_or_freeze(init, state.scope) ? b.call('$.proxy', init) : init
)
: field.kind === 'frozen_state'
? b.call(
'$.source',
should_proxy_or_freeze(init, state.scope) ? b.call('$.freeze', init) : init
)
: field.kind === 'derived_call'
? 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('$.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}')`)]
)
);
}
}
if (replace_class_body(definition, is_private, name)) {
continue;
}
} else if (definition.type === 'PropertyDefinition' && definition.key.type === 'Literal') {
const name = definition.key.value?.toString().replace('-', '_');

if (name && replace_class_body(definition, false, name)) {
continue;
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { test } from '../../test';

export default test({
html: `<button>false</button>`,

async test({ assert, target }) {
const btn = target.querySelector('button');

await btn?.click();
assert.htmlEqual(target.innerHTML, `<button>true</button>`);

await btn?.click();
assert.htmlEqual(target.innerHTML, `<button>false</button>`);
}
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<script>
class Toggle {
"aria-pressed" = $state(false);

toggle(){
this["aria-pressed"] = !this["aria-pressed"]
}
}
const toggle = new Toggle();
</script>

<button on:click={() => toggle.toggle()}>{toggle["aria-pressed"]}</button>
Loading