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: add $state.frozen rune #9851

Merged
merged 19 commits into from
Dec 27, 2023
Merged
Show file tree
Hide file tree
Changes from 14 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/dry-clocks-grow.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

feat: add $state.frozen rune
Rich-Harris marked this conversation as resolved.
Show resolved Hide resolved
33 changes: 20 additions & 13 deletions packages/svelte/src/compiler/phases/2-analyze/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -585,6 +585,7 @@ const legacy_scope_tweaker = {
);
if (
binding.kind === 'state' ||
binding.kind === 'frozen_state' ||
(binding.kind === 'normal' && binding.declaration_kind === 'let')
) {
binding.kind = 'prop';
Expand Down Expand Up @@ -636,18 +637,19 @@ const legacy_scope_tweaker = {
const runes_scope_js_tweaker = {
VariableDeclarator(node, { state }) {
if (node.init?.type !== 'CallExpression') return;
if (get_rune(node.init, state.scope) === null) return;
const rune = get_rune(node.init, state.scope);
if (rune === null) return;

const callee = node.init.callee;
if (callee.type !== 'Identifier') return;
if (callee.type !== 'Identifier' && callee.type !== 'MemberExpression') return;

const name = callee.name;
if (name !== '$state' && name !== '$derived') return;
if (rune !== '$state' && rune !== '$state.frozen' && rune !== '$derived') return;

for (const path of extract_paths(node.id)) {
// @ts-ignore this fails in CI for some insane reason
const binding = /** @type {import('#compiler').Binding} */ (state.scope.get(path.node.name));
binding.kind = name === '$state' ? 'state' : 'derived';
binding.kind =
rune === '$state' ? 'state' : rune === '$state.frozen' ? 'frozen_state' : 'derived';
}
}
};
Expand All @@ -665,28 +667,31 @@ const runes_scope_tweaker = {
VariableDeclarator(node, { state }) {
const init = unwrap_ts_expression(node.init);
if (!init || init.type !== 'CallExpression') return;
if (get_rune(init, state.scope) === null) return;
const rune = get_rune(init, state.scope);
if (rune === null) return;

const callee = init.callee;
if (callee.type !== 'Identifier') return;
if (callee.type !== 'Identifier' && callee.type !== 'MemberExpression') return;

const name = callee.name;
if (name !== '$state' && name !== '$derived' && name !== '$props') return;
if (rune !== '$state' && rune !== '$state.frozen' && rune !== '$derived' && rune !== '$props')
return;

for (const path of extract_paths(node.id)) {
// @ts-ignore this fails in CI for some insane reason
const binding = /** @type {import('#compiler').Binding} */ (state.scope.get(path.node.name));
binding.kind =
name === '$state'
rune === '$state'
? 'state'
: name === '$derived'
: rune === '$state.frozen'
? 'frozen_state'
: rune === '$derived'
? 'derived'
: path.is_rest
? 'rest_prop'
: 'prop';
}

if (name === '$props') {
if (rune === '$props') {
for (const property of /** @type {import('estree').ObjectPattern} */ (node.id).properties) {
if (property.type !== 'Property') continue;

Expand Down Expand Up @@ -898,7 +903,9 @@ const common_visitors = {

if (
node !== binding.node &&
(binding.kind === 'state' || binding.kind === 'derived') &&
(binding.kind === 'state' ||
binding.kind === 'frozen_state' ||
binding.kind === 'derived') &&
context.state.function_depth === binding.scope.function_depth
) {
warn(context.state.analysis.warnings, node, context.path, 'static-state-reference');
Expand Down
7 changes: 5 additions & 2 deletions packages/svelte/src/compiler/phases/2-analyze/validation.js
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,7 @@ export const validation = {
if (
!binding ||
(binding.kind !== 'state' &&
binding.kind !== 'frozen_state' &&
binding.kind !== 'prop' &&
binding.kind !== 'each' &&
binding.kind !== 'store_sub' &&
Expand Down Expand Up @@ -661,7 +662,7 @@ function validate_export(node, scope, name) {
error(node, 'invalid-derived-export');
}

if (binding.kind === 'state' && binding.reassigned) {
if ((binding.kind === 'state' || binding.kind === 'frozen_state') && binding.reassigned) {
error(node, 'invalid-state-export');
}
}
Expand Down Expand Up @@ -835,7 +836,9 @@ function validate_no_const_assignment(node, argument, scope, is_binding) {
is_binding,
// This takes advantage of the fact that we don't assign initial for let directives and then/catch variables.
// If we start doing that, we need another property on the binding to differentiate, or give up on the more precise error message.
binding.kind !== 'state' && (binding.kind !== 'normal' || !binding.initial)
binding.kind !== 'state' &&
binding.kind !== 'frozen_state' &&
(binding.kind !== 'normal' || !binding.initial)
);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -233,15 +233,18 @@ export function client_component(source, analysis, options) {
'$.bind_prop',
b.id('$$props'),
b.literal(alias ?? name),
binding?.kind === 'state' ? b.call('$.get', b.id(name)) : b.id(name)
binding?.kind === 'state' || binding?.kind === 'frozen_state'
? b.call('$.get', b.id(name))
: b.id(name)
)
);
});

const properties = analysis.exports.map(({ name, alias }) => {
const binding = analysis.instance.scope.get(name);
const is_source =
binding?.kind === 'state' && (!state.analysis.immutable || binding.reassigned);
(binding?.kind === 'state' || binding?.kind === 'frozen_state') &&
(!state.analysis.immutable || binding.reassigned);

// TODO This is always a getter because the `renamed-instance-exports` test wants it that way.
// Should we for code size reasons make it an init in runes mode and/or non-dev mode?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export interface ComponentClientTransformState extends ClientTransformState {
}

export interface StateField {
kind: 'state' | 'derived';
kind: 'state' | 'frozen_state' | 'derived';
id: PrivateIdentifier;
}

Expand Down
76 changes: 51 additions & 25 deletions packages/svelte/src/compiler/phases/3-transform/client/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ export function serialize_get_binding(node, state) {
}

if (
(binding.kind === 'state' &&
((binding.kind === 'state' || binding.kind === 'frozen_state') &&
(!state.analysis.immutable || state.analysis.accessors || binding.reassigned)) ||
binding.kind === 'derived' ||
binding.kind === 'legacy_reactive'
Expand Down Expand Up @@ -162,40 +162,53 @@ export function serialize_set_binding(node, context, fallback) {

// Handle class private/public state assignment cases
while (left.type === 'MemberExpression') {
if (
left.object.type === 'ThisExpression' &&
left.property.type === 'PrivateIdentifier' &&
context.state.private_state.has(left.property.name)
) {
if (left.object.type === 'ThisExpression' && left.property.type === 'PrivateIdentifier') {
const private_state = context.state.private_state.get(left.property.name);
const value = get_assignment_value(node, context);
if (state.in_constructor) {
// See if we should wrap value in $.proxy
if (context.state.analysis.runes && should_proxy(value)) {
const assignment = fallback();
if (assignment.type === 'AssignmentExpression') {
assignment.right = b.call('$.proxy', value);
return assignment;
if (private_state !== undefined) {
if (state.in_constructor) {
// See if we should wrap value in $.proxy
if (context.state.analysis.runes && should_proxy_or_freeze(value)) {
const assignment = fallback();
if (assignment.type === 'AssignmentExpression') {
assignment.right =
private_state.kind === 'frozen_state'
? b.call('$.freeze', value)
: b.call('$.proxy', value);
return assignment;
}
}
} else {
return b.call(
'$.set',
left,
context.state.analysis.runes && should_proxy_or_freeze(value)
? private_state.kind === 'frozen_state'
? b.call('$.freeze', value)
: b.call('$.proxy', value)
: value
);
}
} else {
return b.call(
'$.set',
left,
context.state.analysis.runes && should_proxy(value) ? b.call('$.proxy', value) : value
);
}
} else if (
left.object.type === 'ThisExpression' &&
left.property.type === 'Identifier' &&
context.state.public_state.has(left.property.name) &&
state.in_constructor
) {
const public_state = context.state.public_state.get(left.property.name);
const value = get_assignment_value(node, context);
// See if we should wrap value in $.proxy
if (context.state.analysis.runes && should_proxy(value)) {
if (
context.state.analysis.runes &&
public_state !== undefined &&
should_proxy_or_freeze(value)
) {
const assignment = fallback();
if (assignment.type === 'AssignmentExpression') {
assignment.right = b.call('$.proxy', value);
assignment.right =
public_state.kind === 'frozen_state'
? b.call('$.freeze', value)
: b.call('$.proxy', value);
return assignment;
}
}
Expand Down Expand Up @@ -232,6 +245,7 @@ export function serialize_set_binding(node, context, fallback) {

if (
binding.kind !== 'state' &&
binding.kind !== 'frozen_state' &&
binding.kind !== 'prop' &&
binding.kind !== 'each' &&
binding.kind !== 'legacy_reactive' &&
Expand All @@ -249,12 +263,24 @@ export function serialize_set_binding(node, context, fallback) {
return b.call(left, value);
} else if (is_store) {
return b.call('$.store_set', serialize_get_binding(b.id(left_name), state), value);
} else {
} else if (binding.kind === 'state') {
return b.call(
'$.set',
b.id(left_name),
context.state.analysis.runes && should_proxy(value) ? b.call('$.proxy', value) : value
context.state.analysis.runes && should_proxy_or_freeze(value)
? b.call('$.proxy', value)
: value
);
} else if (binding.kind === 'frozen_state') {
return b.call(
'$.set',
b.id(left_name),
context.state.analysis.runes && should_proxy_or_freeze(value)
? b.call('$.freeze', value)
: value
);
} else {
return b.call('$.set', b.id(left_name), value);
}
} else {
if (is_store) {
Expand Down Expand Up @@ -493,7 +519,7 @@ export function create_state_declarators(declarator, scope, value) {
}

/** @param {import('estree').Expression} node */
export function should_proxy(node) {
export function should_proxy_or_freeze(node) {
if (
!node ||
node.type === 'Literal' ||
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export const global_visitors = {
// use runtime functions for smaller output
if (
binding?.kind === 'state' ||
binding?.kind === 'frozen_state' ||
binding?.kind === 'each' ||
binding?.kind === 'legacy_reactive' ||
binding?.kind === 'prop' ||
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { get_rune } from '../../../scope.js';
import { is_hoistable_function, transform_inspect_rune } from '../../utils.js';
import * as b from '../../../../utils/builders.js';
import * as assert from '../../../../utils/assert.js';
import { create_state_declarators, get_prop_source, should_proxy } from '../utils.js';
import { create_state_declarators, get_prop_source, should_proxy_or_freeze } from '../utils.js';
import { unwrap_ts_expression } from '../../../../utils/ast.js';

/** @type {import('../types.js').ComponentVisitors} */
Expand All @@ -29,10 +29,11 @@ export const javascript_visitors_runes = {

if (definition.value?.type === 'CallExpression') {
const rune = get_rune(definition.value, state.scope);
if (rune === '$state' || rune === '$derived') {
if (rune === '$state' || rune === '$state.frozen' || rune === '$derived') {
/** @type {import('../types.js').StateField} */
const field = {
kind: rune === '$state' ? 'state' : 'derived',
kind:
rune === '$state' ? 'state' : rune === '$state.frozen' ? 'frozen_state' : 'derived',
// @ts-expect-error this is set in the next pass
id: is_private ? definition.key : null
};
Expand Down Expand Up @@ -84,7 +85,9 @@ export const javascript_visitors_runes = {

value =
field.kind === 'state'
? b.call('$.source', should_proxy(init) ? b.call('$.proxy', init) : init)
? b.call('$.source', should_proxy_or_freeze(init) ? b.call('$.proxy', init) : init)
: field.kind === 'frozen_state'
? b.call('$.source', should_proxy_or_freeze(init) ? b.call('$.freeze', init) : init)
: b.call('$.derived', b.thunk(init));
} else {
// if no arguments, we know it's state as `$derived()` is a compile error
Expand Down Expand Up @@ -114,6 +117,19 @@ export const javascript_visitors_runes = {
);
}

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' && state.options.dev) {
body.push(
b.method(
Expand Down Expand Up @@ -217,13 +233,24 @@ export const javascript_visitors_runes = {
const binding = /** @type {import('#compiler').Binding} */ (
state.scope.get(declarator.id.name)
);
if (should_proxy(value)) {
if (should_proxy_or_freeze(value)) {
value = b.call('$.proxy', value);
}

if (!state.analysis.immutable || state.analysis.accessors || binding.reassigned) {
value = b.call('$.source', value);
}
} else if (rune === '$state.frozen') {
const binding = /** @type {import('#compiler').Binding} */ (
state.scope.get(declarator.id.name)
);
if (should_proxy_or_freeze(value)) {
value = b.call('$.freeze', value);
}

if (binding.reassigned) {
value = b.call('$.source', value);
}
} else {
value = b.call('$.derived', b.thunk(value));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1245,6 +1245,7 @@ function serialize_event_handler(node, { state, visit }) {
if (
binding !== null &&
(binding.kind === 'state' ||
binding.kind === 'frozen_state' ||
binding.kind === 'legacy_reactive' ||
binding.kind === 'derived' ||
binding.kind === 'prop' ||
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,7 @@ function serialize_set_binding(node, context, fallback) {

if (
binding.kind !== 'state' &&
binding.kind !== 'frozen_state' &&
binding.kind !== 'prop' &&
binding.kind !== 'each' &&
binding.kind !== 'legacy_reactive' &&
Expand Down Expand Up @@ -558,7 +559,7 @@ const javascript_visitors_runes = {
if (node.value != null && node.value.type === 'CallExpression') {
const rune = get_rune(node.value, state.scope);

if (rune === '$state' || rune === '$derived') {
if (rune === '$state' || rune === '$state.frozen' || rune === '$derived') {
return {
...node,
value:
Expand Down
1 change: 1 addition & 0 deletions packages/svelte/src/compiler/phases/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ export const ElementBindings = [

export const Runes = /** @type {const} */ ([
'$state',
'$state.frozen',
'$props',
'$derived',
'$effect',
Expand Down
Loading