fix(map): trigger onDelete for replacing same key

crimx · Jun 20, 2024 · a956c0a · a956c0a
1 parent 428b0ce
commit a956c0a
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 19 deletions.
diff --git a/README.md b/README.md
@@ -54,7 +54,7 @@ npm add value-enhancer
 | `{ compute }`                 | 192 B        |
 | `{ flattenFrom }`             | 227 B        |
 | `{ flatten }`                 | 36 B         |
-| `{ reactiveMap }`             | 479 B        |
+| `{ reactiveMap }`             | 464 B        |
 | `{ reactiveSet }`             | 359 B        |
 | `{ reactiveList }`            | 528 B        |
 

diff --git a/src/collections/map.ts b/src/collections/map.ts
@@ -59,7 +59,7 @@ export type ReadonlyReactiveMap<TKey, TValue> = Pick<
   readonly $: ReadonlyVal<ReadonlyReactiveMap<TKey, TValue>>;
 };
 
-export interface ReactiveMapConfig<TKey, TValue> {
+export interface ReactiveMapConfig<TValue> {
   /**
    * A callback function that will be called when an entry is deleted.
    *
@@ -69,7 +69,7 @@ export interface ReactiveMapConfig<TKey, TValue> {
    * - `map.clear()` is called.
    * - `map.dispose()` is called.
    */
-  onDeleted?: (value: TValue, key: TKey) => void;
+  onDeleted?: (value: TValue) => void;
 }
 
 class ReactiveMapImpl<TKey, TValue>
@@ -78,7 +78,7 @@ class ReactiveMapImpl<TKey, TValue>
 {
   public constructor(
     entries?: Iterable<readonly [TKey, TValue]> | null,
-    config?: ReactiveMapConfig<TKey, TValue>
+    config?: ReactiveMapConfig<TValue>
   ) {
     super();
 
@@ -101,14 +101,14 @@ class ReactiveMapImpl<TKey, TValue>
 
   #notify: () => void;
 
-  #onDeleted?: (value: TValue, key: TKey) => void;
+  #onDeleted?: (value: TValue) => void;
 
   #delete(key: TKey): boolean {
     if (this.#onDeleted) {
       if (super.has(key)) {
         const value = super.get(key)!;
         super.delete(key);
-        this.#onDeleted(value, key);
+        this.#onDeleted(value);
         return true;
       }
       return false;
@@ -118,10 +118,10 @@ class ReactiveMapImpl<TKey, TValue>
 
   #clear(): void {
     if (this.#onDeleted) {
-      const deleted = [...this];
+      const deleted = new Set(this.values());
       super.clear();
-      for (const [key, value] of deleted) {
-        this.#onDeleted(value, key);
+      for (const value of deleted) {
+        this.#onDeleted(value);
       }
     } else {
       super.clear();
@@ -162,7 +162,7 @@ class ReactiveMapImpl<TKey, TValue>
       }
       super.set(key, value);
       if (this.#onDeleted) {
-        this.#onDeleted(oldValue, key);
+        this.#onDeleted(oldValue);
       }
     } else {
       super.set(key, value);
@@ -190,21 +190,21 @@ class ReactiveMapImpl<TKey, TValue>
 
   public replace(entries: Iterable<readonly [TKey, TValue]>): Iterable<TValue> {
     const oldMap = new Map(this);
-    const deleted = new Map<TKey, TValue>(this);
-    let isDirty = false;
+    const deleted = new Set<TValue>(this.values());
+    let hasNewValue = false;
     super.clear();
 
     for (const [key, value] of entries) {
-      isDirty =
-        isDirty || !oldMap.has(key) || !Object.is(oldMap.get(key), value);
       super.set(key, value);
-      deleted.delete(key);
+      deleted.delete(value);
+      hasNewValue =
+        hasNewValue || !oldMap.has(key) || !Object.is(oldMap.get(key), value);
     }
 
-    if (isDirty || deleted.size > 0) {
+    if (hasNewValue || deleted.size > 0) {
       if (this.#onDeleted) {
-        for (const [key, value] of deleted) {
-          this.#onDeleted(value, key);
+        for (const value of deleted) {
+          this.#onDeleted(value);
         }
       }
       this.#notify();
@@ -257,5 +257,5 @@ class ReactiveMapImpl<TKey, TValue>
  */
 export const reactiveMap = <TKey, TValue>(
   entries?: Iterable<readonly [TKey, TValue]> | null,
-  config?: ReactiveMapConfig<TKey, TValue>
+  config?: ReactiveMapConfig<TValue>
 ): ReactiveMap<TKey, TValue> => new ReactiveMapImpl(entries, config);
diff --git a/test/collections/map.test.ts b/test/collections/map.test.ts
@@ -212,6 +212,12 @@ describe("ReactiveMap", () => {
       expect(map.get("baz")).toEqual(3);
     });
 
+    it("should return deleted same key entries", () => {
+      const map = reactiveMap(Object.entries({ foo: 1, bar: 2 }));
+      const deleted = map.replace(Object.entries({ foo: 3, bar: 4 }));
+      expect([...deleted]).toHaveLength(2);
+    });
+
     it("should notify on replace", () => {
       const map = reactiveMap<string, number>();
       const mockNotify = jest.fn();
@@ -380,6 +386,17 @@ describe("ReactiveMap", () => {
       expect(map.size).toBe(0);
       expect(spy).toHaveBeenCalledTimes(1);
     });
+
+    it("should trigger one onDelete for same value different keys", () => {
+      const spy = jest.fn();
+      const map = reactiveMap(Object.entries({ foo: 2, bar: 2 }), {
+        onDeleted: spy,
+      });
+      const deleted = map.replace(Object.entries({ foo: 3, bar: 4 }));
+      expect([...deleted]).toHaveLength(1);
+      expect(spy).toHaveBeenCalledTimes(1);
+      expect(spy).toHaveBeenCalledWith(2);
+    });
   });
 
   describe("toJSON", () => {