ngModel changes model in 1.3 without any input from user, 1.2 is ok #9959
Comments
|
Hi, if you want numbers, why not add |
|
This is a duplicate of a few different issues --- so, I know we are translating the view value, but I honestly don't know where we're translating the model value, it's just bizarre. In the mean time, merging this with the original issues |
|
Edit: this is an answer to Igalfaso, sorry forget to mention that. |
|
Actually, I guess it's slightly dfferent, though it's been discussed on those other issues. |
|
I'm with @frfancha we shouldn't be changing the model value at all because of these transforms. We do need the transforms for the maxlength and other validators to work, but it just doesn't make sense to transform the model value too there. |
|
caitp, thanks for the quick answers. It is really comforting. It was a very big decision for us to fully switch to angular 10 months ago, and today while testing 1.3 nothing was working anymore in our applications we were quite disappointed. |
|
I have tried to explain in #9913 (comment) why the modelValue is transformed to string on init - feedback welcome. Basically, it's a combination of the string formatter, and the observers inside the validators firing immediately after init, either because ng-repeat messes with init order, or because the observer fires after the ngModel init. |
|
Hi, as you explain with the date example, parser/formatter can go from a date object in model to the string representing this date in view and vice versa. And there is something worse but I can't find a simple way to reproduce it: even without the maxLength attribute, in some of our complex screens angular still can't wait the parser/formatter and transforms number to string. |
|
the length validators always validate the $viewValue. This is the only way it makes sense. Complex max / min validations exists for date and number, or must be provided by the developer. So maxlength does not transform the value. It's all textbased input types that do it. Do you have an example where the problem happens with a date input type? I'd be very interested to see that. |
|
You say: "the length validators always validate the $viewValue" Our "complex" problem occurs also with numbers (the date case was only mentionned to illustrate why the viewValue must be used to validate). |
|
yeah we need to get a view value, but we should't be applying the parsed view value to the model, you know? :z |
|
I am currently testing if we actually need the toString formatter. It seems to me that we don't, if length validators do the conversion themselves. |
|
I'm adding this to 1.3.3 as it seems that we have a general issue with the model being updated to a string in a non-intuitive way that we should get to grips with this iteration. @Narretz - you seem to be doing some digging into these. Can I assign these issues to you? I would be happy to find some time to pair on it with you during the week. |
|
@petebacondarwin Perfectly okay. I actually have a fix pending, we can talk about it this week. |
|
I think I just reported another duplicate of this thing (i saw it just now, sorry), I think the problems is this:
The $validate() function should not change the model, if should just enable the feedback to the user. |
|
@AGiorgetti Thanks for your report. The problem is not that $validate changes the model. It's necessary that $validate can do that, because if the validation requirements change, the model must be set to undefined to be in line with what happens when a user inputs invalid values (this was the case in 1.2.x too). So the specific design problem is that $validate parses the model again - this should not happen. This is unfortunately a remant of the earlier design from back when the $parsers were doing validation, and it's possible that someone relies on it, so we cannot change the behavior now. |
|
I was actually wrong, as far as I've seen the docs make no mention of |
|
I think that a angularJS is really a great framework (I have decided to rewrite all our data entry application using AngualrJS) and therefore I would like to give my opinion on how model and view should be linked. Of course this is only a personal opinion, but it is based on 20 years of data entry application development in Smalltalk in insurance and bank companies, so I (naively?) hope that it can help AngularJS to progress. The view must update the model only when it is valid (at events defined by dev: on each key stroke? delayed? on blur? as is now possible with modelOptions). Of course the various "valid" and "error" flags must be set when the view is invalid, but don't touch the model: setting null or undefined (or whatever) in the model when the view is not "valid" shouldn't be done. "Users" (by users I mean code using $scope.data.theModel) of the model won't rely on "null" to know that the model is invalid: null can be a perfect valid option, so the real test is using the valid/error flags, not testing null in the model. And "users" (again: code using $scope.data.theModel) of the model will react when the model changes, so changing the model to null (which, again, can be in the list of valid values) will trigger all kind of useless actions. If you would like to add features to the validation process of fields, something else would be very interesting: display in the input field the error message (or just ERROR) on blur when the field is invalid, and on focus reput the previous incorrect string. One final word, you should not put too much energy in complex validation rules for individual fields: in real life data entry application, validation rules are almost always done between fields: if this field is this or that, then this field must be filled in with a value greater than this field, etc, etc, ... Fred |
I am interested in this statement and why this is important to you. One of the concerns in this model-view setup is dealing with situations where the model and the view become out of sync. The idea of setting model to Your suggestion is that the model should just be left alone if the view is invalid. I feel that for many of the reasons that you suggest for I wonder if, instead, the difference between the model and the view should only be driven by the parsers/formatters, irrelevant of its validity. Of course if a parser is unable to make the transformation then sure it should return something sensible (perhaps What do you think? |
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
|
@petebacondarwin See also the comments from Dan Walhin in issue #10035, he thinks exactly the same. Thanks for reading. |
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
|
Validation is for inspection of the model value ... period. It should never change the model value. Never. It is a read-only operation, not a mutator. If I have my car smog checked and it fails that check, I expect them to tell me that it failed. I do not expect them to rip out the tail pipe and catalytic converter. I asked for an inspection, not a fix and certainly not destruction. Parsers are another matter. When engaged as part of the flow of data from the view to the data model I expect them to morph the user's input. It is OK if a validator short-circuits that flow; that's its role in the process. But it should not change the value in any way. |
|
@wardbell - so are you in favour of the model not being modified at all when the viewValue change would cause it to become invalid or to allow the invalid value to be assigned to the model while indicating that this is now invalid? |
|
@petebacondarwin - There are 2 kinds of "errors": |
|
I am not sure if you are suggesting this, but I don't see a possiblity to attach invalid indicators to the model directly, and I don't favor mixing our validation and binding behavior even more in this way. What we could do is to allow the user to control the update behavior after user input -> parsing -> validation. |
|
The logical way is: |
|
Why can't validators run on the view? There are cases where validators only make sense on the view, such as maxlength / minlength. When the model gets converted an object or an array, how will you validate the length? |
|
Yes you're right maxLength on the model doesn't make sense when the view is "transformed" to get the model. |
|
@petebacondarwin - Sorry for taking so long to reply. The principle that has never failed me in 15 years is "validation never touches the model". Never. As in never. I appear to be in good company on that point with others from the community. I am indeed in favor of the developer deciding whether invalid data entry should be allowed through to the model. Of course the binding has to know what to do. I don't care what the default is as long as it (1) is a simple rule, (2) consistently applied, (3) clearly documented, and (4) easily changed by the developer (e.g., the Validation is just one stop in the pipeline between user input and model update. The current default seems to imply "if fails validation, don't update model". With the right flag I may instruct the binding to allow invalid values into the model. In practice - and this is my complaint - a failed validation destroys the model value. This is a disaster. It is far too easy for the unexpectedly altered model to leak out of the edit view and perhaps be saved to the database. In a more advanced system, the binding pipeline allows the developer to reason about the {view value, validation result, all of the model} and allow or disallow model update accordingly ... which is what parsers and formatters ought to be able to do. I believe that is what you meant when you wrote
You ask how I feel about view and model being out-of-sync. "Uncomfortable and resigned to it." I want the ability to do something about it but I don't want validation making that decision and I don't want Ng to impose a decision upon me. I certainly do not want it to destroy existing model values, especially not in such a manner that I cannot recover them. |
|
@Narretz - you wrote
I agree that Ng cannot do that because it is not Ng's prerogative to adorn the model. That puts Ng in a difficult spot because Validation is fundamentally model logic, not UI logic and therefore belongs in the model, not in the UI. Of course that logic is manifested in the UI and requires access to proposed values coming from the UI. But the logic itself ... and the collection of validation results ... is part of the model, not the UI. I am confident this perspective is widely accepted ... citations available upon request :-) BreezeJS locates validation logic in the model itself. Breeze is a library for managing entities and entities have "behavior". You buy into that fact when you buy into Breeze. Ng is agnostic about the model - properly so IMO - and therefore cannot and should not make assumptions about the model nor adorn it with its own material. This principle means Ng's can only offer a purely UI validation scheme which works fairly well much of the time but is necessarily incomplete. For example, it cannot remember the validity of the model once it is no longer on screen, cannot help other application components to validate the model, and cannot prevent invalid models from being saved. I hasten to add that Ng validation is a perfect fit where the view is bound to a simple object model. It's great for validating user input bound to _ephemeral objects_ whose data are subsequently transformed into entities or passed through to commands as message bodies. It just isn't the best choice for binding directly to entities or any object that lives outside the view/controller pair. Unfortunately, this distinction is not widely understood and people are applying Ng UI validation to bindings that update long-lived, shared objects. We can't stop that. But we can minimize the damage by being careful to avoid hidden side-effects such as erasing bound properties when the user input is invalid. |
|
Thanks @wardbell and @frfancha for your comments. We plan to make a bigger update to forms / ngModel in the future (no fundamental changes, but possibly breaking and not in 1.3.x), so we really value your input. Since the issue is resolved, I'm gonna close it. But feel free to continue the discussion here or in #10035 |
Simple plunker to illustrate:
http://plnkr.co/edit/pbL86XRPluAjUkha10Dg
Scenario is:
-set model to 3 (number 3, not string "3")
-have a maxLength on field
-add a parser to parse string from input to number
This scenario happens in no particular order (at least for me) as everything comes by binding to scope.
With angular 1.2.26 every is fine, 3 stays 3 while nobody touches the input field.
With angular 1.3.1, angular changes the model to "3" instead of 3 before the parser string to object is set, without anybody "touching" the input field.
The plunker only illustrates this behavior, the corruption of the model.
In our real life problem, when we set the parser we also set a formatter model to view, and this formatter fails with angular 1.3 because it finds "3" instead of the original 3.
With angular 1.2 everything works fine.
The text was updated successfully, but these errors were encountered: