diff --git a/.changeset/wise-turkeys-yell.md b/.changeset/wise-turkeys-yell.md new file mode 100644 index 0000000000..cd5e103de3 --- /dev/null +++ b/.changeset/wise-turkeys-yell.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: prevent ownership warnings if the fallback of a bindable is used diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js index af6e56f70c..819c22bd2f 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js @@ -348,6 +348,7 @@ export function validate_mutation(node, context, expression) { b.literal(binding.prop_alias), b.array(path), expression, + binding.prop_alias != null && b.id(binding.prop_alias), loc && b.literal(loc.line), loc && b.literal(loc.column) ); diff --git a/packages/svelte/src/internal/client/constants.js b/packages/svelte/src/internal/client/constants.js index 7e5196c606..f67af28122 100644 --- a/packages/svelte/src/internal/client/constants.js +++ b/packages/svelte/src/internal/client/constants.js @@ -23,5 +23,6 @@ export const EFFECT_HAS_DERIVED = 1 << 20; export const EFFECT_IS_UPDATING = 1 << 21; export const STATE_SYMBOL = Symbol('$state'); +export const BINDABLE_FALLBACK_SYMBOL = Symbol('bindable fallback'); export const LEGACY_PROPS = Symbol('legacy props'); export const LOADING_ATTR_SYMBOL = Symbol(''); diff --git a/packages/svelte/src/internal/client/dev/ownership.js b/packages/svelte/src/internal/client/dev/ownership.js index e28a40dd77..39bb608091 100644 --- a/packages/svelte/src/internal/client/dev/ownership.js +++ b/packages/svelte/src/internal/client/dev/ownership.js @@ -1,7 +1,7 @@ /** @typedef {{ file: string, line: number, column: number }} Location */ import { get_descriptor } from '../../shared/utils.js'; -import { LEGACY_PROPS, STATE_SYMBOL } from '../constants.js'; +import { BINDABLE_FALLBACK_SYMBOL, LEGACY_PROPS, STATE_SYMBOL } from '../constants.js'; import { FILENAME } from '../../../constants.js'; import { component_context } from '../context.js'; import * as w from '../warnings.js'; @@ -22,12 +22,13 @@ export function create_ownership_validator(props) { * @param {string} prop * @param {any[]} path * @param {any} result + * @param {any} prop_value * @param {number} line * @param {number} column */ - mutation: (prop, path, result, line, column) => { + mutation: (prop, path, result, prop_value, line, column) => { const name = path[0]; - if (is_bound(props, name) || !parent) { + if (is_bound(props, name) || !parent || prop_value()?.[BINDABLE_FALLBACK_SYMBOL]) { return result; } @@ -52,7 +53,14 @@ export function create_ownership_validator(props) { * @param {() => any} value */ binding: (key, child_component, value) => { - if (!is_bound(props, key) && parent && value()?.[STATE_SYMBOL]) { + var val; + if ( + !is_bound(props, key) && + parent && + // we do this trick to prevent calling the value function twice + (val = value())?.[STATE_SYMBOL] && + !val[BINDABLE_FALLBACK_SYMBOL] + ) { w.ownership_invalid_binding( component[FILENAME], key, diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index 5e0aa3dbc3..d67d16fbc4 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -9,7 +9,7 @@ import { object_prototype } from '../shared/utils.js'; import { state as source, set } from './reactivity/sources.js'; -import { STATE_SYMBOL } from './constants.js'; +import { BINDABLE_FALLBACK_SYMBOL, STATE_SYMBOL } from './constants.js'; import { UNINITIALIZED } from '../../constants.js'; import * as e from './errors.js'; import { get_stack } from './dev/tracing.js'; @@ -64,10 +64,13 @@ export function proxy(value) { return new Proxy(/** @type {any} */ (value), { defineProperty(_, prop, descriptor) { if ( - !('value' in descriptor) || - descriptor.configurable === false || - descriptor.enumerable === false || - descriptor.writable === false + // we allow non enumerable/writable defines if the prop being set is our own symbol + // this should be fine for the invariants since the user can't get a handle to the symbol + (!DEV || prop !== BINDABLE_FALLBACK_SYMBOL) && + (!('value' in descriptor) || + descriptor.configurable === false || + descriptor.enumerable === false || + descriptor.writable === false) ) { // we disallow non-basic descriptors, because unless they are applied to the // target object — which we avoid, so that state can be forked — we will run diff --git a/packages/svelte/src/internal/client/reactivity/props.js b/packages/svelte/src/internal/client/reactivity/props.js index bd85b14df0..0e098d2f78 100644 --- a/packages/svelte/src/internal/client/reactivity/props.js +++ b/packages/svelte/src/internal/client/reactivity/props.js @@ -7,13 +7,18 @@ import { PROPS_IS_RUNES, PROPS_IS_UPDATED } from '../../../constants.js'; -import { get_descriptor, is_function } from '../../shared/utils.js'; +import { define_property, get_descriptor, is_function } from '../../shared/utils.js'; import { mutable_source, set, source, update } from './sources.js'; import { derived, derived_safe_equal } from './deriveds.js'; import { get, captured_signals, untrack } from '../runtime.js'; import { safe_equals } from './equality.js'; import * as e from '../errors.js'; -import { LEGACY_DERIVED_PROP, LEGACY_PROPS, STATE_SYMBOL } from '../constants.js'; +import { + BINDABLE_FALLBACK_SYMBOL, + LEGACY_DERIVED_PROP, + LEGACY_PROPS, + STATE_SYMBOL +} from '../constants.js'; import { proxy } from '../proxy.js'; import { capture_store_binding } from './store.js'; import { legacy_mode_flag } from '../../flags/index.js'; @@ -304,6 +309,29 @@ export function prop(props, key, flags, fallback) { if (setter) setter(prop_value); } + /** + * @param {any} value + */ + function set_bindable_fallback(value) { + if (DEV && !setter && fallback_used && value != null && typeof value === 'object') { + // in dev we issue a warning if a bindable prop is passed with bindable + // to a child if the prop doesn't have a setter but if it's a fallback + // it's a false positive since the state it's actually created in this + // component so we store the fact that this is a bindable fallback with + // a symbol + define_property(value, BINDABLE_FALLBACK_SYMBOL, { + enumerable: false, + // it needs to be configurable or the proxy will complain when we return true + // for a non configurable property + configurable: true, + writable: false, + value: true + }); + } + } + + set_bindable_fallback(prop_value); + /** @type {() => V} */ var getter; if (runes) { @@ -399,6 +427,11 @@ export function prop(props, key, flags, fallback) { if (arguments.length > 0) { const new_value = mutation ? get(current_value) : runes && bindable ? proxy(value) : value; + // we only care to add the symbol if the original prop is reassigned + if (runes && bindable && !mutation) { + set_bindable_fallback(new_value); + } + if (!current_value.equals(new_value)) { from_child = true; set(inner_current_value, new_value); diff --git a/packages/svelte/tests/runtime-runes/samples/ownership-invalid-binding-bindable-fallback/Child.svelte b/packages/svelte/tests/runtime-runes/samples/ownership-invalid-binding-bindable-fallback/Child.svelte new file mode 100644 index 0000000000..78b82caed9 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/ownership-invalid-binding-bindable-fallback/Child.svelte @@ -0,0 +1,5 @@ + + +{test} diff --git a/packages/svelte/tests/runtime-runes/samples/ownership-invalid-binding-bindable-fallback/Parent.svelte b/packages/svelte/tests/runtime-runes/samples/ownership-invalid-binding-bindable-fallback/Parent.svelte new file mode 100644 index 0000000000..7bfb17aa64 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/ownership-invalid-binding-bindable-fallback/Parent.svelte @@ -0,0 +1,7 @@ + + + diff --git a/packages/svelte/tests/runtime-runes/samples/ownership-invalid-binding-bindable-fallback/_config.js b/packages/svelte/tests/runtime-runes/samples/ownership-invalid-binding-bindable-fallback/_config.js new file mode 100644 index 0000000000..e93067eb9d --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/ownership-invalid-binding-bindable-fallback/_config.js @@ -0,0 +1,11 @@ +import { test } from '../../test'; + +export default test({ + mode: ['client'], + compileOptions: { + dev: true + }, + async test({ warnings, assert }) { + assert.deepEqual(warnings, []); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/ownership-invalid-binding-bindable-fallback/main.svelte b/packages/svelte/tests/runtime-runes/samples/ownership-invalid-binding-bindable-fallback/main.svelte new file mode 100644 index 0000000000..282afb1771 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/ownership-invalid-binding-bindable-fallback/main.svelte @@ -0,0 +1,5 @@ + + + diff --git a/packages/svelte/tests/runtime-runes/samples/ownership-invalid-mutation-bindable-fallback/Parent.svelte b/packages/svelte/tests/runtime-runes/samples/ownership-invalid-mutation-bindable-fallback/Parent.svelte new file mode 100644 index 0000000000..7d6b248da7 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/ownership-invalid-mutation-bindable-fallback/Parent.svelte @@ -0,0 +1,8 @@ + + + + + +{test} \ No newline at end of file diff --git a/packages/svelte/tests/runtime-runes/samples/ownership-invalid-mutation-bindable-fallback/_config.js b/packages/svelte/tests/runtime-runes/samples/ownership-invalid-mutation-bindable-fallback/_config.js new file mode 100644 index 0000000000..9b4e3479ea --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/ownership-invalid-mutation-bindable-fallback/_config.js @@ -0,0 +1,23 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + mode: ['client'], + compileOptions: { + dev: true + }, + async test({ warnings, assert, target }) { + const [btn, btn2] = target.querySelectorAll('button'); + flushSync(() => { + btn2.click(); + }); + assert.deepEqual(warnings, []); + flushSync(() => { + btn.click(); + }); + flushSync(() => { + btn2.click(); + }); + assert.deepEqual(warnings, []); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/ownership-invalid-mutation-bindable-fallback/main.svelte b/packages/svelte/tests/runtime-runes/samples/ownership-invalid-mutation-bindable-fallback/main.svelte new file mode 100644 index 0000000000..282afb1771 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/ownership-invalid-mutation-bindable-fallback/main.svelte @@ -0,0 +1,5 @@ + + +