Subscribe on changes!

Checkbox v-model array mutation

avatar
Dec 1st 2020

Version

4307610c35255477125c12217be514da068bedb0. This is a problem with the latest master branch, caused by 87581cd2662d6db21b35db7a0b5df2090c8a7fe2.

This is not a problem with 3.0.3.

Reproduction link

https://codesandbox.io/s/stupefied-kirch-4865h?file=/index.html

Steps to reproduce

Check a checkbox and note the console warning about mutating store state outside a mutation.

My example uses Vuex to illustrate the problem but this is not a problem with Vuex itself.

What is expected?

Using v-model with an array on a checkbox should not mutate the array.

What is actually happening?

The array is mutated.


PR #2663, merged yesterday, makes a significant change to how v-model behaves. Based on the comments on that PR I don't think the full ramifications of that change have been properly considered. I think that change should be reverted.

The example I've created uses Vuex but that isn't really important. The key point is that a data object should only be mutated by its owner. In this case that's the store but it could equally be an ancestor component, passing the array down via a prop. Standard one-way data flow.

From a one-way data flow perspective an <input> is no different from a custom component. The <input> 'component' is not the data's owner, so it shouldn't be modifying it. Standard v-model semantics are to assign a new value to the property instead.

Further, my use of a computed property to bind store state is just one possible use for a computed property. The new v-model behaviour completely bypasses the set. There could be all manner of other logic within the set that will now be bypassed. In general, mutating an object/array returned by a computed property is not safe. I can provide examples of realistic use cases if required.

The original problem that motivated #2663 was updating a reactive([]) created within setup. However, that problem is not unique to <input type="checkbox">. It equally applies to <select multiple> and to custom input components. Why single out checkboxes as a special case?

Perhaps an alternative would be a modifier on v-model, for both checkboxes and selects, to enable mutations? That would retain backwards compatibility and keep one-way data flow as the default. Custom component authors would be able to implement the same modifier, allowing custom components to maintain consistency with native inputs.

In my opinion, using Set with v-model should also be changed to avoid mutating the Set for exactly the same reason. I've seen arguments around performance but in practice the number of checkboxes would have to be unrealistically huge for that to be a significant consideration.

avatar
Dec 1st 2020

I agree. I rather have a caveat of not being able to use a reactive([]) with v-model than loose the possibility of computed setters which is one of the most useful features of Vue to deal with data with side effects

avatar
Dec 1st 2020

I think reverting makes sense since the breakage is bigger than expected.

However, the issue raised in #2662 will need to be clearly documented as a caveat (/cc @vuejs/docs) - I couldn't find a way to detect the case and emit a runtime warning (we should probably still try to find a way to warn against this).

avatar
Dec 1st 2020

closed via 83a79a82