fix: make media bindings more robust (#12206)

The media bindings where fragile because the "prevent rerun if update in progress"-logic was flawed: It didn't (re)set the `updating` flag correctly, because it assumed an event and a render effect would always directly follow each other, with one always being first - but that's not true.
It's much more robust to instead compare the value with what's currently present in the DOM. For the very fast-firing current-time-binding a variable is used to not invoke the DOM getter as much, for the others this is not necessary.
Lastly, the playback-rate-binding contained another bug where the listener was setup inside the effect and therefore reinitialized on each binding change - it needs to move into a different effect.

Author: dummdidumm

.changeset/popular-feet-rule.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: make media bindings more robust

packages/svelte/src/internal/client/dom/elements/bindings/media.js

@@ -22,7 +22,8 @@ function time_ranges_to_array(ranges) {
 export function bind_current_time(media, get_value, update) {
 	/** @type {number} */
 	var raf_id;
-	var updating = false;
+	/** @type {number} */
+	var value;
 
 	// Ideally, listening to timeupdate would be enough, but it fires too infrequently for the currentTime
 	// binding, which is why we use a raf loop, too. We additionally still listen to timeupdate because
@@ -34,22 +35,21 @@ export function bind_current_time(media, get_value, update) {
 			raf_id = requestAnimationFrame(callback);
 		}
 
-		updating = true;
-		update(media.currentTime);
+		var next_value = media.currentTime;
+		if (value !== next_value) {
+			update((value = next_value));
+		}
 	};
 
 	raf_id = requestAnimationFrame(callback);
 	media.addEventListener('timeupdate', callback);
 
 	render_effect(() => {
-		var value = get_value();
+		var next_value = Number(get_value());
 
-		// through isNaN we also allow number strings, which is more robust
-		if (!updating && !isNaN(/** @type {any} */ (value))) {
-			media.currentTime = /** @type {number} */ (value);
+		if (value !== next_value && !isNaN(/** @type {any} */ (next_value))) {
+			media.currentTime = value = next_value;
 		}
-
-		updating = false;
 	});
 
 	teardown(() => cancelAnimationFrame(raf_id));
@@ -113,22 +113,21 @@ export function bind_ready_state(media, update) {
  * @param {(playback_rate: number) => void} update
  */
 export function bind_playback_rate(media, get_value, update) {
-	var updating = false;
-
-	// Needs to happen after the element is inserted into the dom, else playback will be set back to 1 by the browser.
-	// For hydration we could do it immediately but the additional code is not worth the lost microtask.
+	// Needs to happen after element is inserted into the dom (which is guaranteed by using effect),
+	// else playback will be set back to 1 by the browser
 	effect(() => {
-		var value = get_value();
+		var value = Number(get_value());
 
-		// through isNaN we also allow number strings, which is more robust
-		if (!isNaN(/** @type {any} */ (value)) && value !== media.playbackRate) {
-			updating = true;
-			media.playbackRate = /** @type {number} */ (value);
+		if (value !== media.playbackRate && !isNaN(value)) {
+			media.playbackRate = value;
 		}
+	});
 
+	// Start listening to ratechange events after the element is inserted into the dom,
+	// else playback will be set to 1 by the browser
+	effect(() => {
 		listen(media, ['ratechange'], () => {
-			if (!updating) update(media.playbackRate);
-			updating = false;
+			update(media.playbackRate);
 		});
 	});
 }
@@ -200,9 +199,7 @@ export function bind_paused(media, get_value, update) {
  * @param {(volume: number) => void} update
  */
 export function bind_volume(media, get_value, update) {
-	var updating = false;
 	var callback = () => {
-		updating = true;
 		update(media.volume);
 	};
 
@@ -213,14 +210,11 @@ export function bind_volume(media, get_value, update) {
 	listen(media, ['volumechange'], callback, false);
 
 	render_effect(() => {
-		var value = get_value();
+		var value = Number(get_value());
 
-		// through isNaN we also allow number strings, which is more robust
-		if (!updating && !isNaN(/** @type {any} */ (value))) {
-			media.volume = /** @type {number} */ (value);
+		if (value !== media.volume && !isNaN(value)) {
+			media.volume = value;
 		}
-
-		updating = false;
 	});
 }
 
@@ -230,10 +224,7 @@ export function bind_volume(media, get_value, update) {
  * @param {(muted: boolean) => void} update
  */
 export function bind_muted(media, get_value, update) {
-	var updating = false;
-
 	var callback = () => {
-		updating = true;
 		update(media.muted);
 	};
 
@@ -244,9 +235,8 @@ export function bind_muted(media, get_value, update) {
 	listen(media, ['volumechange'], callback, false);
 
 	render_effect(() => {
-		var value = get_value();
+		var value = !!get_value();
 
-		if (!updating) media.muted = !!value;
-		updating = false;
+		if (media.muted !== value) media.muted = value;
 	});
 }

packages/svelte/tests/runtime-browser/assert.js

@@ -44,6 +44,7 @@ export function equal(a, b, message) {
 /**
  * @param {any} condition
  * @param {string} [message]
+ * @returns {asserts condition}
  */
 export function ok(condition, message) {
 	if (!condition) throw new Error(message || `Expected ${condition} to be truthy`);

packages/svelte/tests/runtime-browser/samples/bind-muted/_config.js

@@ -0,0 +1,29 @@
+import { test, ok } from '../../assert';
+
+export default test({
+	mode: ['client'],
+	async test({ assert, target }) {
+		const audio = target.querySelector('audio');
+		const button = target.querySelector('button');
+		ok(audio);
+
+		assert.equal(audio.muted, false);
+
+		audio.muted = true;
+		audio.dispatchEvent(new CustomEvent('volumechange'));
+		await new Promise((r) => setTimeout(r, 100));
+		assert.equal(audio.muted, true, 'event');
+
+		button?.click();
+		await new Promise((r) => setTimeout(r, 100));
+		assert.equal(audio.muted, false, 'click 1');
+
+		button?.click();
+		await new Promise((r) => setTimeout(r, 100));
+		assert.equal(audio.muted, true, 'click 2');
+
+		button?.click();
+		await new Promise((r) => setTimeout(r, 100));
+		assert.equal(audio.muted, false, 'click 3');
+	}
+});

packages/svelte/tests/runtime-browser/samples/bind-muted/main.svelte

@@ -0,0 +1,6 @@
+<script>
+	let muted = $state(false);
+</script>
+
+<audio bind:muted></audio>
+<button onclick={() => (muted = !muted)}>toggle</button>

packages/svelte/tests/runtime-browser/samples/bind-playbackrate/_config.js

@@ -0,0 +1,29 @@
+import { test, ok } from '../../assert';
+
+export default test({
+	mode: ['client'],
+	async test({ assert, target }) {
+		const audio = target.querySelector('audio');
+		const button = target.querySelector('button');
+		ok(audio);
+
+		assert.equal(audio.playbackRate, 0.5);
+
+		audio.playbackRate = 1.0;
+		audio.dispatchEvent(new CustomEvent('ratechange'));
+		await new Promise((r) => setTimeout(r, 100));
+		assert.equal(audio.playbackRate, 1.0);
+
+		button?.click();
+		await new Promise((r) => setTimeout(r, 100));
+		assert.equal(audio.playbackRate, 2);
+
+		button?.click();
+		await new Promise((r) => setTimeout(r, 100));
+		assert.equal(audio.playbackRate, 3);
+
+		button?.click();
+		await new Promise((r) => setTimeout(r, 100));
+		assert.equal(audio.playbackRate, 4);
+	}
+});

packages/svelte/tests/runtime-browser/samples/bind-playbackrate/main.svelte

@@ -0,0 +1,6 @@
+<script>
+	let playbackRate = $state(0.5);
+</script>
+
+<audio bind:playbackRate></audio>
+<button onclick={() => (playbackRate += 1)}>increment</button>

packages/svelte/tests/runtime-browser/samples/bind-volume/_config.js

@@ -0,0 +1,29 @@
+import { test, ok } from '../../assert';
+
+export default test({
+	mode: ['client'],
+	async test({ assert, target }) {
+		const audio = target.querySelector('audio');
+		const button = target.querySelector('button');
+		ok(audio);
+
+		assert.equal(audio.volume, 0.1);
+
+		audio.volume = 0.2;
+		audio.dispatchEvent(new CustomEvent('volumechange'));
+		await new Promise((r) => setTimeout(r, 100));
+		assert.equal(audio.volume, 0.2);
+
+		button?.click();
+		await new Promise((r) => setTimeout(r, 100));
+		assert.equal(audio.volume, 0.2 + 0.1); // JavaScript can't add floating point numbers correctly
+
+		button?.click();
+		await new Promise((r) => setTimeout(r, 100));
+		assert.equal(audio.volume, 0.2 + 0.1 + 0.1);
+
+		button?.click();
+		await new Promise((r) => setTimeout(r, 100));
+		assert.equal(audio.volume, 0.2 + 0.1 + 0.1 + 0.1);
+	}
+});

packages/svelte/tests/runtime-browser/samples/bind-volume/main.svelte

@@ -0,0 +1,6 @@
+<script>
+	let volume = $state(0.1);
+</script>
+
+<audio bind:volume></audio>
+<button onclick={() => (volume += 0.1)}>increment</button>