fix(input): don't invoke viewChangeListeners before real user input #9913
Conversation
| // String input types can transform a value into a string without touching it. | ||
| if (oldVal != null && !isNaN(oldVal) && | ||
| ctrl.$modelValue === oldVal.toString()) { | ||
| return; |
There was a problem hiding this comment.
so we still change the scope value, the only difference is we don't invoke change listeners. @petebacondarwin you get to review this one and decide if it's worth fixing.
|
#9918 is totally unrelated, and we need to see a repro before dealing with that. The other issue looks related, but not easy to fix in one CL, i'll open a second one for that. Updating the model value during validation was always a terrible idea and I have no idea why we feel the need to do that. |
|
This fix doesn't seem right to me - but I can't put my finger on it. It seems like we shouldn't be permanently converting the value to/from a string only to validate. |
|
we shouldn't, but that's what we do --- we also can't throw it away because we don't know if we did it or if a user did it. Best we can do is just not notify change listeners (if the model is basically the same) |
|
OK, so I think what is happening is... Without a validator With a validator So I am thinking, perhaps the workaround is to put in place a parser that converts the input to a number? |
|
Yep, so if the input is |
|
That doesn't really make sense --- we shouldn't require people to set up formatters/parsers, and there's no way for us to know if that's what they would want in the first place, so we can't do it in core. I'm pretty sure the thing to do is just not fire view change listeners if the model is essentially unchanged |
|
If the user puts a number into a model that is attached to a text input box then that input box will convert the value to a string, which effectively changes the model and so triggers ng-change. I think we just need to give guidance that they should ensure that the type of the model data is not modified by the input if they are bothered about change events by doing one of the following:
If anything the bug is that we don't do a round trip triggering the ng-change if there are no validators. |
|
The trouble with this fix is that we are assuming that the only conversion is to and from a string. If there are formatters/parsers in the NgModelController that do other kinds of conversion (such as two and from Date objects) then you would still get the same problem. |
I understand what we're doing. But this is crap, we should not be doing this. We can't "not translate the model", we need the model to be a string for other reasons --- but we should not be shooting off the change listeners when there's no good reason to. Lets stop coming up with non-solutions, because they're just going to cause the framework to continue to be absolute crap |
|
We are in agreement that the forms story in Angular has become complete nonsense, but there are things we can't change, and there are things we can --- this is one of the things we can change without clobbering the rest of the system |
Can you explain what you mean by "we need the model to be a string for other reasons"? Am I missing something here? As I see it the problem is that the |
We format string input types to a string value so that we can be sure all of the builtin validators work correctly for them. Not formatting breaks them there --- and requires us to use really awful hacks that sometimes work and sometimes don't. It's not clear why the modelValue is being set to this, though. |
|
We should find the exact reasons for having the string formatter in text input types. The commit doesn't tell us why: 1eda183 As far as I remember, here's why: In #7968, we decided to use the $viewValue for the length validation. However, if the model is set programatically to a number, or $setViewValue is called with a number, validating the $viewValue for length doesn't work. Regarding the $viewChangeListener - from its name it's called when the view / control changes. But this is not correct - it only changes when the viewValue and the modelValue change. And I think we should keep it that way: fire it only when the change in the modelValue is driven from the input (user). If the change is model -> view -> validator ???, don't fire it. |
We know why --- we need them for a variety of the builtin validator attributes for string input types. What doesn't make sense, is why the model value is getting updated this way. View value is fine, but the model value should not be getting stringified |
This is too vague. I linked to the issue where the length validation is discussed; no other validation operations are mentioned. I really don't want to make decisions based on these vague statements. Here's how the modelValue becomes string:
Now why this only happens if the input is in an ng-repeat? It looks like the ng-repeat changes the timing or the initialization order of the directives. And because $validate before the model is initialized does nothing, the whole parsing shenanigans never happens. The order of function calls with ng-repeat: The order without ng-repeat see: http://plnkr.co/edit/EIgrUCxN9Sglr5qkfD7G?p=preview |
|
It's really not very fun, the forms story has become incredibly stupid and frustrating, and we can't really fix it :/ |
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
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
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
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
|
Closed by e3764e3 |
Closes #9867