ngModel changes model in 1.3 without any input from user, 1.2 is ok

[frfancha]

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.

[lgalfaso]

Hi, if you want numbers, why not add type="number" ?

[caitp]

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

[frfancha]

Edit: this is an answer to Igalfaso, sorry forget to mention that. Because all these fields have formatter/parser set from DB settings + user preferences, these arrive together with the data to manage display. E.g. 0.5 is displayed as (and parsed from) 50,00% for French user with db saying this is a percentage field. This was working very well in 1.2 , the goal is to replace all our data entry applications.

[caitp]

Actually, I guess it's slightly dfferent, though it's been discussed on those other issues.

[caitp]

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.

[frfancha]

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.

[Narretz]

I have tried to explain in https://github.com/angular/angular.js/pull/9913#issuecomment-61907841 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.

[frfancha]

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. In the view you have a string, and in the model "something". When you want to apply "string" validation, you must use the "view", the string version, and not the object. How do want to apply a "maxLength" on a date object ? So using validation must not transform view to model.

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. I can't find a simple case to demonstrate it, I'm only able to "smell" that it happens at the same place as the maxLength does the transformation, and that if the maxLength problem is fixed most probably the more complex case will be fixed also.

[Narretz]

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.

[frfancha]

You say: "the length validators always validate the $viewValue" I totally agree with you that this is the only way to use. But then... why is the model recomputed in the process?

Our "complex" problem occurs also with numbers (the date case was only mentionned to illustrate why the viewValue must be used to validate).

[caitp]

yeah we need to get a view value, but we should't be applying the parsed view value to the model, you know? :z

[Narretz]

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.

[petebacondarwin]

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.

[Narretz]

@petebacondarwin Perfectly okay. I actually have a fix pending, we can talk about it this week.

[AGiorgetti]

I think I just reported another duplicate of this thing (i saw it just now, sorry), I think the problems is this:

• validators internally use: $validate() function wich does $$parseAndValidate(), this function first go through the whole validation chain and if everything is ok it changes the $modelValue... which is the root of these side effects in my opinion.

The $validate() function should not change the model, if should just enable the feedback to the user.

[Narretz]

@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.

[Narretz]

I was actually wrong, as far as I've seen the docs make no mention of $validate caling the parsers, so in theory nobody should depend on it.

[frfancha]

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. One could say: yes but these actions must be protected by testing the "valid" flag. Hum, why not but then what is the point of setting null in the model at the first place? Take an example where valid values are any text 0, 2 or 4 characters long. The user (true user now, not code) fills in the field: type A, then B => valid model is "AB", but now he continues type C, then D. Now the model is ABCD. Ok. But when the user has typed "C" (no D yet), view is ABC, invalid, why should you put null (or empty string, or whatever) in the model? Let it "AB" while the user finish to type and don't wake watchers for nothing.

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. This way, in a date field (e.g.), if the user types 4/4 (he must still type /14 to get April, 4th 2014) and leave the field, the field displays ERROR. When the user put the focus again in the field the text 4/4 comes back (with the cursor at the end) and the user can continue to enter the date, just typing /14 instead of loosing all the entry text just because, e.g., he hits the TAB key by accident. That would be useful, instead of loosing the entry text because you leave the field on a incomplete (or invalid) entry.

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, ... All kind of real life rules that you can only verify once the full (or at least a part) of the form is filled in, unless you force the users to follow very specific path (fill in this, then this, then this, which hey doesn't like).

Fred

[petebacondarwin]

@frfancha:

The view must update the model only when it is valid

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 null when the view was invalid was one attempt at dealing with that situation.

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 null, allowing the model to contain the previous value when the view is invalid also seems fairly arbitrary. If we are going to have to check the validity of the view before using the model then why do we need to enforce a model can only be valid constraint?

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 null or "" - whatever is appropriate for that parser). Before using the model in our business logic we can check its validity - but at least the relationship is always consistent. Moreover, this makes asynchronous validation more intuitive - the validation state can be "valid", "invalid" or "pending". It also allows us to use invalid data - say for instance a draft that is not "valid" but should be stored for future use.

What do you think?

[frfancha]

@petebacondarwin Thanks for your interest in my comment. It is normal that the model and the view are not "always" in sync: when you introduce a date, you are busy typing, let's say 4/4/2014, when you have already typed 4/4, the model can not be "in sync", you must first finish your entry. And when you update the model only on blur (which is possible with the new ngOptions => excellent thanks), of course the model is only "in sync" after blur. When the view is not valid, setting null in the model (and null can one of the valid model!) doesn't help, it just trigger all watchers for... nothing: either the watchers test the valid flags and won't use the null value, or they don't test the flags and will believe that the user has really introduce null, which is incorrect. In both cases it is much better to let the model untouched.

See also the comments from Dan Walhin in issue #10035, he thinks exactly the same.

Thanks for reading.

[wardbell]

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.

[petebacondarwin]

@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?

[frfancha]

@petebacondarwin - There are 2 kinds of "errors": A-the view can't even be parsed: user entered "abc" and parser wants to convert to number B-view can be parsed (user entered "123") but validation rules are violated (e.g. must be less than 100) In A the model can't be set (of course) and must be left untouched, and corresponding invalid indicators must be on In B the model can and must be set, and corresponding invalid indicators must be on

[Narretz]

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.

[frfancha]

The logical way is: View change => it is parsed and model is updated => model changes => validators run on the model Validators can't run on the view (let's say rule is "less than 200": the view "123" isn't < 200, but the model 123 is < 200)

[Narretz]

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?

[frfancha]

Yes you're right maxLength on the model doesn't make sense when the view is "transformed" to get the model. But: The other validators must use the model not view (date between, number between, ...) and mainly: when the view is "transformed" to get the model, such validators doesn't make sense at all: maxLength = 5 Then I can enter the string "12345" to get the number 12345 But I can't enter "+12,345" to get the same number?? And if the model is nicely formatted to 12,345.00 then it is incorrect while the user has entered only 12345 (5 characters) to get it?? Conclusion => using "view oriented" validators such as maxLength when the view is "transformed" to get the model will never be really "correct". So you can of course let the possibility for validators to use the model and the view when necessary, but you will face this type of contradiction for these kinds of cases.

[wardbell]

@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 allowInvalid binding flag).

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

I wonder if, instead, the difference between the model and the view should only be driven by the parsers/formatters, irrelevant of its validity.

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.

[wardbell]

@Narretz - you wrote

I don't see a possibility to attach invalid indicators to the model directly, and I don't favor mixing our validation and binding behavior even more in this way.

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.

[Narretz]

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. I think the most important point here is that the ngModelController / input needs to expose a way to more granularly control whether the model should be updated when there's a change in value or validity. More options are one way, which I don't really like. Another way is to expose a customizable function similar to $render, which receives the necessary arguments (previous and current values) and allows the user to decide if the model should be updated or not. If you have ideas, please share!

Since the issue is resolved, I'm gonna close it. But feel free to continue the discussion here or in https://github.com/angular/angular.js/issues/10035