breaking: apply fallback value every time in runes mode (#10797)

* breaking: apply fallback value every time in runes mode

closes #9948

* apply fallback value in setter

* encapsulate fallback logic

* we should keep this logic out of b.set, since it's very specific to accessors

* oops

---------

Co-authored-by: Rich Harris <[email protected]>

Author: dummdidumm

.changeset/small-spiders-fail.md

@@ -0,0 +1,5 @@
+---
+"svelte": patch
+---
+
+breaking: apply fallback value every time in runes mode

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

@@ -255,10 +255,23 @@ export function client_component(source, analysis, options) {
 
 			const key = binding.prop_alias ?? name;
 
-			properties.push(
-				b.get(key, [b.return(b.call(b.id(name)))]),
-				b.set(key, [b.stmt(b.call(b.id(name), b.id('$$value'))), b.stmt(b.call('$.flushSync'))])
-			);
+			const getter = b.get(key, [b.return(b.call(b.id(name)))]);
+
+			const setter = b.set(key, [
+				b.stmt(b.call(b.id(name), b.id('$$value'))),
+				b.stmt(b.call('$.flushSync'))
+			]);
+
+			if (analysis.runes && binding.initial) {
+				// turn `set foo($$value)` into `set foo($$value = expression)`
+				setter.value.params[0] = {
+					type: 'AssignmentPattern',
+					left: b.id('$$value'),
+					right: /** @type {import('estree').Expression} */ (binding.initial)
+				};
+			}
+
+			properties.push(getter, setter);
 		}
 	}
 

packages/svelte/src/compiler/utils/builders.js

