alternative fix

sveltejs · Rich-Harris · Feb 11, 2024 · Feb 15, 2024 · Feb 20, 2024 · Feb 20, 2024
commit 1e62e943741ba0c84d8504c10ebc65c6f7798ca0
diff --git a/packages/svelte/src/store/index.js b/packages/svelte/src/store/index.js
@@ -1,6 +1,8 @@
 import { noop, run } from '../internal/common.js';
 import { subscribe_to_store } from './utils.js';
 
+const SUBSCRIBERS_SYMBOL = Symbol('subscribers');
+
 /**
  * @type {Array<import('./private').SubscribeInvalidateTuple<any> | any>}
  */
@@ -16,8 +18,11 @@ const subscriber_queue = [];
  * @returns {import('./public').Readable<T>}
  */
 export function readable(value, start) {
+	const w = writable(value, start);
 	return {
-		subscribe: writable(value, start).subscribe
+		// @ts-expect-error we don't want this to be on the public types
+		[SUBSCRIBERS_SYMBOL]: w[SUBSCRIBERS_SYMBOL],
+		subscribe: w.subscribe
 	};
 }
 
@@ -46,82 +51,32 @@ export function safe_not_equal(a, b) {
 export function writable(value, start = noop) {
 	/** @type {import('./public').Unsubscriber | null} */
 	let stop = null;
-	let invalidated = false;
 
 	/** @type {Set<import('./private').SubscribeInvalidateTuple<T>>} */
 	const subscribers = new Set();
 
-	function invalidate() {
-		if (invalidated)
-			return;
-
-		if (stop) {
-			invalidated = true;
-
-			// immediately signal each subscriber as invalid
-			for (const subscriber of subscribers) {
-				subscriber[1]();
-			}
-		}
-	}
-
-	function revalidate() {
-		if (!invalidated)
-			return;
-
-		invalidated = false;
-
-		if (stop) {
-			// immediately signal each subscriber as revalidated
-			for (const subscriber of subscribers) {
-				subscriber[2]();
-			}
-		}
-	}
-
 	/**
 	 * @param {T} new_value
 	 * @returns {void}
 	 */
 	function set(new_value) {
-		if (safe_not_equal(value, new_value)) {
-			value = new_value;
-			invalidated = false;
-
-			if (stop) {
-				// store is ready
-				const run_queue = !subscriber_queue.length;
-				for (const subscriber of subscribers) {
-					subscriber[1]();
-					subscriber_queue.push(subscriber, value);
-				}
-				if (run_queue) {
-					for (let i = 0; i < subscriber_queue.length; i += 2) {
-						subscriber_queue[i][0](subscriber_queue[i + 1]);
-					}
-					subscriber_queue.length = 0;
+		value = new_value;
+		if (stop) {
+			// store is ready
+			const run_queue = !subscriber_queue.length;
+			for (const subscriber of subscribers) {
+				subscriber[1]();
+				subscriber_queue.push(subscriber, value);
+			}
+			if (run_queue) {
+				for (let i = 0; i < subscriber_queue.length; i += 2) {
+					subscriber_queue[i][0](subscriber_queue[i + 1]);
 				}
+				subscriber_queue.length = 0;
 			}
 		}
-		else
-			revalidate();
 	}
 
-
-	const complex_set = Object.assign(
-		/**
-		 * @param {T} new_value
-		 * @returns {void}
-		 */
-		(new_value) => set(new_value),
-		{
-			set,
-			update,
-			invalidate,
-			revalidate
-		}
-	)
-
 	/**
 	 * @param {import('./public').Updater<T>} fn
 	 * @returns {void}
@@ -133,15 +88,14 @@ export function writable(value, start = noop) {
 	/**
 	 * @param {import('./public').Subscriber<T>} run
 	 * @param {import('./private').Invalidator<T>} [invalidate]
-	 * @param {import('./private').Revalidator<T>} [revalidate]
 	 * @returns {import('./public').Unsubscriber}
 	 */
-	function subscribe(run, invalidate = noop, revalidate = noop) {
+	function subscribe(run, invalidate = noop) {
 		/** @type {import('./private').SubscribeInvalidateTuple<T>} */
-		const subscriber = [run, invalidate, revalidate];
+		const subscriber = [run, invalidate];
 		subscribers.add(subscriber);
 		if (subscribers.size === 1) {
-			stop = start(complex_set, update) || noop;
+			stop = start(set, update) || noop;
 		}
 		run(/** @type {T} */ (value));
 		return () => {
@@ -152,7 +106,23 @@ export function writable(value, start = noop) {
 			}
 		};
 	}
-	return { set, update, subscribe };
+
+	/** @param {T} new_value */
+	function set_if_changed(new_value) {
+		if (safe_not_equal(value, new_value)) {
+			set(new_value);
+		}
+	}
+
+	return {
+		// @ts-expect-error
+		[SUBSCRIBERS_SYMBOL]: subscribers,
+		set: set_if_changed,
+		update: (fn) => {
+			set_if_changed(fn(/** @type {T} */ (value)));
+		},
+		subscribe
+	};
 }
 
 /**
@@ -205,19 +175,16 @@ export function derived(stores, fn, initial_value) {
 		throw new Error('derived() expects stores as input, got a falsy value');
 	}
 	const auto = fn.length < 2;
-	return readable(initial_value, (set, update) => {
-		const { invalidate, revalidate } = set;
+	const r = readable(initial_value, (set, update) => {
 		let started = false;
 		/** @type {T[]} */
 		const values = [];
 		let pending = 0;
-		let changed = stores_array.length === 0;
 		let cleanup = noop;
 		const sync = () => {
-			if (!changed || pending) {
+			if (pending) {
 				return;
 			}
-			changed = false;
 			cleanup();
 			const result = fn(single ? values[0] : values, set, update);
 			if (auto) {
@@ -231,24 +198,17 @@ export function derived(stores, fn, initial_value) {
 				store,
 				(value) => {
 					values[i] = value;
-					changed = true;
 					pending &= ~(1 << i);
 					if (started) {
 						sync();
 					}
 				},
 				() => {
 					pending |= 1 << i;
-					invalidate();
-				},
-				() => {
-					pending &= ~(1 << i);
-					if (!changed && !pending) {
-						revalidate();
-					}
-					else
-					if (started) {
-						sync();
+
+					// @ts-expect-error
+					for (const s of r[SUBSCRIBERS_SYMBOL]) {
+						s[1]();
 					}
 				}
 			)
@@ -264,6 +224,7 @@ export function derived(stores, fn, initial_value) {
 			started = false;
 		};
 	});
+	return r;
 }
 
 /**

diff --git a/packages/svelte/src/store/private.d.ts b/packages/svelte/src/store/private.d.ts
@@ -3,11 +3,8 @@ import { Readable, Subscriber } from './public.js';
 /** Cleanup logic callback. */
 export type Invalidator<T> = (value?: T) => void;
 
-/** Cleanup logic callback. */
-export type Revalidator<T> = (value?: T) => void;
-
 /** Pair of subscriber and invalidator. */
-export type SubscribeInvalidateTuple<T> = [Subscriber<T>, Invalidator<T>, Revalidator<T>];
+export type SubscribeInvalidateTuple<T> = [Subscriber<T>, Invalidator<T>];
 
 /** One or more `Readable`s. */
 export type Stores =

diff --git a/packages/svelte/src/store/public.d.ts b/packages/svelte/src/store/public.d.ts
@@ -1,4 +1,4 @@
-import type { Invalidator, Revalidator } from './private.js';
+import type { Invalidator } from './private.js';
 
 /** Callback to inform of a value updates. */
 export type Subscriber<T> = (value: T) => void;
@@ -13,18 +13,13 @@ export type Updater<T> = (value: T) => T;
  * Start and stop notification callbacks.
  * This function is called when the first subscriber subscribes.
  *
- * @param {((value: T) => void) & { set: (value: T) => void, update: (fn: Updater<T>) => void, invalidate: () => void })} set Function that sets the value of the store.
+ * @param {(value: T) => void} set Function that sets the value of the store.
  * @param {(value: Updater<T>) => void} update Function that sets the value of the store after passing the current value to the update function.
  * @returns {void | (() => void)} Optionally, a cleanup function that is called when the last remaining
  * subscriber unsubscribes.
  */
 export type StartStopNotifier<T> = (
-	set: ((value: T) => void) & {
-		set: (value: T) => void,
-		update: (fn: Updater<T>) => void,
-		invalidate: () => void,
-		revalidate: () => void
-	},
+	set: (value: T) => void,
 	update: (fn: Updater<T>) => void
 ) => void | (() => void);
 
@@ -33,10 +28,9 @@ export interface Readable<T> {
 	/**
 	 * Subscribe on value changes.
 	 * @param run subscription callback
-	 * @param invalidate cleanup callback - run when inputs are in an indeterminate state
-	 * @param revalidate cleanup callback - run when inputs have been resolved to their previous values
+	 * @param invalidate cleanup callback
 	 */
-	subscribe(this: void, run: Subscriber<T>, invalidate?: Invalidator<T>, revalidate?: Revalidator<T>): Unsubscriber;
+	subscribe(this: void, run: Subscriber<T>, invalidate?: Invalidator<T>): Unsubscriber;
 }
 
 /** Writable interface for both updating and subscribing. */

diff --git a/packages/svelte/src/store/utils.js b/packages/svelte/src/store/utils.js
@@ -5,10 +5,9 @@ import { noop } from '../internal/common.js';
  * @param {import('./public').Readable<T> | null | undefined} store
  * @param {(value: T) => void} run
  * @param {(value: T) => void} [invalidate]
- * @param {(value: T) => void} [revalidate]
  * @returns {() => void}
  */
-export function subscribe_to_store(store, run, invalidate, revalidate) {
+export function subscribe_to_store(store, run, invalidate) {
 	if (store == null) {
 		// @ts-expect-error
 		run(undefined);
@@ -23,8 +22,7 @@ export function subscribe_to_store(store, run, invalidate, revalidate) {
 	const unsub = store.subscribe(
 		run,
 		// @ts-expect-error
-		invalidate,
-		revalidate
+		invalidate
 	);
 
 	// Also support RxJS

diff --git a/packages/svelte/tests/store/test.ts b/packages/svelte/tests/store/test.ts
@@ -97,6 +97,20 @@ describe('writable', () => {
 		unsubscribe();
 		assert.doesNotThrow(() => unsubscribe());
 	});
+
+	it('ignores no-op sets', () => {
+		const store = writable(0);
+
+		let count = 0;
+		const unsubscribe = store.subscribe(() => {
+			count += 1;
+		});
+
+		store.set(0);
+		assert.equal(count, 1);
+
+		unsubscribe();
+	});
 });
 
 describe('readable', () => {
@@ -554,12 +568,12 @@ describe('derived', () => {
 
 	it('only updates once dependents are resolved', () => {
 		const a = writable(1);
-		const b = derived(a, a => a*2);
-		const c = derived([a,b], ([a,b]) => a+b);
+		const b = derived(a, (a) => a * 2);
+		const c = derived([a, b], ([a, b]) => a + b);
 
 		const values: number[] = [];
 
-		const unsubscribe = c.subscribe(c => {
+		const unsubscribe = c.subscribe((c) => {
 			values.push(c);
 		});