Document $formatters v.1.2.0 -> v.1.3.0: type of value parameter is now a string, not the bound model type

[wardbell]

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.

[Narretz]

True, the behavior changed: 1eda183

[wardbell]

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]

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" notifications@github.com
Gesendet: ‎22.‎09.‎2014 22:22
An: "angular/angular.js" angular.js@noreply.github.com
Cc: "Narretz" mjstaffa@googlemail.com
Betreff: Re: [angular.js] Document $formatters v.1.2.0 -> v.1.3.0: type ofvalue parameter 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]

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 toString version 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]

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]

That's a very good idea!

[wardbell]

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.

[tbosch]

Yes. @Narretz, would you mind looking at my PR?

[dkarten]

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]

@dkarten would you like to open a PR for this?

[dkarten]

@Narretz Sure thing! Do you guys have any branching/PR conventions I should follow?