Vee-validate: RC7 breaks .initial validation in combination with v-if on input (+ what is the cause)

Created on 12 Jul 2017 · 5Comments · Source: logaretm/vee-validate

Versions:

VueJs: 2.3.4
Vee-Validate: 2.0.0-rc.7

Description:

In RC6 and earlier, if you had an input field with v-validate.initial and used a v-if condition on it, then when the condition became true, the validation would run as expected and you can display errors on the now-visible input(s).

However, with RC7, this behaviour is broken. When the v-if condition becomes true, the validation does not seem to run (visibly, that is) and no errors are displayed.

Steps To Reproduce:

I have two identical JSFiddles.
RC6 version: https://jsfiddle.net/qhxn2tpm/4/
RC7 version: https://jsfiddle.net/zw3h7ac8/2/
In both versions, after you click on the "Click me!" button, the input element is rendered and because of the .initial modifier you expect the validation to run and display an error becomes the input is empty and the validation rule is required. With RC6 this is indeed the case. With RC7, no error is displayed.

Investigation

I managed to narrow this change down to a specific commit: https://github.com/baianat/vee-validate/commit/ba8fb89dd9355761ef35c70077ccdd0acd8a5d03 "ensure no null scopes"

The code is a bit difficult to follow, but what seems to be happening is the following (prior to the commit!):

bind is called.
Listener is attached. The scope is "__global__".
Validate is called because .initial is set, the validation fails because the required rule is present and the input is empty. The error is inserted in the ErrorBag.
inserted is called. The scope here is null.
The null scope is of course different from the instance scope "__global__"
The field is still resolved, however, because the scope defaults to "__global__" in _resolveField
_moveFieldScope dutifully moves the field from scope "__global__" to null.
update is called.
update finds that the validation is not cached yet (Note: you could probably cache it sooner since the field is already added and the validation has even run, right?)
update computes the scope to be the string "null". I think this is because the null was written back to data-vv-scope.
_createField is called since the scope doesn't exist, but it just copies the "new" field over the existing one.
Since a new field was created, or so it thinks, it tries to remove the errors for the old scope, but again it uses the string "null" so the errors are not removed from the ErrorBag because they were not inserted with that scope.

As you can see, the old behaviour relied on a bunch of bugs since the scope can be "__global__", null and the string "null" in different parts of the code. This should be fixed so it's consistent.
But there is more:

The code in update seems a bit strange as it checks a cache that should probably be already made, and then if the value is not cached (which is always the case on the first update) then it discards validation errors and re-creates the field. The only reason that this did not screw up validation before is actually because it tried to discard the errors of a non-existing scope and because the field was re-created in its existing position.
So there seem to be two bugs, but bug nr1 actually counter-acted the second bug in RC6 and previous :)
The change in RC7 (partially) fixed the first bug, so now bug nr 2 takes effect and the validation breaks in the case where .initial is used as it discards the validation errors when re-creating the field.

I contemplated trying to fix this myself, but I do not understand all the code and these issues touch quite some functions and behaviour, so I'd probably do something silly.

🐛 bug

Source

ThomHurks

👍1

All 5 comments

I though I had fixed the null issues with scoping, primarily I needed the inserted lifecycle to actually check for scopes correctly. for example the form attribute is never set until then. But again I recently found out that inserted is not a guaranteed hook, meaning it doesn't get called in some situations which is critical, so I went back to the bind hook in rc.6.

The code is actually patchy and hacky ATM, that is why PR #616 is going to resolve most of these issues, basically it treats fields as isolated objects, and no longer will be identified by name and scope, but by a unqiue ID. Also it would allow fields to be more flexible and even allow them to change names and scopes dynamically without breaking validation. Also the code is much simpler than the current one and is better tested.

So i guess I need to finish up the PR, and check if it resolves the issue here instead of patching it one more time for rc.8, I'm currently have to following todos before merging: