logaretm / vee-validate

✅ Painless Vue forms
https://vee-validate.logaretm.com/v4
MIT License
10.71k stars 1.25k forks source link

Validation behavior when removing rows from field array #4017

Closed dkliemsch closed 1 year ago

dkliemsch commented 1 year ago

What happened?

When removing rows from a field array the validation is triggered for some of the remaining rows (seems like always the last untouched row is affected).

Reproduction steps

  1. see the codesandbox demo link
  2. remove one of the rows (e.g. question 3)
  3. the validation of the last row (question 8) is triggered
  4. when removing another row (e.g. second one), the error of question 7 is set
  5. ...

Version

Vue.js 3.x and vee-validate 4.x

What browsers are you seeing the problem on?

Relevant log output

No response

Demo link

https://codesandbox.io/s/amazing-wright-n9bjih

Code of Conduct

logaretm commented 1 year ago

Sorry for the late reply. This is a tricky one since the validation occurs because the last answer value seems to have been changed. This isn't correct of course but because of a race condition, the value swapping happens after the old field was removed making the new value undefined when it should be an empty string instead.

I will try to fix this, but as a workaround, you could disable validateOnValueUpdate option on useField and use events to validate your fields instead.

dkliemsch commented 1 year ago

All right I see, that sounds pretty annoying. A fix would be nice, but the workaround is totally fine for us! So thanks for pointing me in the right direction - I hadn't though of that possibility

viajes7 commented 1 year ago

@logaretm

I found that a few days ago, new feature were added:fix: only validate when one of the setters is ran closes

Is this a broken change ?

It means that to trigger the validation of useField, you must modify the value returned by useField (modify the value returned by useForm will not trigger).

An example(used vee-validate@4.8.4): https://codesandbox.io/s/vibrant-knuth-sm3ds2?file=/src/App.vue

In the latest version, this will not work because the validator will not triggered.

Will you consider forward compatibility?

And if child component use useVModel with options {deep: true}, it will into an endless loop, can we fix it in vee-validate?

  1. child componet: An array value changed, emits update:modelValue
  2. parent component: use v-model="value" to bind useField's value,so, it's setter will be invoked
  3. valueProxy setter setValue: value.value = newValue;
  4. finally, in child component useVModel will emits update:modelValue again
    function setValue(newValue, shouldValidate = true) {
        value.value = newValue;
        if (!shouldValidate) {
            validateValidStateOnly();
            return;
        }
        const validateFn = shouldValidate ? validateWithStateMutation : validateValidStateOnly;
        validateFn();
    }

    const valueProxy = computed({
        get() {
            return value.value;
        },
        set(newValue) {
            setValue(newValue, validateOnValueUpdate);
        },
    });
logaretm commented 1 year ago

@jusheng27

Is this a broken change ?

Mutating the values directly was never intended, it should've been marked as readonly but I didn't want to restrict it that much, it was a side effect of watching the value but prior to us doing that in the early v4 releases it never worked.

Will you consider forward compatibility?

I don't think we should bring the watch behavior back, validations should run explicitly as a result of the value changing intentionally not as a side effect of:

This makes the validation more predictable and avoids some of the issues seen with FieldArrays and virtual fields. 3 distinct bugs were caused by the watcher in field arrays combined with some edge cases.


And if child component uses useVModel with options {deep: true}, it will into an endless loop, can we fix it in vee-validate?

Yes, we can certainly fix it. Can you create an issue with a reproduction?

viajes7 commented 1 year ago

This makes the validation more predictable and avoids some of the issues seen with FieldArrays and virtual fields. 3 distinct bugs were caused by the watcher in field arrays combined with some edge cases.

Ok, I see, I'll try to change the usage in our project( Change the value of useField directly ).

Thanks for your reply.

Yes, we can certainly fix it. Can you create an issue with a reproduction?

@logaretm Sure, https://github.com/logaretm/vee-validate/issues/4264, for your information.