Skip to content

Commit

Permalink
fix: Fix values and keys tracking in Map
Browse files Browse the repository at this point in the history
  • Loading branch information
Exelord committed Oct 4, 2024
1 parent 0b1db2d commit da3985b
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 44 deletions.
37 changes: 24 additions & 13 deletions packages/map/src/index.ts
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -21,8 +21,8 @@ const $KEYS = Symbol("track-keys");
* userPoints.set(user1, { foo: "bar" });
*/
export class ReactiveMap<K, V> extends Map<K, V> {
#keyTriggers = new TriggerCache<K | typeof $KEYS>();
#valueTriggers = new TriggerCache<K>();
#keyTriggers = new TriggerCache<K | typeof $OBJECT>();
#valueTriggers = new TriggerCache<K | typeof $OBJECT>();

[Symbol.iterator](): MapIterator<[K, V]> {
return this.entries();
Expand All @@ -34,36 +34,38 @@ export class ReactiveMap<K, V> extends Map<K, V> {
}

get size(): number {
this.#keyTriggers.track($KEYS);
this.#keyTriggers.track($OBJECT);
return super.size;
}

*keys(): MapIterator<K> {
this.#keyTriggers.track($KEYS);
this.#keyTriggers.track($OBJECT);

for (const key of super.keys()) {
yield key;
}
}

*values(): MapIterator<V> {
this.#keyTriggers.track($KEYS);
this.#valueTriggers.track($OBJECT);

for (const value of super.values()) {
yield value;
}
}

*entries(): MapIterator<[K, V]> {
this.#keyTriggers.track($KEYS);
this.#keyTriggers.track($OBJECT);
this.#valueTriggers.track($OBJECT);

for (const entry of super.entries()) {
yield entry;
}
}

forEach(callbackfn: (value: V, key: K, map: Map<K, V>) => void, thisArg?: any): void {
this.#keyTriggers.track($KEYS);
this.#keyTriggers.track($OBJECT);
this.#valueTriggers.track($OBJECT);
super.forEach(callbackfn, thisArg);
}

Expand All @@ -84,9 +86,14 @@ export class ReactiveMap<K, V> extends Map<K, V> {

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);
}
});
}

Expand All @@ -99,9 +106,13 @@ export class ReactiveMap<K, V> extends Map<K, V> {

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);
}
});
}

Expand Down
50 changes: 19 additions & 31 deletions packages/map/test/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,6 @@ describe("ReactiveMap", () => {
[1, "a"],
[2, "b"],
[3, "c"],
[4, "d"],
]);

const captured: unknown[][] = [];
Expand All @@ -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);
});
Expand All @@ -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();
});
Expand All @@ -248,19 +246,15 @@ describe("ReactiveMap", () => {
[1, "a"],
[2, "b"],
[3, "c"],
[4, "d"],
]);

const captured: unknown[][] = [];

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);
});
Expand All @@ -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();
});
Expand All @@ -292,19 +284,15 @@ describe("ReactiveMap", () => {
[1, "a"],
[2, "b"],
[3, "c"],
[4, "d"],
]);

const captured: unknown[][] = [];

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);
});
Expand All @@ -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"],
]);
Expand All @@ -347,7 +339,6 @@ describe("ReactiveMap", () => {
[1, "a"],
[2, "b"],
[3, "c"],
[4, "d"],
]);

const captured: unknown[][] = [];
Expand All @@ -368,7 +359,6 @@ describe("ReactiveMap", () => {
[1, "a"],
[2, "b"],
[3, "c"],
[4, "d"],
]);

map.set(1, "e");
Expand All @@ -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();
Expand Down

0 comments on commit da3985b

Please sign in to comment.