From 28deabdc5798cf41e6cd851c74990fbf51cf280d Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Thu, 24 Oct 2024 20:52:08 +0100 Subject: [PATCH 1/5] fix: enable bound store props in runes mode components --- .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 | 22 ++++++++++++++++ .../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, 97 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 000000000000..b3d570523180 --- /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 e98c4f04f5f4..ca909955a104 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_sub')), 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 306fc69ca745..3f6ade67346d 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_sub } 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 1df1933c19db..e13b64a319dc 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_marked_store_sub } 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_marked_store_sub(() => /** @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 e28202aaf096..5dbec04267c9 100644 --- a/packages/svelte/src/internal/client/reactivity/store.js +++ b/packages/svelte/src/internal/client/reactivity/store.js @@ -6,6 +6,8 @@ import { get } from '../runtime.js'; import { teardown } from './effects.js'; import { mutable_source, set } from './sources.js'; +let marked_store_sub = 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 +148,23 @@ export function update_pre_store(store, store_value, d = 1) { store.set(value); return value; } + +export function mark_store_sub() { + marked_store_sub = true; +} + +/** + * @template T + * @param {() => T} fn + * @returns {[T, boolean]} + */ +export function capture_marked_store_sub(fn) { + var previous_marked_store_sub = marked_store_sub; + try { + marked_store_sub = false; + var value = fn(); + return [value, marked_store_sub]; + } finally { + marked_store_sub = previous_marked_store_sub; + } +} diff --git a/packages/svelte/src/internal/client/validate.js b/packages/svelte/src/internal/client/validate.js index 0666e5f87bb0..681fd0c3fcf2 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_marked_store_sub } 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_marked_store_sub(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 000000000000..8197a71d7c1e --- /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 000000000000..a2ba61718c92 --- /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 000000000000..6ecc4f156e78 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/bound-store-sub/main.svelte @@ -0,0 +1,11 @@ + + + +{JSON.stringify($form)} From 9f3ce12a28f08ffef5dad55fa7d5467dff702cff Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 24 Oct 2024 17:03:16 -0400 Subject: [PATCH 2/5] add some JSDoc, since it could be a headscratcher for future us --- .../svelte/src/internal/client/reactivity/store.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/packages/svelte/src/internal/client/reactivity/store.js b/packages/svelte/src/internal/client/reactivity/store.js index 5dbec04267c9..234783134fd5 100644 --- a/packages/svelte/src/internal/client/reactivity/store.js +++ b/packages/svelte/src/internal/client/reactivity/store.js @@ -6,6 +6,11 @@ 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 marked_store_sub = false; /** @@ -149,11 +154,17 @@ export function update_pre_store(store, store_value, d = 1) { return value; } +/** + * Called inside prop getters to communicate that the prop is a store binding + */ export function mark_store_sub() { marked_store_sub = 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]} From 7202bc97207961646919475b646e0460348ad08c Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 24 Oct 2024 17:04:42 -0400 Subject: [PATCH 3/5] make it clear that this is specifically about bindings --- .../phases/3-transform/client/visitors/shared/component.js | 2 +- packages/svelte/src/internal/client/index.js | 2 +- packages/svelte/src/internal/client/reactivity/store.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) 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 ca909955a104..8e1a53670708 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 @@ -194,7 +194,7 @@ export function build_component(node, component_name, context, anchor = context. if (is_store_sub) { push_prop( - b.get(attribute.name, [b.stmt(b.call('$.mark_store_sub')), b.return(expression)]) + b.get(attribute.name, [b.stmt(b.call('$.mark_store_binding')), b.return(expression)]) ); } else { push_prop(b.get(attribute.name, [b.return(expression)])); diff --git a/packages/svelte/src/internal/client/index.js b/packages/svelte/src/internal/client/index.js index 3f6ade67346d..c401867a0f0b 100644 --- a/packages/svelte/src/internal/client/index.js +++ b/packages/svelte/src/internal/client/index.js @@ -122,7 +122,7 @@ export { store_unsub, update_pre_store, update_store, - mark_store_sub + mark_store_binding } from './reactivity/store.js'; export { set_text } from './render.js'; export { diff --git a/packages/svelte/src/internal/client/reactivity/store.js b/packages/svelte/src/internal/client/reactivity/store.js index 234783134fd5..4d35fc0e45df 100644 --- a/packages/svelte/src/internal/client/reactivity/store.js +++ b/packages/svelte/src/internal/client/reactivity/store.js @@ -157,7 +157,7 @@ export function update_pre_store(store, store_value, d = 1) { /** * Called inside prop getters to communicate that the prop is a store binding */ -export function mark_store_sub() { +export function mark_store_binding() { marked_store_sub = true; } From d1104f635d13355d34d662003e939e8a927ed508 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 24 Oct 2024 17:06:19 -0400 Subject: [PATCH 4/5] skip intermediate value --- packages/svelte/src/internal/client/reactivity/store.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/store.js b/packages/svelte/src/internal/client/reactivity/store.js index 4d35fc0e45df..b9dc02b76e67 100644 --- a/packages/svelte/src/internal/client/reactivity/store.js +++ b/packages/svelte/src/internal/client/reactivity/store.js @@ -171,10 +171,10 @@ export function mark_store_binding() { */ export function capture_marked_store_sub(fn) { var previous_marked_store_sub = marked_store_sub; + try { marked_store_sub = false; - var value = fn(); - return [value, marked_store_sub]; + return [fn(), marked_store_sub]; } finally { marked_store_sub = previous_marked_store_sub; } From 43dc1474fd1259e8f570d463626de98edf3da9c4 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 24 Oct 2024 17:07:35 -0400 Subject: [PATCH 5/5] tweak other names too --- .../svelte/src/internal/client/reactivity/props.js | 4 ++-- .../svelte/src/internal/client/reactivity/store.js | 14 +++++++------- packages/svelte/src/internal/client/validate.js | 4 ++-- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/props.js b/packages/svelte/src/internal/client/reactivity/props.js index e13b64a319dc..eec8e70dc68d 100644 --- a/packages/svelte/src/internal/client/reactivity/props.js +++ b/packages/svelte/src/internal/client/reactivity/props.js @@ -23,7 +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_marked_store_sub } from './store.js'; +import { capture_store_binding } from './store.js'; /** * @param {((value?: number) => number)} fn @@ -278,7 +278,7 @@ export function prop(props, key, flags, fallback) { var prop_value; if (bindable) { - [prop_value, is_store_sub] = capture_marked_store_sub(() => /** @type {V} */ (props[key])); + [prop_value, is_store_sub] = capture_store_binding(() => /** @type {V} */ (props[key])); } else { prop_value = /** @type {V} */ (props[key]); } diff --git a/packages/svelte/src/internal/client/reactivity/store.js b/packages/svelte/src/internal/client/reactivity/store.js index b9dc02b76e67..11eee23e0ae8 100644 --- a/packages/svelte/src/internal/client/reactivity/store.js +++ b/packages/svelte/src/internal/client/reactivity/store.js @@ -11,7 +11,7 @@ import { mutable_source, set } from './sources.js'; * ``. If it is, we treat the prop as mutable even in * runes mode, and skip `binding_property_non_reactive` validation */ -let marked_store_sub = false; +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 @@ -158,7 +158,7 @@ export function update_pre_store(store, store_value, d = 1) { * Called inside prop getters to communicate that the prop is a store binding */ export function mark_store_binding() { - marked_store_sub = true; + is_store_binding = true; } /** @@ -169,13 +169,13 @@ export function mark_store_binding() { * @param {() => T} fn * @returns {[T, boolean]} */ -export function capture_marked_store_sub(fn) { - var previous_marked_store_sub = marked_store_sub; +export function capture_store_binding(fn) { + var previous_is_store_binding = is_store_binding; try { - marked_store_sub = false; - return [fn(), marked_store_sub]; + is_store_binding = false; + return [fn(), is_store_binding]; } finally { - marked_store_sub = previous_marked_store_sub; + 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 681fd0c3fcf2..c3b1821947ee 100644 --- a/packages/svelte/src/internal/client/validate.js +++ b/packages/svelte/src/internal/client/validate.js @@ -4,7 +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_marked_store_sub } from './reactivity/store.js'; +import { capture_store_binding } from './reactivity/store.js'; /** regex of all html void element names */ const void_element_names = @@ -85,7 +85,7 @@ export function validate_binding(binding, get_object, get_property, line, column render_effect(() => { if (warned) return; - var [object, is_store_sub] = capture_marked_store_sub(get_object); + var [object, is_store_sub] = capture_store_binding(get_object); if (is_store_sub) return;