Gotcha with ReactiveSet/Map iterators

[ollpu]

Describe the bug

Using .keys(), .values() or .items() on a ReactiveSet or ReactiveMap is meant to track all keys/values for changes. However, if the iterators are not consumed, then no tracking is registered.

This comes up when trying to declare dependencies of an asynchronous effect beforehand.

createEffect(() => {
  // Track set for changes
  void set.values();

  setTimeout(() => /* Use set contents... */);
});

Though the intention was to track the full content of the set, the above effect is not actually triggered by changes to the set.

---

The reason this happens is that the methods are implemented as generators like so:

solid-primitives/packages/map/src/index.ts

Lines 49 to 55 in 507ff66

	*values(): MapIterator<V> {
	this.#valueTriggers.track($OBJECT);
	for (const value of super.values()) {
	yield value;
	}
	}

The beginning of the method is not run until .next() is called for the first time.

I think the methods should not be generators themselves, rather just return super.values(). That would probably be more performant as well. SvelteSet & Map are implemented like this.

Happy to to write a PR if there is agreement.

Minimal Reproduction Link

https://stackblitz.com/edit/github-r2lfqdu4?file=src%2FApp.tsx

[thetarnav]

You're listening to values that you've read, seems fine to me.
You need to [...thing.values()]
The same way that you need to {...props}

Although Object.values(store) and Object.values(props) does track field value changes, but it returns the values as an array, not lazy iterator.

[ollpu]

There's an argument to be made, but I think the current behavior is just an unnecessary surprise.

Is there any case where this is a useful optimization? Why would you call .values(), and then dynamically choose not to iterate it at all?

In my case the [...thing.values()] would be unnecessary work: I don't want a copy of the whole container, I just want to track it and potentially iterate through it later.