gertqin / vuex-class-modules

Typescript class decorators for vuex modules
MIT License
193 stars 20 forks source link

Support for multi argument mutation #12

Closed 2-5 closed 5 years ago

2-5 commented 5 years ago

I was wondering if it is possible to improve support for multi argument mutations.

At the moment this seems to be the only way of allowing multiple arguments:

@Mutation
public mutate(payload: { a: string; b: string }): void {
  this.a = payload.a
  this.b = payload.b
}

If possible, this would be better:

@Mutation
public mutate(a: string, b: string): void {
  this.a = a
  this.b = b
}
bodograumann commented 5 years ago

That would indeed ease usage of the modules. I have been in this situation myself several times; mainly with actions though. Instead of your first example I would write:

@Mutation
public mutate({ a, b }: { a: string; b: string }): void {
  this.a = a;
  this.b = b;
}

Considering that there might be more than two parameters and I tend to use much longer names, that sometimes gets a little frustrating.

In plain old vuex, modules interpreting multiple direct parameters is not possible, because the mutation also gets the state as argument and an action would get get a context argument. Here it might just work.

For the sake of backwards compatibility the case with one parameter must not be wrapped in an object. Luckily giving multiple parameters was previously not possible; so that can be implemented as we like.

One problem that might come up is, that if–in the future–vuex decides to add more arguments to the mutation or action calls, we can not simply pass them to our methods. I am not sure how likely this scenario is, but personally I don’t consider it a show stopper. If the situation comes up, we can always workaround it, by having to explicitely choose to get the new argument, e.g. through an options argument to the @Mutation or @Action decorators.

So to sum it up: I would love to see this :slightly_smiling_face:

gertqin commented 5 years ago

Hmm, I don't really like it. To keep this project clean I think it should stick as much to the original vuex syntax as possible (but provide good typings).

Let's say this is implemented by always changing the parameters to an array for the actual vuex mutation, and then back to multiple arguments. Now if you are using Vue Devtools or are subscribing to a mutation, you might wonder why an array is showing up as payload - especially if you are only writing one-parameter mutations.

So as long as vuex mutations/actions only allow one parameter payload, I think vuex-class-modules will stick to the same restriction.

bodograumann commented 5 years ago

I was actually thinking of converting the arguments into an object, but when you talked about arrays, I had another look into this. Turns out, in contrast to e.g. python, in javascript you can not see the parameter names of a function. (There are some ugly hacks out there which convert the function into a string, but I would not want to touch that.) In light of that, the feature seems much less desirable, because seeing the actual mapping in the vue devtools is important to me.

gertqin commented 5 years ago

Yes, I'll close it.