The library needs an appropriate way to mutate a sub-property within an action

[Balour]

Hi,

As briefly discussed in #51 it seems there is no proper way of mutating a sub-property of the state within an action. #24 also mentions something similar

using store.set('ui/loggedUser@id', value) has the potential to infinitely loop in certain cases.

Here's a sample project illustrating the problem. We can go around it by passing a new Payload object to the commit manually.

https://codesandbox.io/s/7zxozyo9r6

Thank you again for all your work

[davestewart]

OK so I took a look at this.

TBH, I don't really use the lib much these days, so I'm a bit rusty!

Here's a demo with a bunch of ways set data:

• https://codesandbox.io/s/kx13l01j6o

You mention in your demo that using store.set() gets into a loop. I have not found this to be the case, so perhaps you can check and confirm.

The most promising ways are probably the helpers make.commit which makes a namespaced commit and payload which is a bit of sugar around making a new payload. Check store/utils.js.

The partial payload test is something I just though in there, but didn't expect to work as the lib code doesn't support it.

So... your options are:

• vanilla store.set(). it works, though is a bit verbose
• either of the two helpers I created above. I may at some point look to put one in the lib, but my concern is that I'm adding more code to confuse people.

Let me know how you get on!

[Balour]

* https://codesandbox.io/s/kx13l01j6o

Thanks a bunch

You mention in your demo that using store.set() gets into a loop. I have not found this to be the case, so perhaps you can check and confirm.

Only in this particular scenario, I suspect it may have something to do that it's an action that's called in the beforeMount. I can confirm it loops on my home pc too. In my original codesandbox, without that line, the page runs fine and the state is outputted to console. If I uncomment the import and the line, it never makes it to output the state to console it seems like as the result is not there. In other modules where this was not the case, store.set worked perfect honestly

The most promising ways are probably the helpers make.commit which makes a namespaced commit and payload which is a bit of sugar around making a new payload. Check store/utils.js.

I really like this, it's pretty elegant and could be a good solution for these cases

The partial payload test is something I just though in there, but didn't expect to work as the lib code doesn't support it.

So... your options are:

* vanilla `store.set()`. it works, though is a bit verbose

* either of the two helpers I created above. I may at some point look to put one in the lib, but my concern is that I'm adding more code to confuse people.

I think my favorite would have to be loadUser3 in your example. It uses the path syntax and it's short and sweet which is great. However I could see how it could be confusing as commit would be ambiguous to the commit from the context object.

The store.set method has the advantage of being very explicit and uniform across the code but the short version through the helpers are quite nice too

I think that I will go with the helpers as store.set seems to throw me for a loop in this case. In my project code, it starts sending hundreds of requests to the server and in the codesandbox it locks up the navigator tab for me

Thanks again, I will report back tomorrow after working on it

[Balour]

Reporting back

So I tried the helper commit but for that particular case it still does the loop thing, on both firefox and chrome

however the namespaced commit really makes store.set lines much cleaner

I also very much like the payload function, again makes the code a bit cleaner when using that

for now I'll be using the store commit in combination with the payload function for the one where it loops and I think I'll change store.set lines to use the helper commit as store.set in sub modules start getting long depending on the names of them

Thanks

[davestewart]

It would be good to get to the bottom of the loop you mention.

If you stick a breakpoint in, can you spot the loop entry point?

[Balour]

Yes, on my original codesandbox, if I look at the normal behavior of the code, it normally makes the promise, then resolves and goes for the commits

However if the line is uncommented, the promise is made multiple times and resolves multiple times. The breakpoints on the commits never fire until after the browser asks to kill a page that is slowing you down, it then breaks out of the loop and then stops on the commits

I would think that using store.set while in a beforeMount might be the problem

[davestewart]

OK, so the looping was fairly easy to figure out.

This must be a difference in coding styles between people, but the reason your looping is happening is because of a feature of Pathify called "accessor priority", and I'll explain why.

• https://davestewart.github.io/vuex-pathify/#/api/properties?id=accessor-priority

The bit of text that is key, is "a mapped action will be prioritised over a mapped mutation (as the action will call it)".

This basically means that if Pathify finds a setValue action and a value mutation, then you call set() or store.set() it will take the action over the mutation.

The reason for this is to keep the syntax simple, then it's up to the user to decide whether they want an dispatch which will call the commit, or just a commit.

In your case, you have an action called "setLoggedUser" and a mutation called "loggedUser".

What's happening with the store.set, is that store.set('ui/loggedUser') – where you expected it to be a commit – is being converted into an action dispatch('ui/setLoggedUser') because of the mapping algorithm and so the code is looping infinitely.

There's not much I can do about that from a technical point of view, as it's a library feature, and I guess just one of the pitfalls of abstraction.

I think I mentioned in another post that your code could do a bit of a cleanup, and this isn't that, but it's related; your naming is kinda "wooly" (no offence).

Actions generally load things, so it pays to use verbs like "load" and try to steer away from words like "set" because, actually it's the mutation that's doing the set.

So, unlucky naming really! Nothing more than that.

Here's your demo working:

• https://codesandbox.io/s/olzw4p743z

Good luck with the project.

I'll look to get one or both of those additions in at some point.

[Balour]

That makes perfect sense!

I think I will take this opportunity to have better names haha

Thanks for all the support and the lib!

[davestewart]

No probs! Thanks for raising some valid issues :)

[davestewart]

See #79