fix: eagerly unsubscribe when store is changed (#10727)

fixes #9346

Author: dummdidumm

.changeset/short-countries-rush.md

@@ -0,0 +1,5 @@
+---
+"svelte": patch
+---
+
+fix: eagerly unsubscribe when store is changed

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

@@ -393,28 +393,37 @@ export function serialize_set_binding(node, context, fallback, options) {
 				return b.call(left, value);
 			} else if (is_store) {
 				return b.call('$.store_set', serialize_get_binding(b.id(left_name), state), value);
-			} else if (binding.kind === 'state') {
-				return b.call(
-					'$.set',
-					b.id(left_name),
-					context.state.analysis.runes &&
-						!options?.skip_proxy_and_freeze &&
-						should_proxy_or_freeze(value, context.state.scope)
-						? b.call('$.proxy', value)
-						: value
-				);
-			} else if (binding.kind === 'frozen_state') {
-				return b.call(
-					'$.set',
-					b.id(left_name),
-					context.state.analysis.runes &&
-						!options?.skip_proxy_and_freeze &&
-						should_proxy_or_freeze(value, context.state.scope)
-						? b.call('$.freeze', value)
-						: value
-				);
 			} else {
-				return b.call('$.set', b.id(left_name), value);
+				let call;
+				if (binding.kind === 'state') {
+					call = b.call(
+						'$.set',
+						b.id(left_name),
+						context.state.analysis.runes &&
+							!options?.skip_proxy_and_freeze &&
+							should_proxy_or_freeze(value, context.state.scope)
+							? b.call('$.proxy', value)
+							: value
+					);
+				} else if (binding.kind === 'frozen_state') {
+					call = b.call(
+						'$.set',
+						b.id(left_name),
+						context.state.analysis.runes &&
+							!options?.skip_proxy_and_freeze &&
+							should_proxy_or_freeze(value, context.state.scope)
+							? b.call('$.freeze', value)
+							: value
+					);
+				} else {
+					call = b.call('$.set', b.id(left_name), value);
+				}
+
+				if (state.scope.get(`$${left.name}`)?.kind === 'store_sub') {
+					return b.call('$.store_unsub', call, b.literal(`$${left.name}`), b.id('$$subscriptions'));
+				} else {
+					return call;
+				}
 			}
 		} else {
 			if (is_store) {

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

@@ -47,6 +47,27 @@ export function store_get(store, store_name, stores) {
 	return value === UNINITIALIZED ? entry.last_value : value;
 }
 
+/**
+ * Unsubscribe from a store if it's not the same as the one in the store references container.
+ * We need this in addition to `store_get` because someone could unsubscribe from a store but
+ * then never subscribe to the new one (if any), causing the subscription to stay open wrongfully.
+ * @param {import('#client').Store<any> | null | undefined} store
+ * @param {string} store_name
+ * @param {import('#client').StoreReferencesContainer} stores
+ */
+export function store_unsub(store, store_name, stores) {
+	/** @type {import('#client').StoreReferencesContainer[''] | undefined} */
+	let entry = stores[store_name];
+
+	if (entry && entry.store !== store) {
+		// Don't reset store yet, so that store_get above can resubscribe to new store if necessary
+		entry.unsubscribe();
+		entry.unsubscribe = noop;
+	}
+
+	return store;
+}
+
 /**
  * @template V
  * @param {import('#client').Store<V> | null | undefined} store

packages/svelte/tests/runtime-runes/samples/store-unsubscribe-not-referenced-after/_config.js

@@ -0,0 +1,52 @@
+import { tick } from 'svelte';
+import { ok, test } from '../../test';
+
+// Test that the store is unsubscribed from, even if it's not referenced once the store itself is set to null
+export default test({
+	async test({ target, assert }) {
+		assert.htmlEqual(
+			target.innerHTML,
+			`<input type="number"> <p>0</p> <button>add watcher</button>`
+		);
+
+		target.querySelector('button')?.click();
+		await tick();
+		assert.htmlEqual(
+			target.innerHTML,
+			`<input type="number"> <p>1</p> 1 <button>remove watcher</button>`
+		);
+
+		const input = target.querySelector('input');
+		ok(input);
+
+		input.stepUp();
+		input.dispatchEvent(new Event('input', { bubbles: true }));
+		await tick();
+		assert.htmlEqual(
+			target.innerHTML,
+			`<input type="number"> <p>2</p> 2 <button>remove watcher</button>`
+		);
+
+		target.querySelector('button')?.click();
+		await tick();
+		assert.htmlEqual(
+			target.innerHTML,
+			`<input type="number"> <p>2</p> <button>add watcher</button>`
+		);
+
+		input.stepUp();
+		input.dispatchEvent(new Event('input', { bubbles: true }));
+		await tick();
+		assert.htmlEqual(
+			target.innerHTML,
+			`<input type="number"> <p>2</p> <button>add watcher</button>`
+		);
+
+		target.querySelector('button')?.click();
+		await tick();
+		assert.htmlEqual(
+			target.innerHTML,
+			`<input type="number"> <p>3</p> 3 <button>remove watcher</button>`
+		);
+	}
+});

packages/svelte/tests/runtime-runes/samples/store-unsubscribe-not-referenced-after/main.svelte

@@ -0,0 +1,24 @@
+<script>
+	import { writable, derived } from "svelte/store";
+
+	const obj = writable({ a: 1 });
+	let count = $state(0);
+	let watcherA = $state();
+
+	function watch (prop) {
+		return derived(obj, (o) => {
+			count++;
+			return o[prop];
+		});
+	}
+</script>
+
+<input type="number" bind:value={$obj.a}>
+<p>{count}</p>
+
+{#if watcherA}
+	{$watcherA}
+	<button on:click={() => watcherA = null}>remove watcher</button>
+{:else}
+	<button on:click={() => watcherA = watch("a")}>add watcher</button>
+{/if}