wip: fix(ngModel): don't update when neither validity nor modelValue changed #9890

Narretz · 2014-11-03T20:11:26Z

This is in response to #9063.

don't update when neither validity nor modelValue changed

is what it actually does in the code, but is written to do that:

don't fire ngChange when an input with ngRequired is initially invalid, but its model is defined.

A better fix would require to find out why required fires the observer only once, but ng-requiredfires it twice, see #9063 (comment)

caitp · 2014-11-05T14:48:49Z

test/ng/directive/inputSpec.js

@@ -3914,6 +3922,18 @@ describe('input', function() {
          expect(scope.value).toBeUndefined();
          expect(scope.form.numberInput.$error.required).toBeFalsy();
        });
+
+        it('should not set the model to undefined if the model is initially null', function() {


I don't think this is right --- we want to set the model value to undefined (because for whatever reason that's what we do in angular), we just don't want to call the view change listeners. Can you adjust this so that we don't change the behaviour (crazy as it is) of the "invalid -> undefined" thing, and just avoid calling the change listeners?

Since sometime in 1.3, on ngModel init, invalid values are kept as-is - for viewValue and modelValue. The problem is now that the observers that are attached to some validators fire immediately after ngModel init, and sometimes change the value, even though there was neither user input nor programatic model change. The conditions in the fix are afaik only true immediately after init.

yeah --- so what I think we want to do is ---- yes update the view value, but no don't update the model value.

but I guess we can't do that since we need to parse the view value when validating. Why are we doing that? :(

what if we had like an optional "do-not-parse" mode

We could do that for our own validators; but if anyone else uses observers in validators, they will run into the same problem. I don't think we want to introduce a new api for that. If we do it only for $validate, it's going to be a breaking change for those who use it where they need to parse the viewValue.
I agree we should have separted parsing and validating more when we had the chance to.

$validate() doesn't sound like something that you do when you want to parse the view value --- it really doesn't make sense that we're even doing this to begin with.

It still would be breaking if we change it now ... (though I agree)

Previously, $validate would execute the parsers to obtain a modelValue for validation. This was necessary, because a validator that is called outside of model / view update (e.g. from an observer) otherwise might only an undefined modelValue, because a previous view update has found a validation $error and set the model to undefined (as is tradition in angular) This is problematic as validators that are run immediately after the ngModelController initializes would parse the modelValue and replace the model, even though there had been no user input. The solution is to go back to an older design: the ngModelController will now internally record the $$rawModelValue. This means a model or view update will store the set / parsed modelValue regardless of validity, that is, it will never set it to undefined because of validation errors. When $validate is called, the $$rawModelValue will passed to the validators. If the validity has changed, the usual behavior is kept: if it became invalid, set the model to undefined, if valid, restore the last available modelValue - the $$rawModelValue. Additionally, $validate will only update the model when the validity changed. This is to prevent setting initially invalid models other than undefined to undefined (see angular#9063) Fixes: angular#9063 Fixes: angular#9959 Fixes: angular#9996 Fixes: angular#10025 Closes: angular#9890 Closes: angular#9913 Closes: angular#9997

Narretz · 2014-11-13T20:50:24Z

This is superseded by #10048 (even though it now contains the same changes ...)

Previously, $validate would execute the parsers to obtain a modelValue for validation. This was necessary, because a validator that is called outside of model / view update (e.g. from an observer) otherwise might only an undefined modelValue, because a previous view update has found a validation $error and set the model to undefined (as is tradition in angular) This is problematic as validators that are run immediately after the ngModelController initializes would parse the modelValue and replace the model, even though there had been no user input. The solution is to go back to an older design: the ngModelController will now internally record the $$rawModelValue. This means a model or view update will store the set / parsed modelValue regardless of validity, that is, it will never set it to undefined because of validation errors. When $validate is called, the $$rawModelValue will passed to the validators. If the validity has changed, the usual behavior is kept: if it became invalid, set the model to undefined, if valid, restore the last available modelValue - the $$rawModelValue. Additionally, $validate will only update the model when the validity changed. This is to prevent setting initially invalid models other than undefined to undefined (see angular#9063) Fixes: angular#9063 Fixes: angular#9959 Fixes: angular#9996 Fixes: angular#10025 Closes: angular#9890 Closes: angular#9913 Closes: angular#9997

Previously, $validate would execute the parsers to obtain a modelValue for validation. This was necessary, because a validator that is called outside of model / view update (e.g. from an observer) otherwise might only an undefined modelValue, because a previous view update has found a validation $error and set the model to undefined (as is tradition in angular) This is problematic as validators that are run immediately after the ngModelController initializes would parse the modelValue and replace the model, even though there had been no user input. The solution is to go back to an older design: the ngModelController will now internally record the $$rawModelValue. This means a model or view update will store the set / parsed modelValue regardless of validity, that is, it will never set it to undefined because of validation errors. When $validate is called, the $$rawModelValue will passed to the validators. If the validity has changed, the usual behavior is kept: if it became invalid, set the model to undefined, if valid, restore the last available modelValue - the $$rawModelValue. Additionally, $validate will only update the model when the validity changed. This is to prevent setting initially invalid models other than undefined to undefined (see angular#9063) Fixes: angular#9063 Fixes: angular#9959 Fixes: angular#9996 Fixes: angular#10025 Closes: angular#9890 Closes: angular#9913 Closes: angular#9997 Closes: angular#10048

mary-poppins added cla: yes and removed cla: no labels Nov 3, 2014

Narretz mentioned this pull request Nov 5, 2014

fix(input): don't invoke viewChangeListeners before real user input #9913

Closed

caitp reviewed Nov 5, 2014
View reviewed changes

jeffbcross added this to the 1.3.x milestone Nov 11, 2014

jeffbcross added the needs: work label Nov 11, 2014

Narretz force-pushed the fix-ngModel-ngChange branch 2 times, most recently from 8013145 to 50f7e66 Compare November 13, 2014 20:06

Narretz force-pushed the fix-ngModel-ngChange branch from 50f7e66 to 2182f8e Compare November 13, 2014 20:32

Narretz mentioned this pull request Nov 13, 2014

fix(ngModel): don't run parsers when executing $validate #10048

Merged

Narretz closed this Nov 13, 2014

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

wip: fix(ngModel): don't update when neither validity nor modelValue changed #9890

wip: fix(ngModel): don't update when neither validity nor modelValue changed #9890

Narretz commented Nov 3, 2014

caitp Nov 5, 2014

Narretz Nov 7, 2014

caitp Nov 7, 2014

caitp Nov 7, 2014

Narretz Nov 7, 2014

caitp Nov 7, 2014

Narretz Nov 7, 2014

Narretz commented Nov 13, 2014

wip: fix(ngModel): don't update when neither validity nor modelValue changed #9890

wip: fix(ngModel): don't update when neither validity nor modelValue changed #9890

Conversation

Narretz commented Nov 3, 2014

caitp Nov 5, 2014

Choose a reason for hiding this comment

Narretz Nov 7, 2014

Choose a reason for hiding this comment

caitp Nov 7, 2014

Choose a reason for hiding this comment

caitp Nov 7, 2014

Choose a reason for hiding this comment

Narretz Nov 7, 2014

Choose a reason for hiding this comment

caitp Nov 7, 2014

Choose a reason for hiding this comment

Narretz Nov 7, 2014

Choose a reason for hiding this comment

Narretz commented Nov 13, 2014