feat(ngModel) Allow running the formatters without a change to the modelValue

[realityking]

This is needed when a formatter can be configured in some way and needs to be rerun after the change. Issue propped up for me when I tried to fix UI Bootstrap with Angular 1.3 (angular-ui/bootstrap#2659)

Fixes #3407

[Narretz]

It's a simple change and I think we should merge this. Let's see what @petebacondarwin says.
However, I don't think $runFormatters should call $$runValidators. We have too much magic happen in ngModel already, where stuff triggers other stuff. If someone wants validation after calling `$runFormatters, that's what $validate is for (which also has some additional checks).

[Narretz]

Oh, since ngModel is planned to change quite a lot in 1.4, it probably only makes sense if we merge this into 1.3.x, too

[petebacondarwin]

I would prefer the new API to be of the form: $setModelValue(newModelValue), which is symmetric with the $setViewValue(newViewValue) on the other end of the pipeline. To get the same functionality as in $runFormatters() you could simply call:

ngModel.$setModelValue(ngModel.$modelValue);

Would this cause any problems if it updates the $$rawModelValue too?

[realityking]

@Narretz I've removed the validation from $runFormatters.
I'd be great if this could make it into 1.3. I was planning on duplicating the code in UI Bootstrap but this way I wouldn't have to.

@petebacondarwin If it would be done this way, shouldn't only $$rawModelValue be set and all following steps in the pipeline be run? However I think there would still be value in a method that only formats the value, especially now that the validators aren't run. Maybe the new method should be called $format to be more consistent with $validate?

[Narretz]

@petebacondarwin I like the idea to make it analogous to $setViewValue() - in that case, though we should run the validators in the end, to keep it consistent. I don't think it's a problem that the $$rawModelValue is set, as long as the value inside $setModelValue() corresponds to the value on the scope. But same as with $setViewValue, it's the dev's responsibility to guarantee this.

@realityking I don't think $format is like $validate, it's really like the non-existent $runParsers function. Would $setModelValue work for your use case?

[realityking]

@Narretz I can changed it to $setModelValue. If it can be guaranteed, that formatters are always run after calling it, that this will cover my use case. I'll write the documentation after I get a general ok on the API.

[petebacondarwin]

Other tests to consider:

• that the formatters are run
• that the validators are run
• what happens if something goes wrong (say a formatter fails) to the modelValue/viewValue/validity
• do the formatters/validators run if the modelValue has not actually changed

[realityking]

If the API is ok, most tests from the next section (model -> view) could be moved here. model -> view basically only needs to tests that it calls $setModelValue if the model value has changed.

[petebacondarwin]

Also what is the relationship between the model expression on the scope and the $modelValue after running $setModelValue... Will the next digest change the model value back?

[realityking]

@petebacondarwin that's a good question, I'll test it to make sure

[realityking]

@petebacondarwin I finally got back to this. Indeed the value is override on the next digest. (I've added a test for this)

I've managed to work around it, but I'm not sure if the fromModel argument should be exposed as part of the API. What do you think?

[joeenzminger]

I ran into this thread while researching a similar problem. My question about adding something to the API would be "why doesn't ngModel watch for changes in formatters and do this on it's own (without adding an API call)?".

[realityking]

@joeenzminger While nice from DX perspective (however such an API would still be necessary if something internal to the formatter changes), I don't think it's worth adding two watchers for each ngModel.

[joeenzminger]

I guess it revolves around how expensive running the formatters is. I don't think it would require adding an extra watch, but it would make the existing watch more expensive (you'd have to run the formatters each digest to see if something internal or external changed). But I see your point.

[realityking]

@petebacondarwin Any news on this?

[petebacondarwin]

I will take a look this week.

[realityking]

@petebacondarwin ping

[petebacondarwin]

Actually looking at this again I don't like that we can set the $modelValue to a value that is different to the scope.

For your use case @realityking you should always call this method as ngModel.$setModelValue(ngModel.$modelValue) and providing a different value should be documented as not supported.
This would remove the need for this test and the associated behaviour.

[realityking]

Thanks for the follow up.

This sounds like a bad API (you can call this function only with this value, nothing else). To be honest, given this comment, I'd be more comfortable returning to $runFormatters (maybe called $format to be more inline with the other methods in ngModelController)

[petebacondarwin]

Hmm, I guess you are right.
If we had ngModel.$formatModel, then I suspect we would need to have a complementary ngModel.$parseView? Since changes to the formatters might well include changes to the parsers too? In which case one would want the new viewValue to be parsed and update the model....

[realityking]

Sounds like a decent idea, even if only for API symmetry reasons. (I believe you can already gain the effect by calling '$setViewValue` with the current view value)

I'll try to get it done over the weekend.

[petebacondarwin]

I am going to be away from next week: @Narretz could you take over this one?

[realityking]

Just a small FYI, I've been working on this a bit but haven't got it done yet. Maybe I get to it this weekend.

[ghost]

Any progress with this?

[rwoloszyn]

Is this going to be merged ?

[rubenswieringa]

It would be great to have this functionality in there – @petebacondarwin / @realityking, do you have any plans to finish the last bit of this PR so it can be merged?