From 771b1e89055b0acd8134e1cdf36283d519dd3168 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Thu, 24 Oct 2024 22:12:48 +0100 Subject: [PATCH] fix: enable bound store props in runes mode components (#13887) * fix: enable bound store props in runes mode components * add some JSDoc, since it could be a headscratcher for future us * make it clear that this is specifically about bindings * skip intermediate value * tweak other names too --------- Co-authored-by: Rich Harris --- .changeset/cool-clocks-march.md | 5 +++ .../client/visitors/shared/component.js | 12 ++++++- packages/svelte/src/internal/client/index.js | 3 +- .../src/internal/client/reactivity/props.js | 11 +++++-- .../src/internal/client/reactivity/store.js | 33 +++++++++++++++++++ .../svelte/src/internal/client/validate.js | 6 +++- .../samples/bound-store-sub/Child.svelte | 7 ++++ .../samples/bound-store-sub/_config.js | 25 ++++++++++++++ .../samples/bound-store-sub/main.svelte | 11 +++++++ 9 files changed, 108 insertions(+), 5 deletions(-) create mode 100644 .changeset/cool-clocks-march.md create mode 100644 packages/svelte/tests/runtime-runes/samples/bound-store-sub/Child.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/bound-store-sub/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/bound-store-sub/main.svelte diff --git a/.changeset/cool-clocks-march.md b/.changeset/cool-clocks-march.md new file mode 100644 index 0000000000..b3d5705231 --- /dev/null +++ b/.changeset/cool-clocks-march.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: enable bound store props in runes mode components diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js index e98c4f04f5..8e1a536707 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js @@ -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( diff --git a/packages/svelte/src/internal/client/index.js b/packages/svelte/src/internal/client/index.js index 306fc69ca7..c401867a0f 100644 --- a/packages/svelte/src/internal/client/index.js +++ b/packages/svelte/src/internal/client/index.js @@ -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 { diff --git a/packages/svelte/src/internal/client/reactivity/props.js b/packages/svelte/src/internal/client/reactivity/props.js index 1df1933c19..eec8e70dc6 100644 --- a/packages/svelte/src/internal/client/reactivity/props.js +++ b/packages/svelte/src/internal/client/reactivity/props.js @@ -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 @@ -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); @@ -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; diff --git a/packages/svelte/src/internal/client/reactivity/store.js b/packages/svelte/src/internal/client/reactivity/store.js index e28202aaf0..11eee23e0a 100644 --- a/packages/svelte/src/internal/client/reactivity/store.js +++ b/packages/svelte/src/internal/client/reactivity/store.js @@ -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 + * ``. 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 @@ -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; + } +} diff --git a/packages/svelte/src/internal/client/validate.js b/packages/svelte/src/internal/client/validate.js index 0666e5f87b..c3b1821947 100644 --- a/packages/svelte/src/internal/client/validate.js +++ b/packages/svelte/src/internal/client/validate.js @@ -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 = @@ -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; diff --git a/packages/svelte/tests/runtime-runes/samples/bound-store-sub/Child.svelte b/packages/svelte/tests/runtime-runes/samples/bound-store-sub/Child.svelte new file mode 100644 index 0000000000..8197a71d7c --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/bound-store-sub/Child.svelte @@ -0,0 +1,7 @@ + + +

+ +

diff --git a/packages/svelte/tests/runtime-runes/samples/bound-store-sub/_config.js b/packages/svelte/tests/runtime-runes/samples/bound-store-sub/_config.js new file mode 100644 index 0000000000..a2ba61718c --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/bound-store-sub/_config.js @@ -0,0 +1,25 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; +import { ok } from 'assert'; + +export default test({ + compileOptions: { + dev: true + }, + + html: `

\n{"count":0}`, + ssrHtml: `

\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, `

\n{"count":10}`); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/bound-store-sub/main.svelte b/packages/svelte/tests/runtime-runes/samples/bound-store-sub/main.svelte new file mode 100644 index 0000000000..6ecc4f156e --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/bound-store-sub/main.svelte @@ -0,0 +1,11 @@ + + + +{JSON.stringify($form)}