Fix and improve ReactiveSet, ReactiveMap and Trigger #593
Conversation
|
| @@ -50,6 +50,11 @@ export class TriggerCache<T> { | |||
| this.#map.get(key)?.$$(); | |||
| } | |||
|
|
|||
| dirtyAll() { | |||
There was a problem hiding this comment.
dirtyAll is needed to be able to clean all tracked keys. Otherwise the user has no access to keys and not able to clear them without own keys tracking.
This is was an issue for Set and Map implementation where we have iterated over currently defined keys not all tracked keys.
| export function createWeakSet<T extends object>(initial?: T[]): ReactiveWeakSet<T> { | ||
| return new ReactiveWeakSet(initial); | ||
| export function createWeakSet<T extends object = object>( | ||
| values?: readonly T[] | null, |
There was a problem hiding this comment.
Interaface updated from latest WeakSet types definition
| } | ||
| } | ||
|
|
||
| /** @deprecated */ | ||
| export type SignalSet<T> = Accessor<T[]> & ReactiveSet<T>; | ||
|
|
||
| /** @deprecated */ | ||
| export function createSet<T>(initial?: T[]): SignalSet<T> { | ||
| const set = new ReactiveSet(initial); | ||
| export function createSet<T>(values?: readonly T[] | null): SignalSet<T> { |
There was a problem hiding this comment.
Interaface updated from latest Set types definition
| @@ -104,41 +114,47 @@ export class ReactiveSet<T> extends Set<T> { | |||
| export class ReactiveWeakSet<T extends object> extends WeakSet<T> { | |||
| #triggers = new TriggerCache<T>(WeakMap); | |||
|
|
|||
| constructor(values?: Iterable<T> | null) { | |||
| constructor(values?: readonly T[] | null) { | |||
There was a problem hiding this comment.
Interface updated from latest WeakSet types definition
| for (const v of super.keys()) this.#triggers.dirty(v); | ||
| super.clear(); | ||
| this.#triggers.dirty($KEYS); | ||
| this.#triggers.dirtyAll(); |
There was a problem hiding this comment.
This is an important fix, to clear all tracked keys not only the ones currently defined.
| clear(): void { | ||
| if (super.size) { | ||
| super.clear(); |
There was a problem hiding this comment.
clearing, and all super operations should always happen before dirtying, otherwise the computations can have access to an old state.
| super.forEach(callbackfn as any); | ||
|
|
||
| for (const value of super.values()) { | ||
| this.#triggers.track(value); |
There was a problem hiding this comment.
While using forEach on Set we could track the used values to trigger a recomputation when a value gets updated.
| return this.values(); | ||
| } | ||
| forEach(callbackfn: (value: T, value2: T, set: this) => void) { | ||
| forEach(fn: (value1: T, value2: T, set: Set<T>) => void): void { |
There was a problem hiding this comment.
Update typings with latest definitions
| for (const key of super.keys()) { | ||
| this.#triggers.track(key); | ||
| yield [key, key]; | ||
| for (const entry of super.entries()) { |
There was a problem hiding this comment.
using entries() for better memory management (no need to spawn new arrays while yielding)
| return super.has(v); | ||
|
|
||
| keys(): IterableIterator<T> { | ||
| return this.values(); |
There was a problem hiding this comment.
In case of Set for correctness keys are equal values, not the other way around.
| return new Proxy(() => [...map], { | ||
| get: (_, b) => map[b as keyof typeof map], | ||
| }) as SignalMap<K, V>; | ||
| } | ||
|
|
||
| /** @deprecated */ | ||
| export function createWeakMap<K extends object, V>(initial?: [K, V][]): ReactiveWeakMap<K, V> { | ||
| return new ReactiveWeakMap(initial); | ||
| export function createWeakMap<K extends object = object, V = any>( |
There was a problem hiding this comment.
Updated typings with latest TS definitions
| } | ||
| } | ||
|
|
||
| /** @deprecated */ | ||
| export type SignalMap<K, V> = Accessor<[K, V][]> & ReactiveMap<K, V>; | ||
|
|
||
| /** @deprecated */ | ||
| export function createMap<K, V>(initial?: [K, V][]): SignalMap<K, V> { | ||
| const map = new ReactiveMap(initial); | ||
| export function createMap<K, V>(entries?: readonly (readonly [K, V])[] | null): SignalMap<K, V> { |
There was a problem hiding this comment.
Updated typings with latest TS definitions
| batch(() => { | ||
| this.#keyTriggers.dirty(key); | ||
| this.#valueTriggers.dirty(key); | ||
| if (currentValue !== undefined) this.#valueTriggers.dirty(key); |
There was a problem hiding this comment.
This is an optimization to not dirty a state for the same values.
| this.#valueTriggers.dirty(key); | ||
| super.set(key, value); | ||
| if (!hasKey) this.#keyTriggers.dirty(key); | ||
| if (value !== currentValue) this.#valueTriggers.dirty(key); |
There was a problem hiding this comment.
This is an optimization to not dirty a state for the same values.
| super(); | ||
| if (initial) for (const v of initial) super.set(v[0], v[1]); | ||
| if (entries) for (const entry of entries) super.set(...entry); |
There was a problem hiding this comment.
I should probably use apply here to avoid destructering
| clear(): void { | ||
| if (super.size) { | ||
| super.clear(); |
There was a problem hiding this comment.
clear should happen before dirting in order to avoid old state in computations
| batch(() => { | ||
| this.#keyTriggers.dirty(key); | ||
| this.#keyTriggers.dirty($KEYS); | ||
| this.#valueTriggers.dirty(key); | ||
| if (currentValue !== undefined) this.#valueTriggers.dirty(key); |
There was a problem hiding this comment.
optimization to not dirty a value that is already undefined
| this.#valueTriggers.dirty(key); | ||
| super.set(key, value); | ||
|
|
||
| if (value !== currentValue) this.#valueTriggers.dirty(key); |
There was a problem hiding this comment.
optimization to not dirty a value that is already the same value
| *entries(): IterableIterator<[K, V]> { | ||
| for (const entry of super.entries()) { | ||
| this.#keyTriggers.track(entry[0]); |
There was a problem hiding this comment.
entries should track keys as well
No description provided.