fix for #41, reactive nested properties and dynamic properties support #54
Conversation
| // console.log('BEFORE: prev[key]:', prev[key]); | ||
|
|
||
| if (index < array.length - 1) { | ||
| Vue.set(prev, key, prev[key] ? prev[key] : {}); |
There was a problem hiding this comment.
Is it really necessary to set the value of prev[key] to prev[key] again? I guess this could affect performance in a negative way.
There was a problem hiding this comment.
Vue.set is the only way to ensure deep reactivity (https://vuejs.org/v2/guide/reactivity.html).
I could save Vue.set call with a direct assignment if the object property is not responsive (check the existence of a 'ob' attribute) but I think Vue.set does that anyway.
|
|
||
| expect(store.state.form.foo1.foo11.foo2.foo3).toBe(`fooValue`); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Thanks for adding an integration test, please also add / update Unit Tests to bring back coverage to 100%.
Thank you for contributing!
|
think I've covered all change requests |
Thanks! I'll take a closer look later this week. |
|
after further reflection i'dd like to cancel the pull request. |
|
While adding whole chunks of a chain seems like it could be excessive, adding properties at the end of the chain seems useful to me. I was thinking about pulling at least part of these changes locally into my code. Did you have reasons you thought the first use case was bad? |
|
@fusionbeam could you elaborate on your decision to close this pr? |
5 participants
store commits via updateField now creates reactive properties across the nested chain.
also when dealing with dynamic properties (chains that are not known at compile time) this fix ensures the safety of the nested chain by creating store objects as needed.
the nested properties are made reactive by default according to vue recommendations (https://vuejs.org/v2/guide/reactivity.html#Declaring-Reactive-Properties)