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

Observable sets #69

Closed
kmalakoff opened this issue Dec 9, 2015 · 25 comments
Closed

Observable sets #69

kmalakoff opened this issue Dec 9, 2015 · 25 comments
Milestone

Comments

@kmalakoff
Copy link
Contributor

I have been using observable maps as sets, eg. myMap.set(key, true). Not an urgency, but if sets were provided in mobservable, I'd use them!

@mweststrate
Copy link
Member

Closing this one for now, until more people need it.

@mull
Copy link

mull commented Jul 11, 2016

I just ran into this issue. I'd like to offer up my "need it". Seems like it doesn't work at the moment (though the milestone indicates it should.) Maybe I'm doing something else wrong.

@leebenson
Copy link

would love to see observable sets, too. I have a ton of components that rely on unique values. Currently, we're doing array look-ups (or the .set(val, true) trick @kmalakoff mentioned) which can get expensive when there's thousands of items. Sets solve this neatly.

@gknaxtrader
Copy link

I'd second that, thank you. @computed doesn't react on an observable someSet.clear() action.

@mweststrate
Copy link
Member

I'll reopen it :) PR's are welcome ;-)

@mweststrate
Copy link
Member

Closing. It seems we are pretty okay without, and having only a limited set of complex types (map, array, object) greatly simplifies libraries built on top of MobX

@mull
Copy link

mull commented Apr 7, 2017

Truth be told I feel non-okay without it. Using an observable Map and checking whether some key is === true ends up being very awkward, especially when new developers in the team join in and start using Mobx.

I wish I had the time to actually go ahead and implement it. Truth be told I had a look at doing it through Proxies but quite frankly couldn't navigate the source code well enough as I'm nearly oblivious to Typescript.

How do you feel about a bounty for implementing it? Hell even if it's a second npm module (until properly tested?) I'd be alright with that. See if we who request the feature will put our money where our mouth is!

@matthewrobb
Copy link

matthewrobb commented Apr 11, 2017

@mull Hey, I had this need recently and was able to get most of what I want doing this:

import {observable, ObservableMap } from 'mobx';
const { enhancer: deepEnhancer } = observable.map();

export class ObservableSet extends ObservableMap {
  constructor(initialValues=[], name) {
    super(initialValues.map(v => [v,v]), deepEnhancer, name);
  }

  static create() {
    return new this(...arguments);
  }

  add(value) {
    return this.set(value, value);
  }

  forEach(callback, thisArg = this) {
    for(const value of this) {
      callback.call(thisArg, value);
    }
  }

  [Symbol.iterator]() {
    return this.keys();
  }
}

You might need to override some of the added methods on ObservableMap to be Set-ish but this is all that's required to get base Set functionality to work 1:1 on top of ObservableMap.

@natew
Copy link

natew commented May 24, 2017

+1 on this, sets offer a nicer API for common patterns.

@mweststrate
Copy link
Member

mweststrate commented May 24, 2017 via email

@phpnode
Copy link

phpnode commented May 25, 2017

@mweststrate it's small enough and a really useful feature, why not make it part of core?

@Aldredcz
Copy link

Aldredcz commented Jun 13, 2017

@mweststrate +1 on this. Gotta workaround with Map with true as value :(
As I see it, one of the biggest benefits of MobX is that it mimics native JS objects seamlessly, and tha fact that Set is missing is really annoying.

@phiresky
Copy link
Contributor

phiresky commented Jun 17, 2017

@observable mySet = new Set() just silently creates a ES6 Set, not triggering any observers for changes in the set. This is pretty confusing considering @observable myMap = new Map() works as expected.

It should at least print a warning, but then it could also just create a observable set<T> based on a observable map<T, null>

@bradyep
Copy link

bradyep commented Aug 1, 2017

A set is such a fundamental part of programming that I am surprised to see that they are not supported as observables in MobX, especially now that, as of ES6, they are now a standard part of JavaScript.

@mweststrate
Copy link
Member

mweststrate commented Aug 1, 2017 via email

@zfalen
Copy link

zfalen commented Aug 17, 2017

+1 for this, Sets accomplish most of what simple list, etc. components need for rendering unique values and are just plain cleaner to work with than Maps or even Arrays in a lot of cases.

Plus, Sets just let you store and compare objects, which you can't do with the ObservableMap hack since keys have to be simple data types (Boolean, String, Number)

@mweststrate
Copy link
Member

Sets and Maps won't be supported within the current major, as sets would be limited to strings, as Sets are not natively available on all ES5 complient JS runtimes and it goes well beyond the scope of this project to polyfill them

@danielkcz
Copy link
Contributor

danielkcz commented Feb 12, 2018

Polyfilling shouldn't be a concern for any library, you should just assume that Set is available and you are pretty much safe till user does @observable mySet = new Set(). At that point, it's clear that Set is available. So I find it a bit weak argument that you don't want to support it because of backward compatibility.

Can you please reconsider this? I am joining other people here as it's really awkward using Map for something simple as a unique list of items.

@mweststrate mweststrate reopened this Feb 28, 2018
@mweststrate
Copy link
Member

MobX 4 added support for real Maps, so if anybody wants to volunteer and add Sets in a similar fashion, PR's are welcome :). Please base the PR on mobx4 branch (see #1321)

@Prior99
Copy link

Prior99 commented Mar 19, 2018

I want to chime in and say "I need this". Especially for marking selections on Checkbox groups an there like Sets are almost a must-have.

@mnpenner
Copy link

I wasn't so keen on extending ObservableMap, so here's another approach:

import {observable, action} from 'mobx';

export default class ObservableSet {
    constructor(values) {
        this.map = values ? observable.map(values.map(v => [v, true])) : observable.map();
    }

    add(value) {
        this.map.set(value, true)
        return this;
    }

    delete(value) {
        return this.map.delete(value);
    }

    has(value) {
        return this.map.has(value);
    }

    @action.bound
    set(values) {
        this.clear();
        for(let v of values) {
            this.add(v);
        }
    }

    clear() {
        this.map.clear();
    }

    get size() {
        return this.map.size;
    }

    [Symbol.iterator]() {
        return this.map.keys();
    }
}

I've only been using it for a few minutes, but it seems to be working very well so far.

I added a bonus method set that will let you reset your Set without firing reactions multiple times.

It's still missing a few methods from ES6 Set... I haven't needed those yet.

@jkeruzec
Copy link

jkeruzec commented Sep 12, 2018

Maybe, an implementation with map is fine for MobX4 for IE11 compatibility, and new version of MobX 5 can have the real js Set implementation.

Interested in too !

@stnwk
Copy link

stnwk commented Oct 30, 2018

Would be great to have something like this! @mweststrate

@mweststrate
Copy link
Member

Released in 4.9.0 / 5.9.0! (Note that your browser has to support Set to be able to use this feature)

@lock
Copy link

lock bot commented Jul 21, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs or questions.

@lock lock bot locked as resolved and limited conversation to collaborators Jul 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests