feat: warn if binding to a non-reactive property (#12500)

* feat: warn if binding to a non-reactive property

* tweak

Author: Rich-Harris

.changeset/nasty-mayflies-smoke.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+feat: warn if binding to a non-reactive property

packages/svelte/messages/client-warnings/warnings.md

@@ -1,3 +1,9 @@
+## binding_property_non_reactive
+
+> `%binding%` is binding to a non-reactive property
+
+> `%binding%` (%location%) is binding to a non-reactive property
+
 ## hydration_attribute_changed
 
 > The `%attribute%` attribute on `%html%` changed its value between server and client renders. The client value, `%value%`, will be ignored in favour of the server value

packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js

@@ -1,4 +1,6 @@
 /** @import { BlockStatement, CallExpression, Expression, ExpressionStatement, Identifier, Literal, MemberExpression, ObjectExpression, Pattern, Property, Statement, Super, TemplateElement, TemplateLiteral } from 'estree' */
+/** @import { BindDirective } from '#compiler' */
+/** @import { ComponentClientTransformState } from '../types' */
 import {
 	extract_identifiers,
 	extract_paths,
@@ -776,11 +778,15 @@ function serialize_inline_component(node, component_name, context, anchor = cont
 				push_prop(b.init(attribute.name, value));
 			}
 		} else if (attribute.type === 'BindDirective') {
+			const expression = /** @type {Expression} */ (context.visit(attribute.expression));
+
+			if (expression.type === 'MemberExpression' && context.state.options.dev) {
+				context.state.init.push(serialize_validate_binding(context.state, attribute, expression));
+			}
+
 			if (attribute.name === 'this') {
 				bind_this = attribute.expression;
 			} else {
-				const expression = /** @type {Expression} */ (context.visit(attribute.expression));
-
 				if (context.state.options.dev) {
 					binding_initializers.push(
 						b.stmt(b.call(b.id('$.add_owner_effect'), b.thunk(expression), b.id(component_name)))
@@ -2824,6 +2830,17 @@ export const template_visitors = {
 	BindDirective(node, context) {
 		const { state, path, visit } = context;
 		const expression = node.expression;
+
+		if (expression.type === 'MemberExpression' && context.state.options.dev) {
+			context.state.init.push(
+				serialize_validate_binding(
+					context.state,
+					node,
+					/**@type {MemberExpression} */ (visit(expression))
+				)
+			);
+		}
+
 		const getter = b.thunk(/** @type {Expression} */ (visit(expression)));
 		const assignment = b.assignment('=', expression, b.id('$$value'));
 		const setter = b.arrow(
@@ -3230,3 +3247,34 @@ export const template_visitors = {
 	CallExpression: javascript_visitors_runes.CallExpression,
 	VariableDeclaration: javascript_visitors_runes.VariableDeclaration
 };
+
+/**
+ * @param {import('../types.js').ComponentClientTransformState} state
+ * @param {BindDirective} binding
+ * @param {MemberExpression} expression
+ */
+function serialize_validate_binding(state, binding, expression) {
+	const string = state.analysis.source.slice(binding.start, binding.end);
+
+	const get_object = b.thunk(/** @type {Expression} */ (expression.object));
+	const get_property = b.thunk(
+		/** @type {Expression} */ (
+			expression.computed
+				? expression.property
+				: b.literal(/** @type {Identifier} */ (expression.property).name)
+		)
+	);
+
+	const loc = locator(binding.start);
+
+	return b.stmt(
+		b.call(
+			'$.validate_binding',
+			b.literal(string),
+			get_object,
+			get_property,
+			loc && b.literal(loc.line),
+			loc && b.literal(loc.column)
+		)
+	);
+}

packages/svelte/src/internal/client/index.js

@@ -146,6 +146,7 @@ export {
 	hasContext
 } from './runtime.js';
 export {
+	validate_binding,
 	validate_dynamic_component,
 	validate_each_keys,
 	validate_prop_bindings

packages/svelte/src/internal/client/validate.js

@@ -1,7 +1,9 @@
-import { untrack } from './runtime.js';
+import { dev_current_component_function, untrack } from './runtime.js';
 import { get_descriptor, is_array } from '../shared/utils.js';
 import * as e from './errors.js';
 import { FILENAME } from '../../constants.js';
+import { render_effect } from './reactivity/effects.js';
+import * as w from './warnings.js';
 
 /** regex of all html void element names */
 const void_element_names =
@@ -89,3 +91,44 @@ export function validate_prop_bindings($$props, bindable, exports, component) {
 		}
 	}
 }
+
+/**
+ * @param {string} binding
+ * @param {() => Record<string, any>} get_object
+ * @param {() => string} get_property
+ * @param {number} line
+ * @param {number} column
+ */
+export function validate_binding(binding, get_object, get_property, line, column) {
+	var warned = false;
+
+	var filename = dev_current_component_function?.[FILENAME];
+
+	render_effect(() => {
+		if (warned) return;
+
+		var object = get_object();
+		var property = get_property();
+
+		var ran = false;
+
+		// by making the (possibly false, but it would be an extreme edge case) assumption
+		// that a getter has a corresponding setter, we can determine if a property is
+		// reactive by seeing if this effect has dependencies
+		var effect = render_effect(() => {
+			if (ran) return;
+
+			// eslint-disable-next-line @typescript-eslint/no-unused-expressions
+			object[property];
+		});
+
+		ran = true;
+
+		if (effect.deps === null) {
+			var location = filename && `${filename}:${line}:${column}`;
+			w.binding_property_non_reactive(binding, location);
+
+			warned = true;
+		}
+	});
+}

packages/svelte/src/internal/client/warnings.js

@@ -5,6 +5,20 @@ import { DEV } from 'esm-env';
 var bold = 'font-weight: bold';
 var normal = 'font-weight: normal';
 
+/**
+ * `%binding%` (%location%) is binding to a non-reactive property
+ * @param {string} binding
+ * @param {string | undefined | null} [location]
+ */
+export function binding_property_non_reactive(binding, location) {
+	if (DEV) {
+		console.warn(`%c[svelte] binding_property_non_reactive\n%c${location ? `\`${binding}\` (${location}) is binding to a non-reactive property` : `\`${binding}\` is binding to a non-reactive property`}`, bold, normal);
+	} else {
+		// TODO print a link to the documentation
+		console.warn("binding_property_non_reactive");
+	}
+}
+
 /**
  * The `%attribute%` attribute on `%html%` changed its value between server and client renders. The client value, `%value%`, will be ignored in favour of the server value
  * @param {string} attribute

packages/svelte/tests/runtime-runes/samples/binding-property-static/Child.svelte

@@ -0,0 +1,5 @@
+<script lang="ts">
+	let { value = $bindable() }: { value: number } = $props();
+</script>
+
+<p>{value}</p>

packages/svelte/tests/runtime-runes/samples/binding-property-static/_config.js

@@ -0,0 +1,16 @@
+import { test } from '../../test';
+
+export default test({
+	compileOptions: {
+		dev: true
+	},
+
+	async test({ assert, warnings }) {
+		assert.deepEqual(warnings, [
+			`\`bind:value={pojo.value}\` (main.svelte:50:7) is binding to a non-reactive property`,
+			`\`bind:value={frozen.value}\` (main.svelte:51:7) is binding to a non-reactive property`,
+			`\`bind:value={pojo.value}\` (main.svelte:52:7) is binding to a non-reactive property`,
+			`\`bind:value={frozen.value}\` (main.svelte:53:7) is binding to a non-reactive property`
+		]);
+	}
+});

packages/svelte/tests/runtime-runes/samples/binding-property-static/main.svelte

@@ -0,0 +1,61 @@
+<script>
+	import Child from './Child.svelte';
+
+	let pojo = {
+		value: 1
+	};
+
+	let frozen = $state.frozen({
+		value: 2
+	});
+
+	let reactive = $state({
+		value: 3
+	});
+
+	let value = $state(4);
+	let accessors = {
+		get value() {
+			return value;
+		},
+		set value(v) {
+			value = v;
+		}
+	};
+
+	let proxied = $state(5);
+	let proxy = new Proxy(
+		{},
+		{
+			get(target, prop, receiver) {
+				if (prop === 'value') {
+					return proxied;
+				}
+
+				return Reflect.get(target, prop, receiver);
+			},
+			set(target, prop, value, receiver) {
+				if (prop === 'value') {
+					proxied = value;
+					return true;
+				}
+
+				return Reflect.set(target, prop, value, receiver);
+			}
+		}
+	);
+</script>
+
+<!-- should warn -->
+<input bind:value={pojo.value} />
+<input bind:value={frozen.value} />
+<Child bind:value={pojo.value} />
+<Child bind:value={frozen.value} />
+
+<!-- should not warn -->
+<input bind:value={reactive.value} />
+<input bind:value={accessors.value} />
+<input bind:value={proxy.value} />
+<Child bind:value={reactive.value} />
+<Child bind:value={accessors.value} />
+<Child bind:value={proxy.value} />