address feedback

Author: trueadm

packages/svelte/src/reactivity/map.js

@@ -24,14 +24,10 @@ export class ReactiveMap extends Map {
 		if (DEV) new Map(value);
 
 		if (value) {
-			var sources = this.#sources;
-
 			for (var [key, v] of value) {
-				sources.set(key, source(Symbol()));
 				super.set(key, v);
 			}
-
-			this.#size.v = sources.size;
+			this.#size.v = super.size;
 		}
 	}
 
@@ -45,9 +41,9 @@ export class ReactiveMap extends Map {
 		var s = sources.get(key);
 
 		if (s === undefined) {
-			// If we're working with a non-primitive then we can generate a signal for it
-			if (typeof key !== 'object' || key === null) {
-				s = source(UNINITIALIZED);
+			var ret = super.get(key);
+			if (ret !== undefined) {
+				s = source(Symbol());
 				sources.set(key, s);
 			} else {
 				// We should always track the version in case
@@ -58,7 +54,7 @@ export class ReactiveMap extends Map {
 		}
 
 		get(s);
-		return super.has(key);
+		return true;
 	}
 
 	/**
@@ -73,14 +69,18 @@ export class ReactiveMap extends Map {
 
 	/** @param {K} key */
 	get(key) {
-		var s = this.#sources.get(key);
+		var sources = this.#sources;
+		var s = sources.get(key);
 
 		if (s === undefined) {
-			// Re-use the has code-path to init the signal if needed and also
-			// register to the version if needed.
-			this.has(key);
-			s = this.#sources.get(key);
-			if (s === undefined) {
+			var ret = super.get(key);
+			if (ret !== undefined) {
+				s = source(Symbol());
+				sources.set(key, s);
+			} else {
+				// We should always track the version in case
+				// the Set ever gets this value in the future.
+				get(this.#version);
 				return undefined;
 			}
 		}
@@ -117,10 +117,7 @@ export class ReactiveMap extends Map {
 		var res = super.delete(key);
 
 		if (s !== undefined) {
-			// Remove non-primitive keys
-			if (typeof key === 'object' && key !== null) {
-				sources.delete(key);
-			}
+			sources.delete(key);
 			set(this.#size, super.size);
 			set(s, UNINITIALIZED);
 			this.#increment_version();
@@ -132,15 +129,14 @@ export class ReactiveMap extends Map {
 	clear() {
 		var sources = this.#sources;
 
-		if (sources.size !== 0) {
+		if (super.size !== 0) {
 			set(this.#size, 0);
 			for (var s of sources.values()) {
 				set(s, UNINITIALIZED);
 			}
 			this.#increment_version();
+			sources.clear();
 		}
-
-		sources.clear();
 		super.clear();
 	}
 
@@ -164,6 +160,7 @@ export class ReactiveMap extends Map {
 	}
 
 	get size() {
-		return get(this.#size);
+		get(this.#size);
+		return super.size;
 	}
 }

packages/svelte/src/reactivity/map.test.ts

@@ -36,7 +36,7 @@ test('map.values()', () => {
 		map.clear();
 	});
 
-	assert.deepEqual(log, [5, true, [1, 2, 3, 4, 5], 4, false, [1, 2, 4, 5], 0, []]);
+	assert.deepEqual(log, [5, true, [1, 2, 3, 4, 5], 4, false, [1, 2, 4, 5], 0, false, []]);
 
 	cleanup();
 });
@@ -207,7 +207,7 @@ test('not invoking reactivity when value is not in the map after changes', () =>
 		});
 	});
 
-	assert.deepEqual(log, [1, undefined, undefined, 1]);
+	assert.deepEqual(log, [1, undefined, undefined, undefined, 1, undefined]);
 
 	cleanup();
 });

packages/svelte/src/reactivity/set.js

@@ -27,14 +27,10 @@ export class ReactiveSet extends Set {
 		if (DEV) new Set(value);
 
 		if (value) {
-			var sources = this.#sources;
-
 			for (var element of value) {
-				sources.set(element, source(true));
 				super.add(element);
 			}
-
-			this.#size.v = sources.size;
+			this.#size.v = super.size;
 		}
 
 		if (!inited) this.#init();
@@ -77,9 +73,9 @@ export class ReactiveSet extends Set {
 		var s = sources.get(value);
 
 		if (s === undefined) {
-			// If we're working with a non-primitive then we can generate a signal for it
-			if (typeof value !== 'object' || value === null) {
-				s = source(false);
+			var ret = super.has(value);
+			if (ret) {
+				s = source(true);
 				sources.set(value, s);
 			} else {
 				// We should always track the version in case
@@ -129,15 +125,14 @@ export class ReactiveSet extends Set {
 	clear() {
 		var sources = this.#sources;
 
-		if (sources.size !== 0) {
+		if (super.size !== 0) {
 			set(this.#size, 0);
 			for (var s of sources.values()) {
 				set(s, false);
 			}
 			this.#increment_version();
+			sources.clear();
 		}
-
-		sources.clear();
 		super.clear();
 	}
 

packages/svelte/src/reactivity/set.test.ts

@@ -30,7 +30,7 @@ test('set.values()', () => {
 		set.clear();
 	});
 
-	assert.deepEqual(log, [5, true, [1, 2, 3, 4, 5], 4, false, [1, 2, 4, 5], 0, []]);
+	assert.deepEqual(log, [5, true, [1, 2, 3, 4, 5], 4, false, [1, 2, 4, 5], 0, false, []]);
 
 	cleanup();
 });
@@ -143,8 +143,12 @@ test('not invoking reactivity when value is not in the set after changes', () =>
 		false,
 		'has 2',
 		false,
+		'has 3',
+		false,
 		'has 2',
-		true
+		true,
+		'has 3',
+		false
 	]);
 
 	cleanup();