Fixes for ReactiveSet #688
Conversation
🦋 Changeset detectedLatest commit: 32fcb81 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
packages/set/src/index.ts
Outdated
| values(): IterableIterator<T> { | ||
| return this.keys(); | ||
|
|
||
| *values(): IterableIterator<T> { |
There was a problem hiding this comment.
Iterators no longer track specific keys. That optimization was working under the assumption that a BREAK will always happen in specific iteration, which might not be always a case as it can depend on external logic. All iterators should always be rerun if any op to the Set will happen.
There was a problem hiding this comment.
Can you mention that in the docs?
There was a problem hiding this comment.
Sure, Could you point me a place where I should do that?
There was a problem hiding this comment.
Just another section „break optimization” after the „how to use” should work.
There was a problem hiding this comment.
Not sure I understand what is expected here to document to be honest. There is no break optimization anymore, and the previous was a bug. Should't that be mentioned rather in changelog?
packages/set/src/index.ts
Outdated
| values(): IterableIterator<T> { | ||
| return this.keys(); | ||
|
|
||
| *values(): IterableIterator<T> { |
There was a problem hiding this comment.
Can you mention that in the docs?
Hi @thetarnav and @atk :)
Decided to split this draft PR into 2 chunks Set and Map fixes. Starting with Set here. Best to review commit by commit. Let me know where more reasoning is required, I will add comments.
Thanks!