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

mapSetters (similar to mapGetters) for adding setters to mutations #1037

Closed
Shashwat986 opened this issue Nov 1, 2017 · 13 comments
Closed

Comments

@Shashwat986
Copy link

What problem does this feature solve?

There have been lots of situations where I have some variables in state that I need to assign values to. The correct way to modify the state tree is to create a mutation to set the value of the key in the state. This means a lot of repetitive code is written (as shown below):

export default {
  state: {
    name: null,
    value: 0
  },
  mutations: {
    setName (state, name) {
      state.name = name;
    },
    setValue (state, value) {
      state.value = value;
    }
  }
};

$store.commit('setName', 'John Doe');

mapSetters will allow users to create all these setters using a single like of code.

What does the proposed API look like?

import { mapSetters } from 'vuex'

export default {
  state: {
    name: null,
    value: 0
  },
  mutations: {
    ...mapSetters(['name', 'value']),
    //...
  }
};

Usage: $store.commit('name', "John Doe") OR $store.commit('setValue', 4) depending on how it is implemented. I prefer the second approach myself.

@Shashwat986
Copy link
Author

I'm interested in working on this myself, so no effort beyond approval is required from anyone else's end.

@thebrubaker
Copy link

thebrubaker commented Nov 1, 2017

I wrote a simple function where I "bootstrap" the mutations and getters on my module, which might be useful to you:

export function bootstrap (module) {
  if (module.getters === undefined) {
    module.getters = {}
  }

  if (module.mutations === undefined) {
    module.mutations = {}
  }

  Object.keys(module.state).forEach(key => {
    if (module.getters[key] === undefined) {
      module.getters[key] = state => state[key]
    }
    if (module.mutations[key] === undefined) {
      module.mutations[key] = (state, value) => {
        state[key] = value
      }
    }
  })

  return module
}
import { bootstrap } from '~/utilities/vuex'

export default bootstrap({
  namespaced: true,
  state: {
    name: '',
    email: ''
  }
})

This means every state property gets its own mutator and getter, but if you already defined either of those, it will use yours over the bootstrapped defaults.

@Shashwat986
Copy link
Author

Shashwat986 commented Nov 2, 2017

Exactly, something like this should be part of the actual vuex package.

In fact, with my implementation, your code would look something like:

export default {
  namespaced: true,
  state: {
    name: '',
    email: ''
  },
  getters: mapGetters(['name', 'email']),
  mutations: mapSetters(['name', 'email'])
};

which isn't too bad, right?

@thebrubaker
Copy link

Yup you could do that just as well. It certainly wouldn't hurt to add some more utilities, but at the end of the day it's also pretty trivial to implement it yourself. Who knows!

@darrinmn9
Copy link

darrinmn9 commented Nov 11, 2017

@Shashwat986 I like this idea... i was hoping for something similar #953. My only question on this proposal is what if the user wanted the setter to trigger an action. Could your proposal be modified to support that option? or do you feel this should only apply to mutations?

  actions: {
    ...mapSetters(['name', 'value'])
  }

@Shashwat986
Copy link
Author

So, I was having this work by automatically generating the method to set the value in the state. How would that work for actions? If the action is doing the same task, it would be calling a mutation within it, one that would do the job just as well.

@ktsn
Copy link
Member

ktsn commented Nov 15, 2017

This is similar discussion with #91.
As the helper could easily be implemented in userland, we would not include this.

Please see our policy for adding a new feature: vuejs/vue#6004 (comment)

@ktsn ktsn closed this as completed Nov 15, 2017
@Shashwat986
Copy link
Author

So, I don't mean to be rude, but, doesn't vuejs/vue#6004 (comment) mention

If a proposed API's functionality can be cleanly implemented in userland without hacks, we will pass unless it's strongly needed by a significant majority of the user base (i.e. we've seen it been raised/discussed multiple times).

(emphasis mine)

Past issues mentioning the same/similar problems:
#91
#953
#236 (comment)
#236 (comment)
#936
#559

Existing open-source projects that will greatly benefit from this:
https://github.com/zhaohaodang/vue-WeChat/blob/master/src/vuex/mutations.js
https://github.com/Binaryify/vue-tetris/blob/master/src/vuex/mutations.js
https://github.com/liangxiaojuan/vue-Meizi/blob/master/src/vuex/store.js
https://github.com/ericjjj/douban/blob/douban/frontend/store/index.js

I completely understand that we shouldn't add something that isn't useful, and just ends up making the package bloated, but the reason I'm pushing a little for this feature is because it's clearly something that will reduce a lot of dev effort. Having said that, if you feel it shouldn't be a part of vuex, that's fine.

@ktsn
Copy link
Member

ktsn commented Nov 15, 2017

Well, the issues/comments which you provided seem proposing different helpers each other. After reading them, I feel it would be better to leave such helpers in userland because there are various demand for each user.

Also IMO, the most of mutations would not only assign a payload to a state in real world applications. If many of your mutations become like so, it may be too separated or there are other modules for calculating a next state.

vue-tetris has many of mutations that just assign a payload into state but it looks like calculating the app state out of the vuex. https://github.com/Binaryify/vue-tetris/blob/master/src/control/states.js
I don't think such niche use case justifies to include this helper into the core.

Only thing it could be the case would be a form handling. I see some people binds all form elements with different mutations. Then the mutations are bloated. But creating mutation generator may not completely solve the problem because it cannot handle nested or dynamic form.

I'm still investigating about the form handling issue but I think it would make more sense to handle it in component local state and share a part of form data with store which is really needed in another component.

@Austio
Copy link
Contributor

Austio commented Nov 15, 2017

@ktsn, IMO form handling should 100% be in userland b/c many real world applications will have different xhr things and ways they want to handle optimistic vs pessimistic updates for different forms.

@Shashwat986
Copy link
Author

Sounds good. Thanks for all the help!

(P.S.: I'm still keen to contribute because I'm a big fan of Vue. Do let me know if I can help in any way)

@daggerok
Copy link

daggerok commented Dec 20, 2019

It's sad that since two years we are continue implement similar boilerplates...

We really need something like mapSetters or maybe even some more higher-level functionality, for example to eliminate vuex-map-fields dependency from our vue projects when we want use vuex store state as v-model for two way binding...

If I don't want use separate dependecy I have to implement computed properties with setter and getter which are under the hood will use vuex actions and getters... why it should be so complicated and why you guys don't want make our life easier?


Regards

@Shashwat986
Copy link
Author

Shashwat986 commented Dec 22, 2019

For what it's worth, you can use the code in my original PR to make your task easier: #1060


why it should be so complicated and why you guys don't want make our life easier?

I wouldn't go this far. I'm sure it's a number of users situation. If more users want this, we can get it done. Right now, the consensus says this isn't required, as far as I've understood the situation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants