feat(ngChange): ensure ng-change listeners are run after validation

[NicBright]

Hi there,

right now, ng-change listeners do not always run after validation. It's only the case if ngModelOptions.allowInvalid is falsy.

Please let me show you (based on the latest master when writing this post, 9421674) the code-flow to better understand, why and what we can do to change that:

1. Let's have a look at the ng-change directive, it utilizes the $viewChangeListeners:
   
2. Right now, the $viewChangeListeners are only run inside $$writeModelToScope:
   
3. $$writeModelToScope is invoked inside $validate which is not a problem, but also inside $$parseAndValidate. In the following Screenshot you'll see the problem. As soon as allowInvalid is truthy, then $$writeModelToScope is invoked before the validators are run. This is problematic, as it should be possible to rely (inside ng-change) on the fact, that the validation state of ngModelCtrl is already up-to-date:

Now, my proposal is, to move the yellow colored code block to come after the validation block.
I think that this change should be relatively trivial if I'm correct that the $$runValidators(...) invocation (line 736) is not dependent on ctrl.$modelValue. As I can't imagine any application relying actually on the contrary (that $viewChangeListeners fire before validation, in case allowInvalid is set to true), I think this would not be a breaking change either.

What do you think?

Regards,
Nicolas

[Narretz]

One one hand, it's reasonable, on the other hand you are definitely changing the behavior if you have an asynchronous validator. Because then you definitely have a delay in updating the model / executing ngChange. So this would be a breaking change.
But that the validation state is indetermined when ngChange fires is not good, that must be said.
So I'm undecided if this is worth changing.

[NicBright]

You're right. I totally forgot to think about async validators. However, concerning my proposal, moving the yellow colored block would not really delay updating the model, as the async validators run asynchronously.

If we'd move the block inside the callback, it would be like you say:

Maybe there are 2 options:

1. improve the guarantee about validation state in terms of "all synchronous validators run before ng-change for allowInvalid: true"
2. make a breaking change and align the behaviour to the standard case ("allowInvalid: false"), where the call to $$writeModelToScope only happens in the doneCallback of $$runValidators, which means that the model update is delayed already, as soon as $asyncValidators are involved.

[vacas5]

I am having a similar issue. For me, ngChange is firing prior to validation completing when NgModelOptions allowInvalid is true. Frustrating as I need the allowInvalid to be true for other reasons, however I still need the result of the validation in order to perform other functions.