This repository was archived by the owner on Apr 12, 2024. It is now read-only.
Repository navigation
This repository was archived by the owner on Apr 12, 2024. It is now read-only.
[1.3-beta16] ngModel $parsers regression vs. v1.2 #7965
Description
I'm having issues with the AngularStrap datepicker in the latest v1.3 beta.
Basically, I've found out that the ngModel $parsers won't be called if $setViewValue is called multiple times with the same value/reference. It was the case in the v1.2 branch.
This is be problematic when we are using objects that change values but keep the same reference (such as Dates).
Check the following plunkers (console.warn):
- v1.2: http://plnkr.co/edit/PuleeGHkLHj3RvYSobs5?p=preview
- v1.3: http://plnkr.co/edit/yRL4JR0aVY3kDGgwLXX2?p=preview
Might be related to this commit
Activity
Narretz commentedon Jun 24, 2014
Yes, it looks like facd904#diff-c244afd8def7f268b16ee91a0341c4b2R1759 introduced the change. It works with 1.3.0-beta.9, and there were no other changes related to forms.
mgcrea commentedon Jun 24, 2014
1f6a5a1 might provide a workaround with
revalidate. Not sure this is good enough though as this is a breaking change.shahata commentedon Jun 24, 2014
yes, of course. since we are testing
if (ctrl.$$lastCommittedViewValue === value) { return; }we will never commit a new value in case$$lastCommittedViewValueis an object that keeps being reused by later updates.I can look into this, but I'm not sure that this is the only point in
ngModelControllerthat assumes that the viewValue is not a reference to some object that can update independently after$setViewValueis called. I might be wrong, but it means for example, that when this object gets updated independently, all the scope variables that are bound to it get updated as well. Which means that updates cannot be delayed byngModelOptions. WDYT? Is this really a legitimate way to use this API?shahata commentedon Jun 24, 2014
The example provided by @mgcrea for example can be fixed easily by doing
controller.$setViewValue(angular.copy(date))instead ofcontroller.$setViewValue(date)http://plnkr.co/edit/u44HOYpWrrQQyEFDszV5?p=preview
mgcrea commentedon Jun 24, 2014
@shahata, yup, did patch AngularStrap with a proper workaround. Thanks!
I do agree that the API usage was a bit far-fetched and I'm OK with the new behavior.
shahata commentedon Jun 24, 2014
@Narretz WDYT? I believe this is an acceptable breaking change and can be closed...
Narretz commentedon Jul 1, 2014
It's okay for me, but @matsko or @caitp should have a look at this. I'm not part of the core team. :)
caitp commentedon Jul 1, 2014
It's nonsense to behave differently for objects and primitive values, but it's pretty terrible to make copies of objects to compare against, too, and we can't rely on better mechanisms in 1.x, and even those other mechanisms would be a bit heavy for this. It basically sucks no matter what.
fix(input): trigger modelValue update on viewValue property changes
fix(input): trigger modelValue update on viewValue property changes
petebacondarwin commentedon Jul 2, 2014
I have a proposed solution for this at #8047. Take a look.
petebacondarwin commentedon Jul 3, 2014
Having discussed this with @shahata, the feeling is that making a copy of the
$viewValuewithinngModelis too fraught with difficulties. There if you don't make the copy early enough then changes to the object may slip through between recent calls to$setViewValuewhen you don't want them to. Even if you do make a copy then there are potentially problems with the copy not being an exact replica in the case of the object having dynamic accessor properties, or with property changes on the prototype not being noticed. Finally we would then need to manage the same issue from model to view as well.So the result is that directive developers (i.e. those writing custom input controls such as the date picker in this issue) are responsible for making a copy of their object before passing it to
$setViewValue, which is the workaround suggested by @shahata in this issue and accepted as reasonable by @mgcrea.Of course, if the
$viewValueis not an object, but a string or number then this is a moot point and there is nothing to worry about. Luckily this is case for the vast majority of directives.It would be good if someone could put together a docs PR that explained this - perhaps in the guide section?