mobxjs / mobx-state-tree

Full-featured reactive state management without the boilerplate
https://mobx-state-tree.js.org/
MIT License
6.98k stars 641 forks source link

Add types.set #381

Open the-noob opened 7 years ago

the-noob commented 7 years ago

I have a few stores where I need a reference to be unique (i.e. tags for a product)

Since every mutation has to go through an action it can be catered for there.

  function addTag(entity) {
    if (!self.tags.has(entity.id)) {
      self.tags.put(entity);
    }

    return self.tag.get(entity.id);
  }

It would be nice to have types.set with only tags.add :)

stgolem commented 7 years ago

This is another reason to have ability to create custom types for MST. Here all we have is private Map and public api to interact with it. It would be nice to make it possible to create such Type as it is done inside MST with Maps, Arrays or in some ligther way.

Imagine this as a prototype code:

const setType = types.userType("Set")
    .basedOn(types.model)
    .privateProps(baseType => {
        map: types.map(baseType)
    })
    .actions(self => {
        return {
            function add(entity) {
                 if (!self.map.has(entity.id)) {
                      self.map.put(entity);
                  }
                  return self.map.get(entity.id);
            }
        };
    });

This is totally possible to do with simple types.model, but the concept is more generic and allows us to create extension types for MST and easy reuse them across community.

Idered commented 6 years ago

Is this issue open for PR? I can try to implement types.set.

philspitler commented 6 years ago

@Idered - given that this was reopenened and the announcement requesting more maintainers (#700) , I'd think PRs are very appreciated.

robinfehr commented 6 years ago

@Idered i guess it would be nice to have it in mobx first so we don't have to implement an enhanced map as the workaround. have a look at: https://github.com/mobxjs/mobx/issues/69

flybayer commented 5 years ago

Now that this has been added to mobx, can it be added to MST?

kresli commented 4 years ago

👀

sbussard commented 4 years ago

Please

mweststrate commented 4 years ago

"help/PR welcome"

kresli commented 4 years ago

What should the patches' path reference to?

{
  op: "add",
  path: '', // what should be in path?
  value: { title: 'my-title' },
  oldValue: undefined
}

{
  op: "remove",
  path: '' // what should be in path?,
  oldValue: {title: 'my-title'}
}

EDIT: I think its obvious to use root path path: "/"

But then how we would process the remove patch? There is no key or index to track. Any suggestions @mweststrate ?

mweststrate commented 4 years ago

I think the path should only be the path to the set itself, not a specific child, so for a top level set, i'd expect it to be empty indeed. I think immer might be a good reference point, iirc it always replaces the full contents on the parent position, rather than having a removal / add operators.

Yamo93 commented 2 years ago

Why is this missing in MST? Sets are a quite common data structure.