fix(forms): remove equalsTo validator

[kara]

We are removing the equalsTo validator introduced in 4.0.0.beta.6 because upon review, the concept is inconsistent with and breaks the guarantees of the validation model.

This validator introduces dependencies between sibling controls. This is problematic because our validation model propagates changes up the tree. If you change the value of a form control in the UI, the form control in question is validated, then its parent, and its parent, and so on. Its siblings are not re-validated as their values have not changed. This allows us to skip checking subtrees that haven't updated and keep validation fast.

Because of this, the validator doesn't work as expected. If you add the equalsTo validator onto control "one", when you change the value of control "two" in the UI, the validation status of control "one" will not be re-calculated. Only changes initiated in control "one" itself will actually update its validation status. Even if you add another equalsTo validator on control "two", it will still not be validated on changes to control "one" and both controls' validation statuses will often be incorrect (see plunker here: http://plnkr.co/edit/4gcrKXs47nf6NTbJHJPw?p=preview).

Validating between sibling controls is something that is already possible in the framework with group validators. We generally recommend moving up a level to validate between sibling controls. As validation propagates up, the group will always be updated when the value of any of its descendants changes.

Based on these mismatches, this validator does not belong at the framework level.

[Toxicable]

As mentioned about your note on validating at a group level would it not be fitting to move this validator there then? Or would it still not belong in the framework even without those issues?

[kara]

@Toxicable Given that a group validator only takes a line or two to implement, it seems straightforward enough to leave up to the individual developer. Different projects tend to have very different needs for what's considered "equal", so people will often have to implement their own customization for equality checks anyway. One project might want to check ID only, another might want to do a deep check, etc. Supporting all these cases isn't really possible without asking for a function (which is essentially what we have now).

By the way, wanted to say thanks for contributing the original PR. It sucks that we had to revert. I hope this doesn't discourage you from contributing more to the framework going forward; we appreciate it :)

[Toxicable]

@kara That's a fair point about different implementations of equality, thanks for explaining that. I understand that this is how things work sometimes and don't worry t hasn't discouraged me from contributing, I will help out were I can.

[IgorMinar]

Can you please improve the commit message to explain why this is being removed and why that's ok (because the API was introduced only in a beta and we found it to be incorrect before the launch).

[chuckjaz]

This change needs to be rebased.

[kara]

@IgorMinar This needs approval again after the rebase (folder moved).

[dietergeerts]

equalsTo can be seen in two ways: the way you guys are explaining it and why it got removed, or that a field must be identical to another, like a repeat password field, and then, that validation must only be applied to that second fields, so then there is no problem, so please reconsider this decision!

[kara]

@dietergeerts Even if you only care about validating the second field, the second field's validation would also be wrong. Any edits to the first field would not run the second field's validation.

In other words, if you type:

1. "a" into the first field
2. "a" into the second field
3. then "b" into the first field (so it reads "ab")

They would not be equal, but the second field would still report VALID because it would not be notified of the change in "a".

[dietergeerts]

True, but maybe we need to have the possibility to rerun a validator on field B when field A changes?

[fmalcher]

Can you mark this one as breaking change (or at least extra highlighted) in the change log?
I know this has been introduced in a beta release ;-) However, when just listed under "bug fixes" some might miss that one.
Thanks! 😎🆖 @kara @chuckjaz @IgorMinar

[mlc-mlapis]

@dietergeerts And updateValueAndValidity method is not enough as in other cross validation cases?

[Toxicable]

@dietergeerts The issues described will still occur, but these short comings of the validator can be fix by implementing it at FormGroup level rather than at a FormControl level.

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}