davestewart / vuex-pathify

Vue / Vuex plugin providing a unified path syntax to Vuex stores
https://davestewart.github.io/vuex-pathify
MIT License
1.37k stars 57 forks source link

Vue DevTools bug #121

Closed bryanjamesmiller closed 3 years ago

bryanjamesmiller commented 3 years ago

Describe the bug When I try to update a nested property in an object that is in an array, Vue DevTools does not show the mutation or correct store data.

Expected behaviour I would expect Vue DevTools to show the mutation.

Additional context I can tell the store data itself IS getting updated as expected. It's just I can't see it in Vue DevTools and that makes me worry I'm doing something wrong. It's possible I am doing something wrong - let me know what you think of the code below.

Code example

Super simple example - only the relevant pieces of code are included.

Vue Component:

    computed: {
        ...sync('MyModule/*')
    },

    mounted() {
         // The below ends up evaluating to something like: this.formFields[0]['my_object_property'] 
         this.formFields[index][this.dynamicString] = 'some string';

        // This forces Vue DevTools to update and shows a mutation, otherwise it does not.
        this.formFields = this.formFields;
    }

MyModule:

const state = {
    formFields: []
};

const mutations = {
    ...make.mutations(state)
}

const actions = {
    ...make.actions(state)
}

Final note: Everything is working correctly as far as I can tell, except Vue DevTools simply does not show the mutation happened unless I add the ridiculous line this.formFields = this.formFields;. This does make me wonder if I am doing something wrong. Thanks for any help or if this is a bug, if you can fix!

Side question: With the code setup given above, running this.formFields = 'anything' is actually using actions, correct? I know it may be unnecessary to include the actions, but just wondering.

bryanjamesmiller commented 3 years ago

I also tried the "Reactivity in Depth" type syntaxes, which didn't solve the problem either:

           const myObject = _.cloneDeep(this.myArray[index]);

           myObject[this.myKey] = 'some-string';

           Vue.set(this.myArray, index, myObject);
davestewart commented 3 years ago

Hey hey,

Yes, sorry - this can be confusing!

So you should turn on strict mode, then Vuex will start complaining that you are mutating a property without a commit.

The reason for this is that you are "reaching into" the property in the store...

this.myArray[index][this.dynamicString] = 'some string';

...vs hitting the actual mutation helper created by sync().

There is more info about that here:

It's been a while since I used the array stuff, but you need to let the mutation helper do the work, or use $store.set().

computed: {
  // make mutation handlers for each of the sub-properties
  ...sync('module/prop@*')
},

Try:

this.$store.set(`module/prop@.0.key`, value)

The index stuff is useful in certain cases (from memory, reading) but is not a silver bullet.

You could try using a method to write more easily:

setValue(index, key) {
  this.$store.set(`module/prop@.${index}.${key}`, value)
}

Does that make more sense?

bryanjamesmiller commented 3 years ago

Thanks for the quick reply!

I did not change my computed property, but just used this.$store.set(`module/prop@.0.key`, value) and now I see the mutations showing up as expected, without needing any of the reactivity-in-depth or var = var shenanigans.

Two quick follow up questions:

1) Is that sufficient to do things correctly or do I really need to change my computed property too as you suggested?

2) Just to confirm, my app is using actions to make these (and all) mutations or am I not utilizing what I am trying to do with:

const actions = {
    ...make.actions(state)
}

Thanks again!

davestewart commented 3 years ago

Well... make.actions() is essentially a helper for those actions-only type of developer (they essentially just pass the values to the mutation).

Unless I am doing any additional logic, or async stuff, I generally just do a direct mutation, which is covered by sync() or $store.set().

Regarding the computed property... is up to you. All roads lead to roam / it's all just setting values :)

davestewart commented 3 years ago

Also, just noted that you are using a form:

this.formFields = this.formFields;

An easy win would be to clone any object in the store to your component, get and set the values on that using basic JS, then commit that object to the store only when you submit the form.

This makes for a far simpler flow, and you get the added advantage of being able to NOT submit / cancel if you need to!

bryanjamesmiller commented 3 years ago

@davestewart thanks again for the quick helps. One last(?) question if you don't mind:

Am I correct in assuming that:

this.$store.set(`module/prop@.${index}.${key}`, value)

properly updates the object in the store using Vue.set or something similar to protect the fact that these state items are reactive (I am thinking here of the cautions described here: https://vuex.vuejs.org/guide/mutations.html#mutations-follow-vue-s-reactivity-rules )?

Thanks again, you da man!

davestewart commented 3 years ago

No problem!

So the first comment I linked above describes the boundaries, and the second comment describes the process CS takes to ensure a path is converted into a valid commit. There's some logic before and after the boundary, but yes, a commit is performed.

Essentially, everything before the @ is the normal Vuex path to the top-level property, and everything after the @ is used inside the mutation that make.mutations() creates to set the right sub-property.

Seeing as it sounds you are maybe (correct me if I am wrong) new-ish to Vuex... one last question: are you sure you need Vuex for this piece of state?

If you want to unpack what you are doing (so, tell me what you are actually doing, vs pseudocode) maybe I might be able to offer some thoughts (or, maybe not!).

EDIT: I ask, because it may be that for something this complex, Vuex and its paths and plugins etc might be more trouble than its worth. Take a look at another project of mine, Vue Class Store and let me know if something like that might be a simpler solution.

bryanjamesmiller commented 3 years ago

Your other package looks cool and promising, but in this case I think we have to stick with Vuex, and so using Pathify does reduce the overhead with Vuex and we're glad to use it. It's been a while since I used Vuex and Pathify, but I've used them both before. I greatly appreciate the interaction though as it's been most helpful.