fix: prevent ownership warnings if the fallback of a bindable is used

Author: paoloricciuti

.changeset/wise-turkeys-yell.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: prevent ownership warnings if the fallback of a bindable is used

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)
 	);

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('');

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,

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

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);

packages/svelte/tests/runtime-runes/samples/ownership-invalid-binding-bindable-fallback/Child.svelte

@@ -0,0 +1,5 @@
+<script>
+	const { test = $bindable() } = $props();
+</script>
+
+{test}

packages/svelte/tests/runtime-runes/samples/ownership-invalid-binding-bindable-fallback/Parent.svelte

@@ -0,0 +1,7 @@
+<script>
+	import Child from './Child.svelte';
+
+	let { test = $bindable({}) } = $props();
+</script>
+
+<Child bind:test />

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, []);
+	}
+});

packages/svelte/tests/runtime-runes/samples/ownership-invalid-binding-bindable-fallback/main.svelte

@@ -0,0 +1,5 @@
+<script>
+	import Parent from './Parent.svelte';
+</script>
+
+<Parent />

packages/svelte/tests/runtime-runes/samples/ownership-invalid-mutation-bindable-fallback/Parent.svelte

@@ -0,0 +1,8 @@
+<script>
+	let { test = $bindable({}) } = $props();
+</script>
+
+<button onclick={()=>test = {}}></button>
+<button onclick={()=>test.test = {}}></button>
+
+{test}

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, []);
+	}
+});

packages/svelte/tests/runtime-runes/samples/ownership-invalid-mutation-bindable-fallback/main.svelte

@@ -0,0 +1,5 @@
+<script>
+	import Parent from './Parent.svelte';
+</script>
+
+<Parent />