Fix and optimize ReactiveMap and ReactiveWeakMap #704
Conversation
|
| @@ -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"); | |||
There was a problem hiding this comment.
Changing to object as now we need to track object keys and values in both caches
| #keyTriggers = new TriggerCache<K | typeof $KEYS>(); | ||
| #valueTriggers = new TriggerCache<K>(); | ||
| #keyTriggers = new TriggerCache<K | typeof $OBJECT>(); | ||
| #valueTriggers = new TriggerCache<K | typeof $OBJECT>(); |
There was a problem hiding this comment.
Values will have an object trigger as well to support and values, entires and forEach
| return super.size; | ||
| } | ||
|
|
||
| *keys(): MapIterator<K> { | ||
| this.#keyTriggers.track($OBJECT); |
There was a problem hiding this comment.
Now, keys invalidate whenever a key has been added, changed or removed. No matter if a key was used or not.
This is Removing the same keys optimization as in Set, as we cannot assume a break will happen consistently on each computation.
| for (const [key, v] of super.entries()) { | ||
| this.#valueTriggers.track(key); | ||
| yield v; | ||
| this.#valueTriggers.track($OBJECT); |
There was a problem hiding this comment.
Values are no longer tracking changes in keys changes. Instead they track only values changes.
This is also Removing the same keys optimization as in Set, as we cannot assume a break will happen consistently on each computation.
| *entries(): MapIterator<[K, V]> { | ||
| this.#keyTriggers.track($OBJECT); | ||
| this.#valueTriggers.track($OBJECT); |
There was a problem hiding this comment.
Entries are now tracking global changes to keys and values, ensuring they will be dirtied whenever anything has changed.
This is also Removing the same keys optimization as in Set, as we cannot assume a break will happen consistently on each computation.
| this.#keyTriggers.dirty(key); | ||
| } | ||
| if (hasChanged) { | ||
| this.#valueTriggers.dirty($OBJECT); |
There was a problem hiding this comment.
Global values are dirtied for values, entries and forEach support
| this.#keyTriggers.dirty($KEYS); | ||
| this.#valueTriggers.dirty(key); | ||
|
|
||
| if (isDefined) { |
There was a problem hiding this comment.
Value trigger is only dirtied if the value actually has changed.
| batch(() => { | ||
| this.#keyTriggers.dirty($OBJECT); | ||
| this.#valueTriggers.dirty($OBJECT); |
There was a problem hiding this comment.
This is important: we do dirty global values trigger always, as values() may depend on order. Even though the actual value of the key may not change, removing it, will have impact on amount of iterations in *values()
| } | ||
| super.clear(); | ||
| this.#keyTriggers.dirty($KEYS); | ||
| this.#keyTriggers.dirtyAll(); |
There was a problem hiding this comment.
clear is now optimized using new dirtyAll()
| clear(): void { | ||
| if (super.size) { | ||
| super.clear(); |
There was a problem hiding this comment.
clear is now fixed, calling .clear() at before dirtying
This PR is improving and fixing ReactiveMap and ReactiveWeakMap, following up on #593
All noticeable changes have been commented. @thetarnav please let me know in case you have more questions :)