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

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: update reactive set when deleting initial values

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 (!has) {
-			if (ret) {
+				// If the value doesn't exist, track the version in case it's added later
-				s = source(true);
+				// but don't create sources willy-nilly to track all possible values
-				sources.set(value, s);
-			} else {
-				// We should always track the version in case
-				// the Set ever gets this value in the future.
 				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;
+		if (!super.has(value)) {
-		var res = super.add(value);
+			super.add(value);
-		var s = sources.get(value);
-
-		if (s === undefined) {
-			sources.set(value, source(true));
 			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 this.values();
-		return super.keys();
 	}
 
 	values() {

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(() => {
+		flushSync(() => btn?.click());
-			btn?.click();
+		assert.htmlEqual(output.innerHTML, `<p>0</p>`);
-		});
 
-		assert.htmlEqual(
+		flushSync(() => btn2?.click());
-			target.innerHTML,
+		assert.htmlEqual(output.innerHTML, `<p>1</p><div>1</div>`);
-			`<button>add</button><button>delete</button><button>clear</button><div>1</div>`
-		);
 
-		flushSync(() => {
+		flushSync(() => btn2?.click());
-			btn?.click();
+		flushSync(() => btn2?.click());
-		});
+		assert.htmlEqual(output.innerHTML, `<p>3</p><div>1</div><div>2</div><div>3</div>`);
 
-		flushSync(() => {
+		flushSync(() => btn3?.click());
-			btn?.click();
+		assert.htmlEqual(output.innerHTML, `<p>2</p><div>1</div><div>2</div>`);
-		});
 
-		assert.htmlEqual(
+		flushSync(() => btn4?.click());
-			target.innerHTML,
+		assert.htmlEqual(output.innerHTML, `<p>0</p>`);
-			`<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>`
-		);
 	}
 });

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={() => {
+<button onclick={() => state.delete(0)}>delete initial</button>
-	state.add(state.size + 1);
+<button onclick={() => state.add(state.size + 1)}>add</button>
-}}>add</button>
+<button onclick={() => state.delete(state.size)}>delete</button>
+<button onclick={() => state.clear()}>clear</button>
 
-<button onclick={() => {
+<div id="output">
-	state.delete(state.size);
+	<p>{state.size}</p>
-}}>delete</button>
 
-<button onclick={() => {
+	{#each state as item}
-	state.clear();
+		<div>{item}</div>
-}}>clear</button>
+	{/each}
-
+</div>
-{#each state as item}
-	<div>{item}</div>
-{/each}