From da3985b98866ece6685828a7755ba73c2357b484 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Kwa=C5=9Bniak?= Date: Fri, 4 Oct 2024 12:12:36 +0200 Subject: [PATCH] fix: Fix values and keys tracking in Map --- packages/map/src/index.ts | 37 +++++++++++++++--------- packages/map/test/index.test.ts | 50 +++++++++++++-------------------- 2 files changed, 43 insertions(+), 44 deletions(-) diff --git a/packages/map/src/index.ts b/packages/map/src/index.ts index 610194fbf..3d4df2f5b 100644 --- a/packages/map/src/index.ts +++ b/packages/map/src/index.ts @@ -1,7 +1,7 @@ import { Accessor, batch } from "solid-js"; import { TriggerCache } from "@solid-primitives/trigger"; -const $KEYS = Symbol("track-keys"); +const $OBJECT = Symbol("track-object"); /** * A reactive version of `Map` data structure. All the reads (like `get` or `has`) are signals, and all the writes (`delete` or `set`) will cause updates to appropriate signals. @@ -21,8 +21,8 @@ const $KEYS = Symbol("track-keys"); * userPoints.set(user1, { foo: "bar" }); */ export class ReactiveMap extends Map { - #keyTriggers = new TriggerCache(); - #valueTriggers = new TriggerCache(); + #keyTriggers = new TriggerCache(); + #valueTriggers = new TriggerCache(); [Symbol.iterator](): MapIterator<[K, V]> { return this.entries(); @@ -34,12 +34,12 @@ export class ReactiveMap extends Map { } get size(): number { - this.#keyTriggers.track($KEYS); + this.#keyTriggers.track($OBJECT); return super.size; } *keys(): MapIterator { - this.#keyTriggers.track($KEYS); + this.#keyTriggers.track($OBJECT); for (const key of super.keys()) { yield key; @@ -47,7 +47,7 @@ export class ReactiveMap extends Map { } *values(): MapIterator { - this.#keyTriggers.track($KEYS); + this.#valueTriggers.track($OBJECT); for (const value of super.values()) { yield value; @@ -55,7 +55,8 @@ export class ReactiveMap extends Map { } *entries(): MapIterator<[K, V]> { - this.#keyTriggers.track($KEYS); + this.#keyTriggers.track($OBJECT); + this.#valueTriggers.track($OBJECT); for (const entry of super.entries()) { yield entry; @@ -63,7 +64,8 @@ export class ReactiveMap extends Map { } forEach(callbackfn: (value: V, key: K, map: Map) => void, thisArg?: any): void { - this.#keyTriggers.track($KEYS); + this.#keyTriggers.track($OBJECT); + this.#valueTriggers.track($OBJECT); super.forEach(callbackfn, thisArg); } @@ -84,9 +86,14 @@ export class ReactiveMap extends Map { if (hasChanged || hadNoKey) { batch(() => { - this.#keyTriggers.dirty($KEYS); - if (hadNoKey) this.#keyTriggers.dirty(key); - if (hasChanged) this.#valueTriggers.dirty(key); + if (hadNoKey) { + this.#keyTriggers.dirty($OBJECT); + this.#keyTriggers.dirty(key); + } + if (hasChanged) { + this.#valueTriggers.dirty($OBJECT); + this.#valueTriggers.dirty(key); + } }); } @@ -99,9 +106,13 @@ export class ReactiveMap extends Map { if (result) { batch(() => { - this.#keyTriggers.dirty($KEYS); + this.#keyTriggers.dirty($OBJECT); this.#keyTriggers.dirty(key); - if (isDefined) this.#valueTriggers.dirty(key); + + if (isDefined) { + this.#valueTriggers.dirty($OBJECT); + this.#valueTriggers.dirty(key); + } }); } diff --git a/packages/map/test/index.test.ts b/packages/map/test/index.test.ts index d70c7d726..ccc5f2898 100644 --- a/packages/map/test/index.test.ts +++ b/packages/map/test/index.test.ts @@ -210,7 +210,6 @@ describe("ReactiveMap", () => { [1, "a"], [2, "b"], [3, "c"], - [4, "d"], ]); const captured: unknown[][] = []; @@ -220,7 +219,6 @@ describe("ReactiveMap", () => { const run: unknown[] = []; for (const key of map.keys()) { run.push(key); - if (key === 3) break; // don't iterate over all keys } captured.push(run); }); @@ -233,12 +231,12 @@ describe("ReactiveMap", () => { map.set(1, "e"); expect(captured, "value change").toHaveLength(1); - map.set(5, "f"); - expect(captured, "not seen key change").toHaveLength(1); + map.set(4, "f"); + expect(captured, "new key added").toHaveLength(2); map.delete(1); - expect(captured, "seen key change").toHaveLength(2); - expect(captured[1]).toEqual([2, 3]); + expect(captured, "seen key change").toHaveLength(3); + expect(captured[2]).toEqual([2, 3, 4]); dispose(); }); @@ -248,7 +246,6 @@ describe("ReactiveMap", () => { [1, "a"], [2, "b"], [3, "c"], - [4, "d"], ]); const captured: unknown[][] = []; @@ -256,11 +253,8 @@ describe("ReactiveMap", () => { const dispose = createRoot(dispose => { createEffect(() => { const run: unknown[] = []; - let i = 0; for (const v of map.values()) { run.push(v); - if (i === 2) break; // don't iterate over all keys - i += 1; } captured.push(run); }); @@ -275,14 +269,12 @@ describe("ReactiveMap", () => { expect(captured[1]).toEqual(["e", "b", "c"]); map.set(4, "f"); - expect(captured, "not seen value change").toHaveLength(2); + expect(captured, "new key added").toHaveLength(3); + expect(captured[2]).toEqual(["e", "b", "c", "f"]); map.delete(4); - expect(captured, "not seen key change").toHaveLength(2); - - map.delete(1); - expect(captured, "seen key change").toHaveLength(3); - expect(captured[2]).toEqual(["b", "c"]); + expect(captured, "key removed").toHaveLength(4); + expect(captured[3]).toEqual(["e", "b", "c"]); dispose(); }); @@ -292,7 +284,6 @@ describe("ReactiveMap", () => { [1, "a"], [2, "b"], [3, "c"], - [4, "d"], ]); const captured: unknown[][] = []; @@ -300,11 +291,8 @@ describe("ReactiveMap", () => { const dispose = createRoot(dispose => { createEffect(() => { const run: unknown[] = []; - let i = 0; for (const e of map.entries()) { run.push(e); - if (i === 2) break; // don't iterate over all keys - i += 1; } captured.push(run); }); @@ -327,14 +315,18 @@ describe("ReactiveMap", () => { ]); map.set(4, "f"); - expect(captured, "not seen value change").toHaveLength(2); + expect(captured, "new key added").toHaveLength(3); + expect(captured[2]).toEqual([ + [1, "e"], + [2, "b"], + [3, "c"], + [4, "f"], + ]); map.delete(4); - expect(captured, "not seen key change").toHaveLength(2); - - map.delete(1); - expect(captured, "seen key change").toHaveLength(3); - expect(captured[2]).toEqual([ + expect(captured, "key removed").toHaveLength(4); + expect(captured[3]).toEqual([ + [1, "e"], [2, "b"], [3, "c"], ]); @@ -347,7 +339,6 @@ describe("ReactiveMap", () => { [1, "a"], [2, "b"], [3, "c"], - [4, "d"], ]); const captured: unknown[][] = []; @@ -368,7 +359,6 @@ describe("ReactiveMap", () => { [1, "a"], [2, "b"], [3, "c"], - [4, "d"], ]); map.set(1, "e"); @@ -377,15 +367,13 @@ describe("ReactiveMap", () => { [1, "e"], [2, "b"], [3, "c"], - [4, "d"], ]); - map.delete(4); + map.delete(3); expect(captured).toHaveLength(3); expect(captured[2]).toEqual([ [1, "e"], [2, "b"], - [3, "c"], ]); dispose();