compile time mutation validation

props-bindable
Simon Holthausen 2 years ago
parent 001a8314fa
commit e98c242d2d

@ -182,6 +182,8 @@ const runes = {
`$props() assignment must not contain nested properties or computed keys`, `$props() assignment must not contain nested properties or computed keys`,
'invalid-props-location': () => 'invalid-props-location': () =>
`$props() can only be used at the top level of components as a variable declaration initializer`, `$props() can only be used at the top level of components as a variable declaration initializer`,
'invalid-props-mutation': () =>
'Properties defined by $props() cannot be mutated. Use $props.bindable() instead, or make a copy of the value and reassign it.',
/** @param {string} rune */ /** @param {string} rune */
'invalid-state-location': (rune) => 'invalid-state-location': (rune) =>
`${rune}(...) can only be used as a variable declaration initializer or a class field`, `${rune}(...) can only be used as a variable declaration initializer or a class field`,

@ -436,7 +436,7 @@ export function analyze_component(root, options) {
); );
} }
} else { } else {
instance.scope.declare(b.id('$$props'), 'prop', 'synthetic'); instance.scope.declare(b.id('$$props'), 'bindable_prop', 'synthetic');
instance.scope.declare(b.id('$$restProps'), 'rest_prop', 'synthetic'); instance.scope.declare(b.id('$$restProps'), 'rest_prop', 'synthetic');
for (const { ast, scope, scopes } of [module, instance, template]) { for (const { ast, scope, scopes } of [module, instance, template]) {
@ -466,7 +466,10 @@ export function analyze_component(root, options) {
} }
for (const [name, binding] of instance.scope.declarations) { for (const [name, binding] of instance.scope.declarations) {
if (binding.kind === 'prop' && binding.node.name !== '$$props') { if (
(binding.kind === 'prop' || binding.kind === 'bindable_prop') &&
binding.node.name !== '$$props'
) {
const references = binding.references.filter( const references = binding.references.filter(
(r) => r.node !== binding.node && r.path.at(-1)?.type !== 'ExportSpecifier' (r) => r.node !== binding.node && r.path.at(-1)?.type !== 'ExportSpecifier'
); );
@ -758,7 +761,7 @@ const legacy_scope_tweaker = {
(binding.kind === 'normal' && (binding.kind === 'normal' &&
(binding.declaration_kind === 'let' || binding.declaration_kind === 'var')) (binding.declaration_kind === 'let' || binding.declaration_kind === 'var'))
) { ) {
binding.kind = 'prop'; binding.kind = 'bindable_prop';
if (specifier.exported.name !== specifier.local.name) { if (specifier.exported.name !== specifier.local.name) {
binding.prop_alias = specifier.exported.name; binding.prop_alias = specifier.exported.name;
} }
@ -796,7 +799,7 @@ const legacy_scope_tweaker = {
for (const declarator of node.declaration.declarations) { for (const declarator of node.declaration.declarations) {
for (const id of extract_identifiers(declarator.id)) { for (const id of extract_identifiers(declarator.id)) {
const binding = /** @type {import('#compiler').Binding} */ (state.scope.get(id.name)); const binding = /** @type {import('#compiler').Binding} */ (state.scope.get(id.name));
binding.kind = 'prop'; binding.kind = 'bindable_prop';
} }
} }
} }
@ -871,7 +874,9 @@ const runes_scope_tweaker = {
? 'derived' ? 'derived'
: path.is_rest : path.is_rest
? 'rest_prop' ? 'rest_prop'
: 'prop'; : rune === '$props.bindable'
? 'bindable_prop'
: 'prop';
} }
if (rune === '$props' || rune === '$props.bindable') { if (rune === '$props' || rune === '$props.bindable') {

@ -299,17 +299,19 @@ const validation = {
error(node, 'invalid-binding-expression'); error(node, 'invalid-binding-expression');
} }
const binding = context.state.scope.get(left.name);
if ( if (
assignee.type === 'Identifier' && assignee.type === 'Identifier' &&
node.name !== 'this' // bind:this also works for regular variables node.name !== 'this' // bind:this also works for regular variables
) { ) {
const binding = context.state.scope.get(left.name);
// reassignment // reassignment
if ( if (
!binding || !binding ||
(binding.kind !== 'state' && (binding.kind !== 'state' &&
binding.kind !== 'frozen_state' && binding.kind !== 'frozen_state' &&
binding.kind !== 'prop' && binding.kind !== 'prop' &&
binding.kind !== 'bindable_prop' &&
binding.kind !== 'each' && binding.kind !== 'each' &&
binding.kind !== 'store_sub' && binding.kind !== 'store_sub' &&
!binding.mutated) !binding.mutated)
@ -328,7 +330,9 @@ const validation = {
// TODO handle mutations of non-state/props in runes mode // TODO handle mutations of non-state/props in runes mode
} }
const binding = context.state.scope.get(left.name); if (assignee.type === 'MemberExpression' && binding?.kind === 'prop') {
error(node, 'invalid-props-mutation');
}
if (node.name === 'group') { if (node.name === 'group') {
if (!binding) { if (!binding) {
@ -969,7 +973,9 @@ function validate_no_const_assignment(node, argument, scope, is_binding) {
function validate_assignment(node, argument, state) { function validate_assignment(node, argument, state) {
validate_no_const_assignment(node, argument, state.scope, false); validate_no_const_assignment(node, argument, state.scope, false);
if (state.analysis.runes && argument.type === 'Identifier') { if (!state.analysis.runes) return;
if (argument.type === 'Identifier') {
const binding = state.scope.get(argument.name); const binding = state.scope.get(argument.name);
if (binding?.kind === 'derived') { if (binding?.kind === 'derived') {
error(node, 'invalid-derived-assignment'); error(node, 'invalid-derived-assignment');
@ -978,19 +984,24 @@ function validate_assignment(node, argument, state) {
if (binding?.kind === 'each') { if (binding?.kind === 'each') {
error(node, 'invalid-each-assignment'); error(node, 'invalid-each-assignment');
} }
} else if (argument.type === 'MemberExpression') {
const id = object(argument);
if (id && state.scope.get(id.name)?.kind === 'prop') {
error(node, 'invalid-props-mutation');
}
} }
let object = /** @type {import('estree').Expression | import('estree').Super} */ (argument); let obj = /** @type {import('estree').Expression | import('estree').Super} */ (argument);
/** @type {import('estree').Expression | import('estree').PrivateIdentifier | null} */ /** @type {import('estree').Expression | import('estree').PrivateIdentifier | null} */
let property = null; let property = null;
while (object.type === 'MemberExpression') { while (obj.type === 'MemberExpression') {
property = object.property; property = obj.property;
object = object.object; obj = obj.object;
} }
if (object.type === 'ThisExpression' && property?.type === 'PrivateIdentifier') { if (obj.type === 'ThisExpression' && property?.type === 'PrivateIdentifier') {
if (state.private_derived_state.includes(property.name)) { if (state.private_derived_state.includes(property.name)) {
error(node, 'invalid-derived-assignment'); error(node, 'invalid-derived-assignment');
} }

@ -251,7 +251,8 @@ export function client_component(source, analysis, options) {
if (analysis.accessors) { if (analysis.accessors) {
for (const [name, binding] of analysis.instance.scope.declarations) { for (const [name, binding] of analysis.instance.scope.declarations) {
if (binding.kind !== 'prop' || name.startsWith('$$')) continue; if ((binding.kind !== 'prop' && binding.kind !== 'bindable_prop') || name.startsWith('$$'))
continue;
const key = binding.prop_alias ?? name; const key = binding.prop_alias ?? name;
@ -356,7 +357,7 @@ export function client_component(source, analysis, options) {
/** @type {string[]} */ /** @type {string[]} */
const named_props = analysis.exports.map(({ name, alias }) => alias ?? name); const named_props = analysis.exports.map(({ name, alias }) => alias ?? name);
for (const [name, binding] of analysis.instance.scope.declarations) { for (const [name, binding] of analysis.instance.scope.declarations) {
if (binding.kind === 'prop') named_props.push(binding.prop_alias ?? name); if (binding.kind === 'bindable_prop') named_props.push(binding.prop_alias ?? name);
} }
component_block.body.unshift( component_block.body.unshift(
@ -464,7 +465,8 @@ export function client_component(source, analysis, options) {
const props_str = []; const props_str = [];
for (const [name, binding] of analysis.instance.scope.declarations) { for (const [name, binding] of analysis.instance.scope.declarations) {
if (binding.kind !== 'prop' || name.startsWith('$$')) continue; if ((binding.kind !== 'prop' && binding.kind !== 'bindable_prop') || name.startsWith('$$'))
continue;
const key = binding.prop_alias ?? name; const key = binding.prop_alias ?? name;
const prop_def = typeof ce === 'boolean' ? {} : ce.props?.[key] || {}; const prop_def = typeof ce === 'boolean' ? {} : ce.props?.[key] || {};

@ -78,7 +78,7 @@ export function serialize_get_binding(node, state) {
return typeof binding.expression === 'function' ? binding.expression(node) : binding.expression; return typeof binding.expression === 'function' ? binding.expression(node) : binding.expression;
} }
if (binding.kind === 'prop') { if (binding.kind === 'prop' || binding.kind === 'bindable_prop') {
if (binding.node.name === '$$props') { if (binding.node.name === '$$props') {
// Special case for $$props which only exists in the old world // Special case for $$props which only exists in the old world
// TODO this probably shouldn't have a 'prop' binding kind // TODO this probably shouldn't have a 'prop' binding kind
@ -377,6 +377,7 @@ export function serialize_set_binding(node, context, fallback, options) {
binding.kind !== 'state' && binding.kind !== 'state' &&
binding.kind !== 'frozen_state' && binding.kind !== 'frozen_state' &&
binding.kind !== 'prop' && binding.kind !== 'prop' &&
binding.kind !== 'bindable_prop' &&
binding.kind !== 'each' && binding.kind !== 'each' &&
binding.kind !== 'legacy_reactive' && binding.kind !== 'legacy_reactive' &&
!is_store !is_store
@ -389,7 +390,7 @@ export function serialize_set_binding(node, context, fallback, options) {
const serialize = () => { const serialize = () => {
if (left === node.left) { if (left === node.left) {
if (binding.kind === 'prop') { if (binding.kind === 'prop' || binding.kind === 'bindable_prop') {
return b.call(left, value); return b.call(left, value);
} else if (is_store) { } else if (is_store) {
return b.call('$.store_set', serialize_get_binding(b.id(left_name), state), value); return b.call('$.store_set', serialize_get_binding(b.id(left_name), state), value);
@ -467,7 +468,7 @@ export function serialize_set_binding(node, context, fallback, options) {
b.call('$.untrack', b.id('$' + left_name)) b.call('$.untrack', b.id('$' + left_name))
); );
} else if (!state.analysis.runes) { } else if (!state.analysis.runes) {
if (binding.kind === 'prop') { if (binding.kind === 'bindable_prop') {
return b.call( return b.call(
left, left,
b.sequence([ b.sequence([
@ -571,7 +572,7 @@ function get_hoistable_params(node, context) {
params.push(b.id(binding.expression.object.arguments[0].name)); params.push(b.id(binding.expression.object.arguments[0].name));
} else if ( } else if (
// If we are referencing a simple $$props value, then we need to reference the object property instead // If we are referencing a simple $$props value, then we need to reference the object property instead
binding.kind === 'prop' && (binding.kind === 'prop' || binding.kind === 'bindable_prop') &&
!binding.reassigned && !binding.reassigned &&
binding.initial === null && binding.initial === null &&
!context.state.analysis.accessors !context.state.analysis.accessors

@ -52,6 +52,7 @@ export const global_visitors = {
binding?.kind === 'each' || binding?.kind === 'each' ||
binding?.kind === 'legacy_reactive' || binding?.kind === 'legacy_reactive' ||
binding?.kind === 'prop' || binding?.kind === 'prop' ||
binding?.kind === 'bindable_prop' ||
is_store is_store
) { ) {
/** @type {import('estree').Expression[]} */ /** @type {import('estree').Expression[]} */
@ -64,7 +65,7 @@ export const global_visitors = {
fn += '_store'; fn += '_store';
args.push(serialize_get_binding(b.id(name), state), b.call('$' + name)); args.push(serialize_get_binding(b.id(name), state), b.call('$' + name));
} else { } else {
if (binding.kind === 'prop') fn += '_prop'; if (binding.kind === 'prop' || binding.kind === 'bindable_prop') fn += '_prop';
args.push(b.id(name)); args.push(b.id(name));
} }

@ -40,7 +40,7 @@ export const javascript_visitors_legacy = {
state.scope.get_bindings(declarator) state.scope.get_bindings(declarator)
); );
const has_state = bindings.some((binding) => binding.kind === 'state'); const has_state = bindings.some((binding) => binding.kind === 'state');
const has_props = bindings.some((binding) => binding.kind === 'prop'); const has_props = bindings.some((binding) => binding.kind === 'bindable_prop');
if (!has_state && !has_props) { if (!has_state && !has_props) {
const init = declarator.init; const init = declarator.init;
@ -80,7 +80,7 @@ export const javascript_visitors_legacy = {
declarations.push( declarations.push(
b.declarator( b.declarator(
path.node, path.node,
binding.kind === 'prop' binding.kind === 'bindable_prop'
? get_prop_source(binding, state, binding.prop_alias ?? name, value) ? get_prop_source(binding, state, binding.prop_alias ?? name, value)
: value : value
) )
@ -168,7 +168,7 @@ export const javascript_visitors_legacy = {
// If the binding is a prop, we need to deep read it because it could be fine-grained $state // If the binding is a prop, we need to deep read it because it could be fine-grained $state
// from a runes-component, where mutations don't trigger an update on the prop as a whole. // from a runes-component, where mutations don't trigger an update on the prop as a whole.
if (name === '$$props' || name === '$$restProps' || binding.kind === 'prop') { if (name === '$$props' || name === '$$restProps' || binding.kind === 'bindable_prop') {
serialized = b.call('$.deep_read_state', serialized); serialized = b.call('$.deep_read_state', serialized);
} }

@ -1367,6 +1367,7 @@ function serialize_event_handler(node, { state, visit }) {
binding.kind === 'legacy_reactive' || binding.kind === 'legacy_reactive' ||
binding.kind === 'derived' || binding.kind === 'derived' ||
binding.kind === 'prop' || binding.kind === 'prop' ||
binding.kind === 'bindable_prop' ||
binding.kind === 'store_sub') binding.kind === 'store_sub')
) { ) {
handler = dynamic_handler(); handler = dynamic_handler();

@ -446,6 +446,7 @@ function serialize_set_binding(node, context, fallback) {
binding.kind !== 'state' && binding.kind !== 'state' &&
binding.kind !== 'frozen_state' && binding.kind !== 'frozen_state' &&
binding.kind !== 'prop' && binding.kind !== 'prop' &&
binding.kind !== 'bindable_prop' &&
binding.kind !== 'each' && binding.kind !== 'each' &&
binding.kind !== 'legacy_reactive' && binding.kind !== 'legacy_reactive' &&
!is_store !is_store
@ -1131,7 +1132,7 @@ const javascript_visitors_legacy = {
state.scope.get_bindings(declarator) state.scope.get_bindings(declarator)
); );
const has_state = bindings.some((binding) => binding.kind === 'state'); const has_state = bindings.some((binding) => binding.kind === 'state');
const has_props = bindings.some((binding) => binding.kind === 'prop'); const has_props = bindings.some((binding) => binding.kind === 'bindable_prop');
if (!has_state && !has_props) { if (!has_state && !has_props) {
declarations.push(/** @type {import('estree').VariableDeclarator} */ (visit(declarator))); declarations.push(/** @type {import('estree').VariableDeclarator} */ (visit(declarator)));
@ -2258,7 +2259,7 @@ export function server_component(analysis, options) {
/** @type {import('estree').Property[]} */ /** @type {import('estree').Property[]} */
const props = []; const props = [];
for (const [name, binding] of analysis.instance.scope.declarations) { for (const [name, binding] of analysis.instance.scope.declarations) {
if (binding.kind === 'prop' && !name.startsWith('$$')) { if (binding.kind === 'bindable_prop' && !name.startsWith('$$')) {
props.push(b.init(binding.prop_alias ?? name, b.id(name))); props.push(b.init(binding.prop_alias ?? name, b.id(name)));
} }
} }
@ -2280,7 +2281,7 @@ export function server_component(analysis, options) {
/** @type {string[]} */ /** @type {string[]} */
const named_props = analysis.exports.map(({ name, alias }) => alias ?? name); const named_props = analysis.exports.map(({ name, alias }) => alias ?? name);
for (const [name, binding] of analysis.instance.scope.declarations) { for (const [name, binding] of analysis.instance.scope.declarations) {
if (binding.kind === 'prop') named_props.push(binding.prop_alias ?? name); if (binding.kind === 'bindable_prop') named_props.push(binding.prop_alias ?? name);
} }
component_block.body.unshift( component_block.body.unshift(

@ -241,7 +241,8 @@ export interface Binding {
node: Identifier; node: Identifier;
/** /**
* - `normal`: A variable that is not in any way special * - `normal`: A variable that is not in any way special
* - `prop`: A normal prop (possibly mutated) * - `prop`: A normal prop (possibly reassigned)
* - `bindable_prop`: A prop one can `bind:` to (possibly reassigned or mutated)
* - `rest_prop`: A rest prop * - `rest_prop`: A rest prop
* - `state`: A state variable * - `state`: A state variable
* - `derived`: A derived variable * - `derived`: A derived variable
@ -253,6 +254,7 @@ export interface Binding {
kind: kind:
| 'normal' | 'normal'
| 'prop' | 'prop'
| 'bindable_prop'
| 'rest_prop' | 'rest_prop'
| 'state' | 'state'
| 'frozen_state' | 'frozen_state'

@ -0,0 +1,9 @@
import { test } from '../../test';
export default test({
error: {
code: 'invalid-props-mutation',
message:
'Properties defined by $props() cannot be mutated. Use $props.bindable() instead, or make a copy of the value and reassign it.'
}
});

@ -1,5 +1,5 @@
<script> <script>
let { items = [{ src: 'https://ds' }] } = $props(); let { items = [{ src: 'https://ds' }] } = $props.bindable();
</script> </script>
{#each items as item, i} {#each items as item, i}

@ -1,6 +1,6 @@
<script> <script>
/** @type {{ object: { count: number }}} */ /** @type {{ object: { count: number }}} */
let { object } = $props(); let { object } = $props.bindable();
</script> </script>
<button onclick={() => object.count += 1}> <button onclick={() => object.count += 1}>

@ -1,5 +1,6 @@
<script> <script>
let { count, inc } = $props(); let { inc } = $props();
let { count } = $props.bindable();
</script> </script>
<button onclick={inc}>{count.a} (ok)</button> <button onclick={inc}>{count.a} (ok)</button>

@ -1,6 +1,6 @@
<script> <script>
/** @type {{ object: { count: number }}} */ /** @type {{ object: { count: number }}} */
let { object } = $props(); let { object } = $props.bindable();
</script> </script>
<button onclick={() => object.count += 1}> <button onclick={() => object.count += 1}>

@ -1,5 +1,5 @@
<script> <script>
let { count = 0 } = $props(); let { count = 0 } = $props.bindable();
</script> </script>
<span>{count}</span> <span>{count}</span>

@ -1,6 +1,6 @@
<script> <script>
/** @type {{ object?: { count: number }}} */ /** @type {{ object?: { count: number }}} */
let { object = { count: 0 } } = $props(); let { object = { count: 0 } } = $props.bindable();
</script> </script>
<button onclick={() => object.count += 1}> <button onclick={() => object.count += 1}>

@ -1,6 +1,6 @@
<script> <script>
/** @type {{ object: { count: number }}} */ /** @type {{ object: { count: number }}} */
let { object } = $props(); let { object } = $props.bindable();
</script> </script>
<button onclick={() => object.count += 1}> <button onclick={() => object.count += 1}>

Loading…
Cancel
Save