-
Notifications
You must be signed in to change notification settings - Fork 546
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
Should v-model-checkbox mutate its array? #178
Comments
This seems related to #140 and #175 (which I see you've mentioned already in #156). I might think about this differently, as I wouldn't intuitively think an array should mutate like that. Generally, I don't expect to pass an object down to have it mutated, I'd expect the parent to be notified about the update. So to make the fiddle work, I would add a computed in Child The above wouldn't address the fact that the data would need to be reactive. I do agree on your reactivity point. It does seem like you need to add reactivity in a use case where you maybe don't need it. But for most people in most use cases, I'd expect they'd just make it reactive to ensure the list stays in sync and continues to work properly if they end up using that list in more than one way (i.e. in a way that needs it to be reactive). Not sure what the solution is there because I would find v-model mutation confusing (something like |
About those other issues: yes and no.
Yes. This (or a variation) is certainly the easiest way to make it work currently.
Passing a readonly thing to a v-model feels weird to me, but I see how you could make it work. I don't think there's a right/wrong design in the absolute here and there's previous art for mutating an array bound to list of checkboxes, e.g. Knockout. To be honest I wouldn't really care how it works if I didn't have that weird issue at the moment :/ The only nice thing about mutating the array is that you can then bind a list of checkboxes to a non-assignable array and still get the correct results out of it. |
That'll be a breaking change for computed models with marginal gains in very specific scenarios, so for me it doesn't seem to be worth going the breaking path. const model = {
get() { return this.modelValue },
set(value) {
mutateArray(this.modelValue, value)
}
} |
If the v-model of a checkbox is an array, currently Vue creates a new copy of that array for each change and assigns it (if possible...).
This comes with some drawbacks that make me wonder whether mutating the array would be a better idea.
You can play with and try to fix this fiddle to get a feel for the situations where it's not ideal.
https://jsfiddle.net/3g89mswr/7/
The fact that you need to be able to assign is restrictive in some situations. In the fiddle I try to use a prop from a stateless (could be functional) component but props are not assignable.
Not only must the target be assignable but it must also be reactive. Even if you don't intend on reacting to the changes (in the example above, I only read back the array when clicking on a button) a non-reactive property holding this array is buggy because of the way v-model-checkbox work. If the property is not reactive any change will always be applied to the initial array (and previous changes are lost).
One argument could be made that similar approach would also fail for a string prop with v-model on a textbox.
I would agree with that, but I'm not sure if the comparison is really obvious. I intuitively thought the array would mutate and I was surprised when it didn't. Also the text input version would work even with a non-reactive source property.
Using mutations would IMHO make Vue work in more situations and reduce friction.
The text was updated successfully, but these errors were encountered: