Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

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

Closed

Description

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

Activity

  1. lgalfaso commented on Nov 7, 2014

    @lgalfaso
    Contributor

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

  2. caitp commented on Nov 7, 2014

    @caitp
    Contributor

    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

  3. frfancha commented on Nov 7, 2014

    @frfancha
    Author

    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.

  4. caitp commented on Nov 7, 2014

    @caitp
    Contributor

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

  5. caitp commented on Nov 7, 2014

    @caitp
    Contributor

    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.

  6. added this to the milestone on Nov 7, 2014
  7. frfancha commented on Nov 7, 2014

    @frfancha
    Author

    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.

  8. Narretz commented on Nov 7, 2014

    @Narretz
    Contributor

    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.

  9. frfancha commented on Nov 7, 2014

    @frfancha
    Author

    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.

  10. Narretz commented on Nov 7, 2014

    @Narretz
    Contributor

    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.

  11. frfancha commented on Nov 7, 2014

    @frfancha
    Author

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

  12. 26 remaining items

  13. modified the milestones: 1.3.4, 1.3.3 on Nov 17, 2014
  14. frfancha commented on Nov 18, 2014

    @frfancha
    Author

    @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

  15. Narretz commented on Nov 18, 2014

    @Narretz
    Contributor

    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.

  16. frfancha commented on Nov 18, 2014

    @frfancha
    Author

    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)

  17. Narretz commented on Nov 18, 2014

    @Narretz
    Contributor

    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?

  18. frfancha commented on Nov 18, 2014

    @frfancha
    Author

    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.

  19. wardbell commented on Nov 22, 2014

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

  20. wardbell commented on Nov 22, 2014

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

  21. modified the milestones: 1.3.4, 1.3.5 on Nov 25, 2014
  22. Narretz commented on Nov 26, 2014

    @Narretz
    Contributor

    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 #10035

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions