From bd950a0be2e8f57f06e2ef109b62943ec3d1d3d6 Mon Sep 17 00:00:00 2001
From: Rich Harris <rich.harris@vercel.com>
Date: Sat, 8 Jun 2024 09:05:23 -0400
Subject: [PATCH] fix: more efficient and correct reactive set (#11967)

* fix: more efficient and correct reactive set

* Update .changeset/thin-spoons-float.md
---
 .changeset/thin-spoons-float.md               |  5 ++
 packages/svelte/src/reactivity/set.js         | 50 +++++++++--------
 .../samples/reactive-set/_config.js           | 54 ++++++-------------
 .../samples/reactive-set/main.svelte          | 27 +++++-----
 4 files changed, 57 insertions(+), 79 deletions(-)
 create mode 100644 .changeset/thin-spoons-float.md

diff --git a/.changeset/thin-spoons-float.md b/.changeset/thin-spoons-float.md
new file mode 100644
index 0000000000..d76c33164e
--- /dev/null
+++ b/.changeset/thin-spoons-float.md
@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: update reactive set when deleting initial values
diff --git a/packages/svelte/src/reactivity/set.js b/packages/svelte/src/reactivity/set.js
index 98d941a07d..01d4d5e903 100644
--- a/packages/svelte/src/reactivity/set.js
+++ b/packages/svelte/src/reactivity/set.js
@@ -66,76 +66,74 @@ export class ReactiveSet extends Set {
 
 	/** @param {T} value */
 	has(value) {
+		var has = super.has(value);
 		var sources = this.#sources;
 		var s = sources.get(value);
 
 		if (s === undefined) {
-			var ret = super.has(value);
-			if (ret) {
-				s = source(true);
-				sources.set(value, s);
-			} else {
-				// We should always track the version in case
-				// the Set ever gets this value in the future.
+			if (!has) {
+				// If the value doesn't exist, track the version in case it's added later
+				// but don't create sources willy-nilly to track all possible values
 				get(this.#version);
 				return false;
 			}
+
+			s = source(true);
+			sources.set(value, s);
 		}
 
 		get(s);
-		return super.has(value);
+		return has;
 	}
 
 	/** @param {T} value */
 	add(value) {
-		var sources = this.#sources;
-		var res = super.add(value);
-		var s = sources.get(value);
-
-		if (s === undefined) {
-			sources.set(value, source(true));
+		if (!super.has(value)) {
+			super.add(value);
 			set(this.#size, super.size);
 			increment(this.#version);
-		} else {
-			set(s, true);
 		}
 
-		return res;
+		return this;
 	}
 
 	/** @param {T} value */
 	delete(value) {
+		var deleted = super.delete(value);
 		var sources = this.#sources;
 		var s = sources.get(value);
-		var res = super.delete(value);
 
 		if (s !== undefined) {
 			sources.delete(value);
-			set(this.#size, super.size);
 			set(s, false);
+		}
+
+		if (deleted) {
+			set(this.#size, super.size);
 			increment(this.#version);
 		}
 
-		return res;
+		return deleted;
 	}
 
 	clear() {
-		var sources = this.#sources;
-
 		if (super.size !== 0) {
-			set(this.#size, 0);
+			var sources = this.#sources;
+
 			for (var s of sources.values()) {
 				set(s, false);
 			}
-			increment(this.#version);
+
 			sources.clear();
+			set(this.#size, 0);
+			increment(this.#version);
 		}
+
 		super.clear();
 	}
 
 	keys() {
-		get(this.#version);
-		return super.keys();
+		return this.values();
 	}
 
 	values() {
diff --git a/packages/svelte/tests/runtime-runes/samples/reactive-set/_config.js b/packages/svelte/tests/runtime-runes/samples/reactive-set/_config.js
index b2284071bb..baf2e3df35 100644
--- a/packages/svelte/tests/runtime-runes/samples/reactive-set/_config.js
+++ b/packages/svelte/tests/runtime-runes/samples/reactive-set/_config.js
@@ -1,50 +1,28 @@
 import { flushSync } from '../../../../src/index-client';
-import { test } from '../../test';
+import { ok, test } from '../../test';
 
 export default test({
-	html: `<button>add</button><button>delete</button><button>clear</button>`,
+	html: `<button>delete initial</button><button>add</button><button>delete</button><button>clear</button><div id="output"><p>1</p><div>0</div></div>`,
 
 	test({ assert, target }) {
-		const [btn, btn2, btn3] = target.querySelectorAll('button');
+		const [btn, btn2, btn3, btn4] = target.querySelectorAll('button');
+		const output = target.querySelector('#output');
+		ok(output);
 
-		flushSync(() => {
-			btn?.click();
-		});
+		flushSync(() => btn?.click());
+		assert.htmlEqual(output.innerHTML, `<p>0</p>`);
 
-		assert.htmlEqual(
-			target.innerHTML,
-			`<button>add</button><button>delete</button><button>clear</button><div>1</div>`
-		);
+		flushSync(() => btn2?.click());
+		assert.htmlEqual(output.innerHTML, `<p>1</p><div>1</div>`);
 
-		flushSync(() => {
-			btn?.click();
-		});
+		flushSync(() => btn2?.click());
+		flushSync(() => btn2?.click());
+		assert.htmlEqual(output.innerHTML, `<p>3</p><div>1</div><div>2</div><div>3</div>`);
 
-		flushSync(() => {
-			btn?.click();
-		});
+		flushSync(() => btn3?.click());
+		assert.htmlEqual(output.innerHTML, `<p>2</p><div>1</div><div>2</div>`);
 
-		assert.htmlEqual(
-			target.innerHTML,
-			`<button>add</button><button>delete</button><button>clear</button><div>1</div><div>2</div><div>3</div>`
-		);
-
-		flushSync(() => {
-			btn2?.click();
-		});
-
-		assert.htmlEqual(
-			target.innerHTML,
-			`<button>add</button><button>delete</button><button>clear</button><div>1</div><div>2</div>`
-		);
-
-		flushSync(() => {
-			btn3?.click();
-		});
-
-		assert.htmlEqual(
-			target.innerHTML,
-			`<button>add</button><button>delete</button><button>clear</button>`
-		);
+		flushSync(() => btn4?.click());
+		assert.htmlEqual(output.innerHTML, `<p>0</p>`);
 	}
 });
diff --git a/packages/svelte/tests/runtime-runes/samples/reactive-set/main.svelte b/packages/svelte/tests/runtime-runes/samples/reactive-set/main.svelte
index 523335ced9..1272d28ebe 100644
--- a/packages/svelte/tests/runtime-runes/samples/reactive-set/main.svelte
+++ b/packages/svelte/tests/runtime-runes/samples/reactive-set/main.svelte
@@ -1,21 +1,18 @@
 <script>
-	import {Set} from 'svelte/reactivity';
+	import { Set } from 'svelte/reactivity';
 
-	let state = new Set();
+	let state = new Set([0]);
 </script>
 
-<button onclick={() => {
-	state.add(state.size + 1);
-}}>add</button>
+<button onclick={() => state.delete(0)}>delete initial</button>
+<button onclick={() => state.add(state.size + 1)}>add</button>
+<button onclick={() => state.delete(state.size)}>delete</button>
+<button onclick={() => state.clear()}>clear</button>
 
-<button onclick={() => {
-	state.delete(state.size);
-}}>delete</button>
+<div id="output">
+	<p>{state.size}</p>
 
-<button onclick={() => {
-	state.clear();
-}}>clear</button>
-
-{#each state as item}
-	<div>{item}</div>
-{/each}
+	{#each state as item}
+		<div>{item}</div>
+	{/each}
+</div>