Note: we need to check Google3 to see how "breaking" this change will be, so we can figure out a merge plan.

@kara Thanks for the quick review.

Agreed that it was confusing. The callback name ngModelChange to me implied the model had already been changed.

Good point about the previous value. I definitely overlooked that. But as you noted it's possible for the user to store the previous value. If accessing the previous value is necessary, I think I'd be in favor of more explicit semantics like having previousValue on the AbstractControl interface.

Waiting for the next major release is fine. Our team can work around it by explicitly using the FormControl instead of relying on the callback.

presubmit

http://b/73123567 @kara (reverted as cf8d512)

good thing we didn’t merge it into a patch branch

we should discuss what to do about this PR - should we reattempt to land it + fix g3? abandon it? are there other options?

we should decide what to do with this - should we attempt to merge it again, fix g3 and document the breaking change? what's the impact of this on the non-google developers? at google out of all the projects only one seem to be affected.

@StephenFluin can you please help to assess? thanks

I believe this change seems to be worthwhile because it changes an undocumented behavior into an explicit one. Checking with @kara for any other challenges this might create for developers.

@kara asked the fixed cl to be merged. This PR is ready to go now.

Breaking change message (missing from commit):

Previously, ngModelChange was emitted before its underlying control was updated. This was fine if you passed through the value directly through the $event keyword, e.g.

<input [(ngModel)]="name" (ngModelChange)="onChange($event)">

onChange(value) {
console.log(value); // would log updated value
}

However, if you had a handler for the ngModelChange event that checked the value through the control, you would get the old value rather than the updated value. e.g:

<input #modelDir="ngModel" [(ngModel)]="name" (ngModelChange)="onChange(modelDir)">

onChange(ngModel: NgModel) {
console.log(ngModel.value); // would log old value, not updated value
}

Now the value and validity will be updated before the ngModelChange event is emitted, so the same setup will log the updated value.

onChange(ngModel: NgModel) {
console.log(ngModel.value); // will log updated value
}

We think this order will be less confusing when the control is checked directly. You will only need to update your app if it has relied on this bug to keep track of the old control value. If that is the case, you should be able to track the old value directly by saving it on your component.

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}