@@ -210,7 +210,7 @@ export function function_declaration(id, params, body) {
 /**
  * @param {string} name
  * @param {import('estree').Statement[]} body
- * @returns {import('estree').Property}
+ * @returns {import('estree').Property & { value: import('estree').FunctionExpression}}}
  */
 export function get(name, body) {
 	return prop('get', key(name), function_builder(null, [], block(body)));
@@ -305,11 +305,12 @@ export function object_pattern(properties) {
 }
 
 /**
+ * @template {import('estree').Expression} Value
  * @param {'init' | 'get' | 'set'} kind
  * @param {import('estree').Expression} key
- * @param {import('estree').Expression} value
+ * @param {Value} value
  * @param {boolean} computed
- * @returns {import('estree').Property}
+ * @returns {import('estree').Property & { value: Value }}
  */
 export function prop(kind, key, value, computed = false) {
 	return { type: 'Property', kind, key, value, method: false, shorthand: false, computed };
@@ -355,7 +356,7 @@ export function sequence(expressions) {
 /**
  * @param {string} name
  * @param {import('estree').Statement[]} body
- * @returns {import('estree').Property}
+ * @returns {import('estree').Property & { value: import('estree').FunctionExpression}}
  */
 export function set(name, body) {
 	return prop('set', key(name), function_builder(null, [id('$$value')], block(body)));

packages/svelte/src/internal/client/reactivity/props.js

@@ -8,7 +8,7 @@ import {
 import { get_descriptor, is_function } from '../utils.js';
 import { mutable_source, set } from './sources.js';
 import { derived } from './deriveds.js';
-import { get, inspect_fn, is_signals_recorded } from '../runtime.js';
+import { get, inspect_fn, is_signals_recorded, untrack } from '../runtime.js';
 import { safe_equals, safe_not_equal } from './equality.js';
 
 /**
@@ -133,16 +133,30 @@ export function spread_props(...props) {
  * @param {Record<string, unknown>} props
  * @param {string} key
  * @param {number} flags
- * @param {V | (() => V)} [initial]
+ * @param {V | (() => V)} [fallback]
  * @returns {(() => V | ((arg: V) => V) | ((arg: V, mutation: boolean) => V))}
  */
-export function prop(props, key, flags, initial) {
+export function prop(props, key, flags, fallback) {
 	var immutable = (flags & PROPS_IS_IMMUTABLE) !== 0;
 	var runes = (flags & PROPS_IS_RUNES) !== 0;
+	var lazy = (flags & PROPS_IS_LAZY_INITIAL) !== 0;
+
 	var prop_value = /** @type {V} */ (props[key]);
 	var setter = get_descriptor(props, key)?.set;
 
-	if (prop_value === undefined && initial !== undefined) {
+	var fallback_value = /** @type {V} */ (fallback);
+	var fallback_dirty = true;
+
+	var get_fallback = () => {
+		if (lazy && fallback_dirty) {
+			fallback_dirty = false;
+			fallback_value = untrack(/** @type {() => V} */ (fallback));
+		}
+
+		return fallback_value;
+	};
+
+	if (prop_value === undefined && fallback !== undefined) {
 		if (setter && runes) {
 			// TODO consolidate all these random runtime errors
 			throw new Error(
@@ -153,19 +167,22 @@ export function prop(props, key, flags, initial) {
 			);
 		}
 
-		// @ts-expect-error would need a cumbersome method overload to type this
-		if ((flags & PROPS_IS_LAZY_INITIAL) !== 0) initial = initial();
-
-		prop_value = /** @type {V} */ (initial);
-
+		prop_value = get_fallback();
 		if (setter) setter(prop_value);
 	}
 
-	var getter = () => {
-		var value = /** @type {V} */ (props[key]);
-		if (value !== undefined) initial = undefined;
-		return value === undefined ? /** @type {V} */ (initial) : value;
-	};
+	var getter = runes
+		? () => {
+				var value = /** @type {V} */ (props[key]);
+				if (value === undefined) return get_fallback();
+				fallback_dirty = true;
+				return value;
+			}
+		: () => {
+				var value = /** @type {V} */ (props[key]);
+				if (value !== undefined) fallback_value = /** @type {V} */ (undefined);
+				return value === undefined ? fallback_value : value;
+			};
 
 	// easy mode — prop is never written to
 	if ((flags & PROPS_IS_UPDATED) === 0) {

packages/svelte/tests/runtime-runes/samples/props-default-value-behavior/_config.js

@@ -1,6 +1,6 @@
 import { test } from '../../test';
 
-// Tests that default values apply only initially and that they propagate back correctly for bindings
+// Tests that default values apply every time and that they propagate back correctly for bindings
 export default test({
 	// set accessors to false so that $.prop() is also created
 	accessors: false,
@@ -267,7 +267,7 @@ export default test({
 			<p>props undefined:</p>
 			<p>
 				readonly:
-				readonlyWithDefault:
+				readonlyWithDefault: 1
 				binding:
 			</p>
 			<button>set bindings to 5</button>
@@ -276,7 +276,7 @@ export default test({
 			<p>props defined:</p>
 			<p>
 				readonly:
-				readonlyWithDefault:
+				readonlyWithDefault: 1
 				binding:
 			</p>
 			<button>set bindings to 5</button>
@@ -285,7 +285,7 @@ export default test({
 			<p>bindings undefined:</p>
 			<p>
 				readonly:
-				readonlyWithDefault:
+				readonlyWithDefault: 1
 				binding:
 			</p>
 			<button>set bindings to 5</button>
@@ -294,7 +294,7 @@ export default test({
 			<p>bindings defined:</p>
 			<p>
 				readonly:
-				readonlyWithDefault:
+				readonlyWithDefault: 1
 				binding:
 			</p>
 			<button>set bindings to 5</button>

packages/svelte/tests/runtime-runes/samples/props-default-value-lazy-accessors/_config.js

@@ -0,0 +1,29 @@
+import { test } from '../../test';
+
+// Tests that default values only fire lazily when the prop is undefined, and every time
+export default test({
+	props: {
+		p0: 0,
+		p1: 0,
+		p2: 0,
+		p3: 0
+	},
+	html: `<p>props: 0 0 0 0 1 1 1 1</p><p>log: nested.fallback_value,fallback_fn`,
+	async test({ assert, target, component }) {
+		component.p0 = undefined;
+		component.p1 = undefined;
+		component.p2 = undefined;
+		component.p3 = undefined;
+		// Nuance: these are already undefined in the props, but we're setting them to undefined again,
+		// which calls the fallback value again, even if it will result in the same value. There's no way
+		// to prevent this, and in practise it won't matter - and you shouldn't use accessors in runes mode anyway.
+		component.p4 = undefined;
+		component.p5 = undefined;
+		component.p6 = undefined;
+		component.p7 = undefined;
+		assert.htmlEqual(
+			target.innerHTML,
+			`<p>props: 1 1 1 1 1 1 1 1</p><p>log: nested.fallback_value,fallback_fn,nested.fallback_value,fallback_fn,nested.fallback_value,fallback_fn`
+		);
+	}
+});

packages/svelte/tests/runtime-runes/samples/props-default-value-lazy-accessors/main.svelte

@@ -0,0 +1,29 @@
+<script>
+	let log = $state([]);
+
+	const fallback_value = 1;
+	const nested = {
+		get fallback_value() {
+			log.push('nested.fallback_value');
+			return fallback_value;
+		}
+	}
+	const fallback_fn = () => {
+		log.push('fallback_fn');
+		return fallback_value;
+	}
+
+	const {
+		p0 = 1,
+		p1 = fallback_value,
+		p2 = nested.fallback_value,
+		p3 = fallback_fn(),
+		p4 = 1,
+		p5 = fallback_value,
+		p6 = nested.fallback_value,
+		p7 = fallback_fn()
+	} = $props();
+</script>
+
+<p>props: {p0} {p1} {p2} {p3} {p4} {p5} {p6} {p7}</p>
+<p>log: {log}</p>

packages/svelte/tests/runtime-runes/samples/props-default-value-lazy/_config.js

@@ -1,23 +1,24 @@
+import { tick } from 'svelte';
 import { test } from '../../test';
 
-// Tests that default values only fire lazily when the prop is undefined, and only initially
+// Tests that default values only fire lazily when the prop is undefined, and every time
 export default test({
-	props: {
-		p0: 0,
-		p1: 0,
-		p2: 0,
-		p3: 0
-	},
-	html: `<p>props: 0 0 0 0 1 1 1 1</p><p>log: nested.fallback_value,fallback_fn`,
-	async test({ assert, target, component }) {
-		component.p0 = undefined;
-		component.p1 = undefined;
-		component.p2 = undefined;
-		component.p3 = undefined;
-		component.p4 = undefined;
-		component.p5 = undefined;
-		component.p6 = undefined;
-		component.p7 = undefined;
-		assert.htmlEqual(target.innerHTML, `<p>props: </p><p>log: nested.fallback_value,fallback_fn`);
+	html: `
+	<p>props: 0 0 0 0 1 1 1 1</p>
+	<p>log: nested.fallback_value,fallback_fn</p>
+	<button>Set all to undefined</button>
+	`,
+	async test({ assert, target }) {
+		const btn = target.querySelector('button');
+		btn?.click();
+		await tick();
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+			<p>props: 1 1 1 1 1 1 1 1</p>
+			<p>log: nested.fallback_value,fallback_fn,nested.fallback_value,fallback_fn</p>
+			<button>Set all to undefined</button>
+			`
+		);
 	}
 });

packages/svelte/tests/runtime-runes/samples/props-default-value-lazy/main.svelte

@@ -1,31 +1,25 @@
 <script>
-	let log = $state([]);
+	import Sub from "./sub.svelte";
 
-	const fallback_value = 1;
-	const nested = {
-		get fallback_value() {
-			log.push('nested.fallback_value');
-			log = log;
-			return fallback_value;
-		}
-	}
-	const fallback_fn = () => {
-		log.push('fallback_fn');
-			log = log;
-		return fallback_value;
-	}
-
-	const {
-		p0 = 1,
-		p1 = fallback_value,
-		p2 = nested.fallback_value,
-		p3 = fallback_fn(),
-		p4 = 1,
-		p5 = fallback_value,
-		p6 = nested.fallback_value,
-		p7 = fallback_fn()
-	} = $props();
+	let p0 = $state(0);
+	let p1 = $state(0);
+	let p2 = $state(0);
+	let p3 = $state(0);
+	let p4 = $state();
+	let p5 = $state();
+	let p6 = $state();
+	let p7 = $state();
 </script>
 
-<p>props: {p0} {p1} {p2} {p3} {p4} {p5} {p6} {p7}</p>
-<p>log: {log}</p>
+<Sub {p0} {p1} {p2} {p3} {p4} {p5} {p6} {p7} />
+
+<button onclick={() => {
+  p0 = undefined;
+  p1 = undefined;
+  p2 = undefined;
+  p3 = undefined;
+  p4 = undefined;
+  p5 = undefined;
+  p6 = undefined;
+  p7 = undefined;
+}}>Set all to undefined</button>

packages/svelte/tests/runtime-runes/samples/props-default-value-lazy/sub.svelte

@@ -0,0 +1,29 @@
+<script>
+	let log = $state([]);
+
+	const fallback_value = 1;
+	const nested = {
+		get fallback_value() {
+			log.push('nested.fallback_value');
+			return fallback_value;
+		}
+	}
+	const fallback_fn = () => {
+		log.push('fallback_fn');
+		return fallback_value;
+	}
+
+	const {
+		p0 = 1,
+		p1 = fallback_value,
+		p2 = nested.fallback_value,
+		p3 = fallback_fn(),
+		p4 = 1,
+		p5 = fallback_value,
+		p6 = nested.fallback_value,
+		p7 = fallback_fn()
+	} = $props();
+</script>
+
+<p>props: {p0} {p1} {p2} {p3} {p4} {p5} {p6} {p7}</p>
+<p>log: {log}</p>

packages/svelte/tests/runtime-runes/samples/props-spread-fallback/_config.js

@@ -48,7 +48,7 @@ export default test({
 			`
 				<button>change propA</button>
 				<button>change propB</button>
-				<p>true</p>
+				<p>true fallback</p>
 			`
 		);
 	}