Skip to content

fix: enable bound store props in runes mode components #13887

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

Merged
merged 5 commits into from
Oct 24, 2024
Merged
Show file tree
Hide file tree
Changes from all 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/cool-clocks-march.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: enable bound store props in runes mode components
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,17 @@ export function build_component(node, component_name, context, anchor = context.
);
}

push_prop(b.get(attribute.name, [b.return(expression)]));
const is_store_sub =
attribute.expression.type === 'Identifier' &&
context.state.scope.get(attribute.expression.name)?.kind === 'store_sub';

if (is_store_sub) {
push_prop(
b.get(attribute.name, [b.stmt(b.call('$.mark_store_binding')), b.return(expression)])
);
} else {
push_prop(b.get(attribute.name, [b.return(expression)]));
}

const assignment = b.assignment('=', attribute.expression, b.id('$$value'));
push_prop(
Expand Down
3 changes: 2 additions & 1 deletion packages/svelte/src/internal/client/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,8 @@ export {
store_set,
store_unsub,
update_pre_store,
update_store
update_store,
mark_store_binding
} from './reactivity/store.js';
export { set_text } from './render.js';
export {
Expand Down
11 changes: 9 additions & 2 deletions packages/svelte/src/internal/client/reactivity/props.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { safe_equals } from './equality.js';
import * as e from '../errors.js';
import { BRANCH_EFFECT, DESTROYED, LEGACY_DERIVED_PROP, ROOT_EFFECT } from '../constants.js';
import { proxy } from '../proxy.js';
import { capture_store_binding } from './store.js';

/**
* @param {((value?: number) => number)} fn
Expand Down Expand Up @@ -273,8 +274,14 @@ export function prop(props, key, flags, fallback) {
var runes = (flags & PROPS_IS_RUNES) !== 0;
var bindable = (flags & PROPS_IS_BINDABLE) !== 0;
var lazy = (flags & PROPS_IS_LAZY_INITIAL) !== 0;
var is_store_sub = false;
var prop_value;

var prop_value = /** @type {V} */ (props[key]);
if (bindable) {
[prop_value, is_store_sub] = capture_store_binding(() => /** @type {V} */ (props[key]));
} else {
prop_value = /** @type {V} */ (props[key]);
}
var setter = get_descriptor(props, key)?.set;

var fallback_value = /** @type {V} */ (fallback);
Expand Down Expand Up @@ -343,7 +350,7 @@ export function prop(props, key, flags, fallback) {
// In that case the state proxy (if it exists) should take care of the notification.
// If the parent is not in runes mode, we need to notify on mutation, too, that the prop
// has changed because the parent will not be able to detect the change otherwise.
if (!runes || !mutation || legacy_parent) {
if (!runes || !mutation || legacy_parent || is_store_sub) {
/** @type {Function} */ (setter)(mutation ? getter() : value);
}
return value;
Expand Down
33 changes: 33 additions & 0 deletions packages/svelte/src/internal/client/reactivity/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,13 @@ import { get } from '../runtime.js';
import { teardown } from './effects.js';
import { mutable_source, set } from './sources.js';

/**
* Whether or not the prop currently being read is a store binding, as in
* `<Child bind:x={$y} />`. If it is, we treat the prop as mutable even in
* runes mode, and skip `binding_property_non_reactive` validation
*/
let is_store_binding = false;

/**
* Gets the current value of a store. If the store isn't subscribed to yet, it will create a proxy
* signal that will be updated when the store is. The store references container is needed to
Expand Down Expand Up @@ -146,3 +153,29 @@ export function update_pre_store(store, store_value, d = 1) {
store.set(value);
return value;
}

/**
* Called inside prop getters to communicate that the prop is a store binding
*/
export function mark_store_binding() {
is_store_binding = true;
}

/**
* Returns a tuple that indicates whether `fn()` reads a prop that is a store binding.
* Used to prevent `binding_property_non_reactive` validation false positives and
* ensure that these props are treated as mutable even in runes mode
* @template T
* @param {() => T} fn
* @returns {[T, boolean]}
*/
export function capture_store_binding(fn) {
var previous_is_store_binding = is_store_binding;

try {
is_store_binding = false;
return [fn(), is_store_binding];
} finally {
is_store_binding = previous_is_store_binding;
}
}
6 changes: 5 additions & 1 deletion packages/svelte/src/internal/client/validate.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import * as e from './errors.js';
import { FILENAME } from '../../constants.js';
import { render_effect } from './reactivity/effects.js';
import * as w from './warnings.js';
import { capture_store_binding } from './reactivity/store.js';

/** regex of all html void element names */
const void_element_names =
Expand Down Expand Up @@ -84,7 +85,10 @@ export function validate_binding(binding, get_object, get_property, line, column
render_effect(() => {
if (warned) return;

var object = get_object();
var [object, is_store_sub] = capture_store_binding(get_object);

if (is_store_sub) return;

var property = get_property();

var ran = false;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<script>
let { form = $bindable() } = $props();
</script>

<p>
<input type="number" bind:value={form.count} />
</p>
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { flushSync } from 'svelte';
import { test } from '../../test';
import { ok } from 'assert';

export default test({
compileOptions: {
dev: true
},

html: `<p><input type="number"></p>\n{"count":0}`,
ssrHtml: `<p><input type="number" value="0"></p>\n{"count":0}`,

test({ assert, target }) {
const input = target.querySelector('input');
ok(input);
const inputEvent = new window.InputEvent('input');

input.value = '10';
input.dispatchEvent(inputEvent);

flushSync();

assert.htmlEqual(target.innerHTML, `<p><input type="number"></p>\n{"count":10}`);
}
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<script>
import { writable } from 'svelte/store';
import Child from './Child.svelte';

let form = writable({
count: 0
});
</script>

<Child bind:form={$form} />
{JSON.stringify($form)}