Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Supporting of new Set methods in observable Sets #3850

Closed
inoyakaigor opened this issue Mar 26, 2024 · 3 comments · Fixed by #3853
Closed

Supporting of new Set methods in observable Sets #3850

inoyakaigor opened this issue Mar 26, 2024 · 3 comments · Fixed by #3853
Labels
🍗 enhancement 🙏 help wanted 🎁 mobx Issue or PR related to mobx package

Comments

@inoyakaigor
Copy link
Contributor

Intended outcome:
According to a New Set methods proposal a Set class will have a bunch of new methods (more on MDN). This proposal currently at Stage 3 therefore it means it is becoming part of ECMAScript standart

Current browser's support

Method Firefox (Nigthly) Chrome (> 122) Safari (> 17)
intersection ✔️ ✔️
union ✔️ ✔️
difference ✔️ ✔️
symmetricDifference ✔️ ✔️
isSubsetOf ✔️ ✔️
isSupersetOf ✔️ ✔️
isDisjointFrom ✔️ ✔️

I've created a sandbox with example of code that includes two Sets as store's fields. One of them is observable other one is not.

Actual outcome:
For example if log a store.reactiveSet.intersection we will see an undefined in the console. If log a store.nonReactiveSet.intersection we will see a nonReactiveSet ƒ symmetricDifference() { [native code] } (there I used Chrome 123)
Obviously for other Set's methods result is the same:
image

How to reproduce the issue:

Run code from provided sandbox. Look into browser's console

Versions

Mobx 6.12.1

Also I propose made this as part of Mobx 7 roadmap #3796

@inoyakaigor
Copy link
Contributor Author

Also I think there is only one place for changes that will add support the new methods: mobx/src/types/observableset.ts and it means it is not a big deal but can't imagine what exactly future code should look like?

@mweststrate
Copy link
Member

Great idea. PR welcome. In many cases these kind of operations can be implemented by calling the original version on the Set prototype with iterator of the observable set.

@mweststrate mweststrate added 🍗 enhancement 🙏 help wanted 🎁 mobx Issue or PR related to mobx package and removed 🐛 bug labels Mar 28, 2024
@inoyakaigor
Copy link
Contributor Author

Also I just realised that there also should be added Object.groupBy and Map.groupBy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🍗 enhancement 🙏 help wanted 🎁 mobx Issue or PR related to mobx package
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants