From 4e2d16150e7ab9a3abfa96b5f050820f9f9ad3cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Kwa=C5=9Bniak?= Date: Sun, 8 Sep 2024 20:47:03 +0200 Subject: [PATCH 1/7] feat: Add dirtyAll to TriggerCache --- packages/trigger/README.md | 16 ++++++++++++++++ packages/trigger/src/index.ts | 16 ++++++++++++---- packages/trigger/test/index.test.ts | 15 +++++++++++++++ 3 files changed, 43 insertions(+), 4 deletions(-) diff --git a/packages/trigger/README.md b/packages/trigger/README.md index 4d6b80df0..c1369169e 100644 --- a/packages/trigger/README.md +++ b/packages/trigger/README.md @@ -75,6 +75,22 @@ map.dirty(1); map.dirty(2); ``` +### Triggering all keys + +`dirtyAll` will trigger all keys in the cache. + +```ts +const [track, _dirty, dirtyAll] = createTriggerCache(); + +createEffect(() => { + track(1); + // ... +}); + +// later (will trigger the update) +dirtyAll(); +``` + ### Weak version `createTriggerCache` constructor can take a `WeakMap` constructor as an argument. This will create a `WeakMap` of triggers instead of a `Map`. diff --git a/packages/trigger/src/index.ts b/packages/trigger/src/index.ts index e91011632..08c0a2f02 100644 --- a/packages/trigger/src/index.ts +++ b/packages/trigger/src/index.ts @@ -50,6 +50,11 @@ export class TriggerCache { this.#map.get(key)?.$$(); } + dirtyAll() { + if (isServer) return; + for (const trigger of this.#map.values()) trigger.$$(); + } + track(key: T) { if (!getListener()) return; let trigger = this.#map.get(key); @@ -77,11 +82,12 @@ export class TriggerCache { * Trigger signals added to the cache only when tracked under a computation, * and get deleted from the cache when they are no longer tracked. * - * @returns a tuple of `[track, dirty]` functions + * @returns an array of `[track, dirty, dirtyAll]` functions * * `track` and `dirty` are called with a `key` so that each tracker will trigger an update only when his individual `key` would get marked as dirty. + * `dirtyAll` will mark all keys as dirty and trigger an update for all of them. * @example - * const [track, dirty] = createTriggerCache() + * const [track, dirty, dirtyAll] = createTriggerCache() * createEffect(() => { * track(1) * ... @@ -90,10 +96,12 @@ export class TriggerCache { * dirty(1) * // this won't cause an update: * dirty(2) + * // this will cause an update to all keys: + * dirtyAll() */ export function createTriggerCache( mapConstructor: WeakMapConstructor | MapConstructor = Map, -): [track: (key: T) => void, dirty: (key: T) => void] { +): [track: (key: T) => void, dirty: (key: T) => void, dirtyAll: () => void] { const map = new TriggerCache(mapConstructor); - return [map.track.bind(map), map.dirty.bind(map)]; + return [map.track.bind(map), map.dirty.bind(map), map.dirtyAll.bind(map)]; } diff --git a/packages/trigger/test/index.test.ts b/packages/trigger/test/index.test.ts index 854c776bd..2e5b54645 100644 --- a/packages/trigger/test/index.test.ts +++ b/packages/trigger/test/index.test.ts @@ -22,6 +22,21 @@ describe("createTriggerCache", () => { dispose(); })); + test("dirtyAll", () => + createRoot(dispose => { + const [track, , dirtyAll] = createTriggerCache(Map); + let runs = -1; + createComputed(() => { + track(1); + runs++; + }); + expect(runs).toBe(0); + dirtyAll(); + expect(runs).toBe(1); + + dispose(); + })); + test("weak trigger cache", () => createRoot(dispose => { const [track, dirty] = createTriggerCache(); From cea0f93be3fdac47666163354494586a25bdc1e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Kwa=C5=9Bniak?= Date: Sun, 8 Sep 2024 23:09:57 +0200 Subject: [PATCH 2/7] fix: Clear ReactiveSet before dirtying keys --- packages/set/src/index.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/set/src/index.ts b/packages/set/src/index.ts index e22b8ce79..a3192dd4f 100644 --- a/packages/set/src/index.ts +++ b/packages/set/src/index.ts @@ -82,10 +82,10 @@ export class ReactiveSet extends Set { } clear(): void { if (super.size) { + super.clear(); + batch(() => { - for (const v of super.keys()) this.#triggers.dirty(v); - super.clear(); - this.#triggers.dirty($KEYS); + this.#triggers.dirtyAll(); }); } } From 85008193737c419356ef82da26520abca35fbdf5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Kwa=C5=9Bniak?= Date: Sun, 8 Sep 2024 23:24:31 +0200 Subject: [PATCH 3/7] fix: Don't assume iterators will always break on specific iteration --- packages/set/src/index.ts | 32 ++++++++++++++++++-------------- packages/set/test/index.test.ts | 26 ++++++++++++++++---------- 2 files changed, 34 insertions(+), 24 deletions(-) diff --git a/packages/set/src/index.ts b/packages/set/src/index.ts index a3192dd4f..8a365e413 100644 --- a/packages/set/src/index.ts +++ b/packages/set/src/index.ts @@ -28,35 +28,39 @@ export class ReactiveSet extends Set { this.#triggers.track($KEYS); return super.size; } + has(v: T): boolean { this.#triggers.track(v); return super.has(v); } - *keys(): IterableIterator { - for (const key of super.keys()) { - this.#triggers.track(key); - yield key; - } - this.#triggers.track($KEYS); + keys(): IterableIterator { + return this.values(); } - values(): IterableIterator { - return this.keys(); + + *values(): IterableIterator { + this.#triggers.track($KEYS); + + for (const value of super.values()) { + yield value; + } } + *entries(): IterableIterator<[T, T]> { - for (const key of super.keys()) { - this.#triggers.track(key); - yield [key, key]; - } this.#triggers.track($KEYS); + + for (const entry of super.entries()) { + yield entry; + } } [Symbol.iterator](): IterableIterator { return this.values(); } - forEach(callbackfn: (value: T, value2: T, set: this) => void) { + + forEach(callbackfn: (value1: T, value2: T, set: Set) => void, thisArg?: any): void { this.#triggers.track($KEYS); - super.forEach(callbackfn as any); + super.forEach(callbackfn, thisArg); } // writes diff --git a/packages/set/test/index.test.ts b/packages/set/test/index.test.ts index 8d97062e5..412fba986 100644 --- a/packages/set/test/index.test.ts +++ b/packages/set/test/index.test.ts @@ -100,11 +100,8 @@ describe("ReactiveSet", () => { const dispose = createRoot(dispose => { createEffect(() => { const run: number[] = []; - let i = 0; for (const key of fn(set)) { run.push(key); - if (i === 2) break; // don't iterate over all keys - i += 1; } captured.push(run); }); @@ -112,21 +109,30 @@ describe("ReactiveSet", () => { }); expect(captured).toHaveLength(1); - expect(captured[0]).toEqual([1, 2, 3]); + expect(captured[0]).toEqual([1, 2, 3, 4]); set.delete(4); - expect(captured, "deleted unseen key").toHaveLength(1); + expect(captured).toHaveLength(2); + expect(captured[1]).toEqual([1, 2, 3]); set.delete(1); - expect(captured, "deleted seen").toHaveLength(2); - expect(captured[1]).toEqual([2, 3]); + expect(captured).toHaveLength(3); + expect(captured[2]).toEqual([2, 3]); set.add(4); - expect(captured, "added key in reach").toHaveLength(3); - expect(captured[2]).toEqual([2, 3, 4]); + expect(captured).toHaveLength(4); + expect(captured[3]).toEqual([2, 3, 4]); set.add(5); - expect(captured, "added key out of reach").toHaveLength(3); + expect(captured).toHaveLength(5); + expect(captured[4]).toEqual([2, 3, 4, 5]); + + set.add(5); + expect(captured).toHaveLength(5); + + set.clear(); + expect(captured).toHaveLength(6); + expect(captured[5]).toEqual([]); dispose(); }); From a3038c2617eb78c21448ae3022addf2f854f617b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Kwa=C5=9Bniak?= Date: Sun, 8 Sep 2024 23:27:37 +0200 Subject: [PATCH 4/7] chore: Unify variables names with Set interface in ReactiveSet --- packages/set/src/index.ts | 65 ++++++++++++++++++++++----------------- 1 file changed, 36 insertions(+), 29 deletions(-) diff --git a/packages/set/src/index.ts b/packages/set/src/index.ts index 8a365e413..8e448ddf9 100644 --- a/packages/set/src/index.ts +++ b/packages/set/src/index.ts @@ -23,15 +23,18 @@ export class ReactiveSet extends Set { if (values) for (const v of values) super.add(v); } - // reads + [Symbol.iterator](): IterableIterator { + return this.values(); + } + get size(): number { this.#triggers.track($KEYS); return super.size; } - has(v: T): boolean { - this.#triggers.track(v); - return super.has(v); + has(value: T): boolean { + this.#triggers.track(value); + return super.has(value); } keys(): IterableIterator { @@ -54,36 +57,36 @@ export class ReactiveSet extends Set { } } - [Symbol.iterator](): IterableIterator { - return this.values(); - } - forEach(callbackfn: (value1: T, value2: T, set: Set) => void, thisArg?: any): void { this.#triggers.track($KEYS); super.forEach(callbackfn, thisArg); } - // writes - add(v: T): this { - if (!super.has(v)) { - super.add(v); + add(value: T): this { + if (!super.has(value)) { + super.add(value); batch(() => { - this.#triggers.dirty(v); + this.#triggers.dirty(value); this.#triggers.dirty($KEYS); }); } + return this; } - delete(v: T): boolean { - const r = super.delete(v); - if (r) { + + delete(value: T): boolean { + const result = super.delete(value); + + if (result) { batch(() => { - this.#triggers.dirty(v); + this.#triggers.dirty(value); this.#triggers.dirty($KEYS); }); } - return r; + + return result; } + clear(): void { if (super.size) { super.clear(); @@ -113,21 +116,25 @@ export class ReactiveWeakSet extends WeakSet { if (values) for (const v of values) super.add(v); } - has(v: T): boolean { - this.#triggers.track(v); - return super.has(v); + has(value: T): boolean { + this.#triggers.track(value); + return super.has(value); } - add(v: T): this { - if (!super.has(v)) { - super.add(v); - this.#triggers.dirty(v); + + add(value: T): this { + if (!super.has(value)) { + super.add(value); + this.#triggers.dirty(value); } return this; } - delete(v: T): boolean { - const deleted = super.delete(v); - deleted && this.#triggers.dirty(v); - return deleted; + + delete(value: T): boolean { + const result = super.delete(value); + + result && this.#triggers.dirty(value); + + return result; } } From 98743c346329b7ac368a369c4d3413750ba10bff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Kwa=C5=9Bniak?= Date: Sat, 28 Sep 2024 19:43:54 +0200 Subject: [PATCH 5/7] chore: Apply feedback --- packages/trigger/src/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/trigger/src/index.ts b/packages/trigger/src/index.ts index 08c0a2f02..d370e67ed 100644 --- a/packages/trigger/src/index.ts +++ b/packages/trigger/src/index.ts @@ -82,7 +82,7 @@ export class TriggerCache { * Trigger signals added to the cache only when tracked under a computation, * and get deleted from the cache when they are no longer tracked. * - * @returns an array of `[track, dirty, dirtyAll]` functions + * @returns a tuple of `[track, dirty, dirtyAll]` functions * * `track` and `dirty` are called with a `key` so that each tracker will trigger an update only when his individual `key` would get marked as dirty. * `dirtyAll` will mark all keys as dirty and trigger an update for all of them. From aa596ec1f76b42a10b57e47a5f49af2938d8a66e Mon Sep 17 00:00:00 2001 From: Damian Tarnawski Date: Sun, 29 Sep 2024 13:24:19 +0200 Subject: [PATCH 6/7] Add changeset for set --- .changeset/long-squids-report.md | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 .changeset/long-squids-report.md diff --git a/.changeset/long-squids-report.md b/.changeset/long-squids-report.md new file mode 100644 index 000000000..ef7969523 --- /dev/null +++ b/.changeset/long-squids-report.md @@ -0,0 +1,9 @@ +--- +"@solid-primitives/set": minor +--- + +Fixes for ReactiveSet (#688): +- Iterators and `.forEach()` no longer track specific keys. +- Added support for `thisArg` as per `forEach` spec +- `super.clear()` now called before dirtying signals +- Uses new `dirtyAll` form trigger package From 32fcb818210273b06e30b7dda81486757fd379c3 Mon Sep 17 00:00:00 2001 From: Damian Tarnawski Date: Sun, 29 Sep 2024 13:25:26 +0200 Subject: [PATCH 7/7] Create swift-points-dance.md --- .changeset/swift-points-dance.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/swift-points-dance.md diff --git a/.changeset/swift-points-dance.md b/.changeset/swift-points-dance.md new file mode 100644 index 000000000..003226769 --- /dev/null +++ b/.changeset/swift-points-dance.md @@ -0,0 +1,5 @@ +--- +"@solid-primitives/trigger": minor +--- + +Add `dirtyAll` to `createTriggerCache`