Make ReactiveSet.clear() and ReactiveMap.clear() more efficient by only notifying listeners of existing members (#784)

Author: atk

.changeset/funny-baths-drive.md

@@ -0,0 +1,5 @@
+---
+"@solid-primitives/set": minor
+---
+
+Avoid notifying non-existent members on Set.clear()

.changeset/red-islands-tickle.md

@@ -0,0 +1,5 @@
+---
+"@solid-primitives/map": minor
+---
+
+Avoid notifying non-existent members on Map.clear()

packages/map/src/index.ts

@@ -120,14 +120,17 @@ export class ReactiveMap<K, V> extends Map<K, V> {
   }
 
   clear(): void {
-    if (super.size) {
-      super.clear();
+    if (super.size === 0) return;
+    batch(() => {
+      this.#keyTriggers.dirty($OBJECT);
+      this.#valueTriggers.dirty($OBJECT);
+      for (const key of super.keys()) {
+        this.#keyTriggers.dirty(key);
+        this.#valueTriggers.dirty(key);
+      }
 
-      batch(() => {
-        this.#keyTriggers.dirtyAll();
-        this.#valueTriggers.dirtyAll();
-      });
-    }
+      super.clear();
+    });
   }
 }
 

packages/map/test/index.test.ts

@@ -378,6 +378,45 @@ describe("ReactiveMap", () => {
 
     dispose();
   });
+  test(".clear() notifies only listeners of existing members", () =>
+    createRoot(dispose => {
+      const map = new ReactiveMap([
+        [1, "a"],
+        [2, "b"],
+        [3, "c"],
+      ]);
+
+      const existingKey = vi.fn();
+      createComputed(() => existingKey(map.has(2)));
+
+      const existingValue = vi.fn();
+      createComputed(() => existingValue(map.get(2)));
+
+      const nonexistingKey = vi.fn();
+      createComputed(() => nonexistingKey(map.has(4)));
+
+      const nonexistingValue = vi.fn();
+      createComputed(() => nonexistingValue(map.get(4)));
+
+      expect(existingKey).toHaveBeenNthCalledWith(1, true);
+      expect(existingValue).toHaveBeenNthCalledWith(1, "b");
+
+      expect(nonexistingKey).toHaveBeenNthCalledWith(1, false);
+      expect(nonexistingValue).toHaveBeenNthCalledWith(1, undefined);
+
+      map.clear();
+
+      expect(existingKey).toHaveBeenCalledTimes(2);
+      expect(existingKey).toHaveBeenNthCalledWith(2, false);
+
+      expect(existingValue).toHaveBeenCalledTimes(2);
+      expect(existingValue).toHaveBeenNthCalledWith(2, undefined);
+
+      expect(nonexistingKey).toHaveBeenCalledTimes(1);
+      expect(nonexistingValue).toHaveBeenCalledTimes(1);
+
+      dispose();
+    }));
 });
 
 describe("ReactiveWeakMap", () => {

packages/set/src/index.ts

@@ -88,13 +88,14 @@ export class ReactiveSet<T> extends Set<T> {
   }
 
   clear(): void {
-    if (super.size) {
+    if (!super.size) return;
+    batch(() => {
+      this.#triggers.dirty($KEYS);
+      for (const member of super.values()) {
+        this.#triggers.dirty(member);
+      }
       super.clear();
-
-      batch(() => {
-        this.#triggers.dirtyAll();
-      });
-    }
+    });
   }
 }
 

packages/set/test/index.test.ts

@@ -182,6 +182,29 @@ describe("ReactiveSet", () => {
 
     dispose();
   });
+
+  test("clear notifies only listeners of existing members", () =>
+    createRoot(dispose => {
+      const set = new ReactiveSet([1, 2, 3, 4]);
+
+      const existing = vi.fn();
+      createComputed(() => existing(set.has(2)));
+
+      const nonexisting = vi.fn();
+      createComputed(() => nonexisting(set.has(5)));
+
+      expect(existing).toHaveBeenNthCalledWith(1, true);
+      expect(nonexisting).toHaveBeenNthCalledWith(1, false);
+
+      set.clear();
+
+      expect(existing).toHaveBeenCalledTimes(2);
+      expect(existing).toHaveBeenNthCalledWith(2, false);
+
+      expect(nonexisting).toHaveBeenCalledTimes(1);
+
+      dispose();
+    }));
 });
 
 describe("ReactiveWeakSet", () => {