Repository navigation
Document $formatters v.1.2.0 -> v.1.3.0: type of value parameter is now a string, not the bound model type #9218
Description
v.1.3.0-rc.2 broke my zFloat directive which worked fine in v.1.2.0.
Traced to a change in the value passed to a $formatter. It used to be the data-bound value of the model; in 1.3.0, the value is a string.
Here's the plunker: http://plnkr.co/edit/E2HgF7ASNWIdwsvzs5G0?p=preview
Here's the pertinent code fragment from the directive:
return {
restrict: 'A',
require: 'ngModel',
link: function(scope, elm, attr, ngModelCtrl) {
if (attr.type === 'radio' || attr.type === 'checkbox') return;
ngModelCtrl.$formatters.push(equivalenceFormatter);
function equivalenceFormatter(value) { // 1.2.0: integer; 1.3.0: string
var viewValue = ngModelCtrl.$viewValue // could have used 'elm.val()'
return (value === +viewValue) ? viewValue : value;
}
}
Where the model value was integer 123 in v.1.2.0, it is now the string "123".
Of course the conditional value === +viewValue fails in v.1.3.0 because "123" !== 123
Is that intentional? Please call that out in release notes.
Meanwhile ... fortunately ... a workaround is available for me. I'll just change the conditional to +value === +viewValue which will be compatible with both versions. You may see that in the Plunker by the time you look.
Too bad for everyone else who didn't see this coming.
Activity
Narretz commentedon Sep 22, 2014
True, the behavior changed: 1eda183
Fix zFloat to work with Ng v.1.3.0-rc
fix(ngModel): always format the viewValue as a string for text, url a…
wardbell commentedon Sep 22, 2014
I just want to add that this change makes no sense to me. If I'm writing a formatter of a scope-bound value that can ONLY be because I want to format it. You defeat the purpose by turning it into a string before I get my chance to format it. If you want to turn it into a string "at the last mile", fine.
What was the reason for this?
Narretz commentedon Sep 22, 2014
You can still add a formatter that runs before the built in. You can even remove the built in formatter. I'm stumped myself why this was done but I remember discussing it with matsko. I think it had to something with validation.
----- Ursprüngliche Nachricht -----
Von: "Ward Bell" [email protected]
Gesendet: 22.09.2014 22:22
An: "angular/angular.js" [email protected]
Cc: "Narretz" [email protected]
Betreff: Re: [angular.js] Document $formatters v.1.2.0 -> v.1.3.0: type of
valueparameter is now a string, not the bound model type (#9218)I just want to add that this change makes no sense to me. If I'm writing a formatter of a scope-bound value that can ONLY be because I want to format it. You defeat the purpose by turning it into a string before I get my chance to format it.
What was the reason for this?
I'll add this comment to the commit log as well
—
Reply to this email directly or view it on GitHub.=
IgorMinar commentedon Sep 30, 2014
we can't improve the commit message for that commit but we can post a comment there and update the changelog with the breaking change.
Missing info:
The breaking change is that the viewValue passed into formatters will be a
toStringversion of the formatted modelValue. So if any custom formatters execute after the default formatter, they'll see the string version of the value. If any formatter needs access to the value before it was stringified, the formatter should be registered via$formatters.unshift(customFormatter).The reasoning for this change was to ensure that we are always dealing with the same type of the view value in parsers and validators. Previously depending on how the value was set it could have been string or object or whatever type was set in the model. After this change the value is always a string.
@matsko please check to make sure that I haven't missed anything
IgorMinar commentedon Sep 30, 2014
as discussed with @tbosch we are going to change the timing when our core formatters and parsers are generated so that any custom formatters and parsers run before them.
Narretz commentedon Sep 30, 2014
That's a very good idea!
wardbell commentedon Sep 30, 2014
If I understand correctly, we'll get the original behavior back and no need for unshift. Hooray if so, else I don't understand what Igor just said.
fix(input): register builtin parsers/formatters before anyone else
tbosch commentedon Oct 1, 2014
Yes. @Narretz, would you mind looking at my PR?
fix(input): register builtin parsers/formatters before anyone else
fix(input): register builtin parsers/formatters before anyone else
fix initial selection of select2 box (clear core formatters; see: ang…
dkarten commentedon Dec 2, 2016
Would like to point out this is not in the 1.2 -> 1.3 migration guide
Took me hours to end up at this comment thread and fix it.
Narretz commentedon Dec 2, 2016
@dkarten would you like to open a PR for this?
dkarten commentedon Dec 2, 2016
@Narretz Sure thing! Do you guys have any branching/PR conventions I should follow?