Repository navigation
Different behavior for required and ng-required for initial values #9063
Description
I'm noticing different behavior when using required vs ng-required when combined with ng-change. When the initial model value is any empty value besides undefined (i.e. null or ""), ng-changed is fired when using ng-required="true", but not when using the basic required.
It also seems that this behavior has changed from 1.3.0 rc0 and rc1. In rc0, the change handler is never called.
The behavior for ng-required in rc1 makes sense to me; validation is run for the initial value, null and "" do not pass validation so the model value is set to undefined, and sense undefined does not equal the initial value ng-change is evaluated. But shouldn't the two variants behave the same?
Here is a plunker demonstrating the differences: http://plnkr.co/edit/e1J1VnUSpR8xVhdKIXVY?p=preview
I'm not sure, but this might be related to #8949 and/or #9001
Activity
Narretz commentedon Sep 15, 2014
This is probably because ngRequired observes the required attr and fires the validation a second time.
Actually, I don't think the behavior of ngRequired in rc.1 is correct. The validation calls the parsers, which detect an undefined viewValue and therefore changes the model value to undefined (without calling any parsers), which triggers ngChange. But as @caitp pointed out in #9044 (comment) angular should not actually parse empty input values.
caitp commentedon Sep 15, 2014
to expand on that, I don't think we should parse any values unless setViewValue was called, which should not happen when calling $validate() in isolation (imo)
Narretz commentedon Sep 15, 2014
I agree. At a minimum, running parsers should be extracted from $validate. But I haven't checked how tight the coupling overall still is.
shahata commentedon Sep 15, 2014
For the record, re-parsing on
$validatewas introduced in tbosch@1ec8bdeThe goal was to get rid of
$$invalidModelValue. If we do not want to re-parse, it means we need to add$parsedModelValueor something similar as discussed in #9016 (comment)Narretz commentedon Sep 28, 2014
This "fails" even with 92f05e5 (don't parse if viewValue is undefined)
This is because there's an unnecessary validation after the model is set to the view for the first time:
When the ngModelController is created, it checks the model via$watch, formats it, sets its viewValue, and calls $ $runValidators. This is okay.
The problem is that all directives / validators that use an observer call $validate very often, too often I'd say.
-> before the ngModelCtrl is initialized: these are ignored because the $modelValue isn't set yet => Okay
-> after the modelWatch is called: these go through, and since the modelValue is invalid, they set the model to undefined. Since this observer comes directly after the first model -> value, it is unnecessary, since the viewValue / modelValue cannot have changed yet.
One solution is to not call $validate inside observers when the attr value hasn't changed. It's actually possible to that $validate is called far more often that it needs to be. The problem here is that this is needed for all observers, so everyone who writes a validator needs to be aware of this boilerplate.
IgorMinar commentedon Sep 30, 2014
@tbosch can we have a quick chat about this some time today?
23 remaining items
Narretz commentedon Oct 21, 2014
What I don't understand (and previously didn't pay any attention to) is that
requiredonly fires the observer once, butngRequiredtwice. I can for the love of it not understand why that is.e: it's not the cause of the issue, but it's interesting nevertheless.
The actual cause lies somewhere here, 9ad7d74, probably something about the removal of $$invalidModelValue
onehourlate commentedon Nov 12, 2014
Just to add my two cents : this change even breaks the most basic angular-only behavior with checkbox input types.
If an input checkbox has an initial model of undefined or null, and it has both ng-required and ng-change, the ng-change callback will fire on page load, which is definitely not supposed to happen.
cf http://plnkr.co/edit/5gqfJvtxwHpvSHoPDBfA?p=preview (the $scope.counter equals to 1 after page load, when it should be 0)
Narretz commentedon Nov 12, 2014
@btford I'm taking this over, this is part of a bigger problem with ngModel
fix(ngModel): don't run parsers when executing $validate
fix(ngModel): don't run parsers when executing $validate
fix(ngModel): don't run parsers when executing $validate
fix(ngModel): don't run parsers when executing $validate
fix(ngModel): don't run parsers when executing $validate
fix(ngModel): don't run parsers when executing $validate
Narretz commentedon Nov 25, 2014
This is fixed in 1.3.4 :)