chore: better store subscriptions (#12277)

* use existing teardown function

* set up stores and unsubscriptions in one go

* we never set last_value, this is gibberish

* simplify

* tidy up

* inline

* more

.changeset/thirty-flies-push.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+chore: tidy up store logic

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

@@ -154,19 +154,12 @@ export function client_component(source, analysis, options) {
 		}
 		if (binding.kind === 'store_sub') {
 			if (store_setup.length === 0) {
-				store_setup.push(
-					b.const('$$subscriptions', b.object([])),
-					b.stmt(b.call('$.unsubscribe_on_destroy', b.id('$$subscriptions')))
-				);
+				store_setup.push(b.const('$$stores', b.call('$.setup_stores')));
 			}
+
 			// We're creating an arrow function that gets the store value which minifies better for two or more references
 			const store_reference = serialize_get_binding(b.id(name.slice(1)), instance_state);
-			const store_get = b.call(
-				'$.store_get',
-				store_reference,
-				b.literal(name),
-				b.id('$$subscriptions')
-			);
+			const store_get = b.call('$.store_get', store_reference, b.literal(name), b.id('$$stores'));
 			store_setup.push(
 				b.const(
 					binding.node,

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

@@ -345,7 +345,7 @@ export function serialize_set_binding(node, context, fallback, prefix, options)
 				}
 
 				if (state.scope.get(`$${left.name}`)?.kind === 'store_sub') {
-					return b.call('$.store_unsub', call, b.literal(`$${left.name}`), b.id('$$subscriptions'));
+					return b.call('$.store_unsub', call, b.literal(`$${left.name}`), b.id('$$stores'));
 				} else {
 					return call;
 				}

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

@@ -2438,7 +2438,7 @@ export const template_visitors = {
 					b.thunk(b.sequence(indirect_dependencies))
 				);
 				const invalidate_store = store_to_invalidate
-					? b.call('$.invalidate_store', b.id('$$subscriptions'), b.literal(store_to_invalidate))
+					? b.call('$.invalidate_store', b.id('$$stores'), b.literal(store_to_invalidate))
 					: undefined;
 
 				const sequence = [];

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

@@ -111,10 +111,10 @@ export {
 export {
 	invalidate_store,
 	mutate_store,
+	setup_stores,
 	store_get,
 	store_set,
 	store_unsub,
-	unsubscribe_on_destroy,
 	update_pre_store,
 	update_store
 } from './reactivity/store.js';

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

@@ -1,10 +1,10 @@
-/** @import { StoreReferencesContainer, Source } from '#client' */
+/** @import { StoreReferencesContainer } from '#client' */
 /** @import { Store } from '#shared' */
 import { subscribe_to_store } from '../../../store/utils.js';
 import { noop } from '../../shared/utils.js';
 import { UNINITIALIZED } from '../../../constants.js';
-import { get, untrack } from '../runtime.js';
-import { effect } from './effects.js';
+import { get } from '../runtime.js';
+import { teardown } from './effects.js';
 import { mutable_source, set } from './sources.js';
 
 /**
@@ -18,30 +18,25 @@ import { mutable_source, set } from './sources.js';
  * @returns {V}
  */
 export function store_get(store, store_name, stores) {
-	/** @type {StoreReferencesContainer[''] | undefined} */
-	let entry = stores[store_name];
-	const is_new = entry === undefined;
-
-	if (is_new) {
-		entry = {
+	const entry = (stores[store_name] ??= {
 		store: null,
-			last_value: null,
-			value: mutable_source(UNINITIALIZED),
+		source: mutable_source(UNINITIALIZED),
 		unsubscribe: noop
-		};
-		stores[store_name] = entry;
-	}
+	});
 
-	if (is_new || entry.store !== store) {
+	if (entry.store !== store) {
 		entry.unsubscribe();
 		entry.store = store ?? null;
-		entry.unsubscribe = connect_store_to_signal(store, entry.value);
+
+		if (store == null) {
+			set(entry.source, undefined);
+			entry.unsubscribe = noop;
+		} else {
+			entry.unsubscribe = subscribe_to_store(store, (v) => set(entry.source, v));
+		}
 	}
 
-	const value = get(entry.value);
-	// This could happen if the store was cleaned up because the component was destroyed and there's a leak on the user side.
-	// In that case we don't want to fail with a cryptic Symbol error, but rather return the last value we got.
-	return value === UNINITIALIZED ? entry.last_value : value;
+	return get(entry.source);
 }
 
 /**
@@ -65,20 +60,6 @@ export function store_unsub(store, store_name, stores) {
 	return store;
 }
 
-/**
- * @template V
- * @param {Store<V> | null | undefined} store
- * @param {Source<V>} source
- */
-function connect_store_to_signal(store, source) {
-	if (store == null) {
-		set(source, undefined);
-		return noop;
-	}
-
-	return subscribe_to_store(store, (v) => set(source, v));
-}
-
 /**
  * Sets the new value of a store and returns that value.
  * @template V
@@ -96,24 +77,28 @@ export function store_set(store, value) {
  * @param {string} store_name
  */
 export function invalidate_store(stores, store_name) {
-	const store = stores[store_name];
-	if (store.store) {
-		store_set(store.store, store.value.v);
+	var entry = stores[store_name];
+	if (entry.store !== null) {
+		store_set(entry.store, entry.source.v);
 	}
 }
 
 /**
  * Unsubscribes from all auto-subscribed stores on destroy
- * @param {StoreReferencesContainer} stores
+ * @returns {StoreReferencesContainer}
  */
-export function unsubscribe_on_destroy(stores) {
-	on_destroy(() => {
-		let store_name;
-		for (store_name in stores) {
+export function setup_stores() {
+	/** @type {StoreReferencesContainer} */
+	const stores = {};
+
+	teardown(() => {
+		for (var store_name in stores) {
 			const ref = stores[store_name];
 			ref.unsubscribe();
 		}
 	});
+
+	return stores;
 }
 
 /**
@@ -150,12 +135,3 @@ export function update_pre_store(store, store_value, d = 1) {
 	store.set(value);
 	return value;
 }
-
-/**
- * Schedules a callback to run immediately before the component is unmounted.
- * @param {() => any} fn
- * @returns {void}
- */
-function on_destroy(fn) {
-	effect(() => () => untrack(fn));
-}

packages/svelte/src/internal/client/types.d.ts

@@ -148,9 +148,8 @@ export type StoreReferencesContainer = Record<
 	string,
 	{
 		store: Store<any> | null;
-		last_value: any;
 		unsubscribe: Function;
-		value: Source<any>;
+		source: Source<any>;
 	}
 >;