After initial values, $onChanges is called with an incorrect `this`

[jonathantyates]

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
$onChanges is being called with an incorrect this after the initial value changes.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://plnkr.co or similar (template: http://plnkr.co/edit/tpl:yBpEi4).
http://plnkr.co/edit/WbpIZLsDfmqjmMvXa3Hp?p=preview

What is the expected behavior?
They should be called with the same this.

What is the motivation / use case for changing the behavior?
To give it more consistent expected behavior.

Which versions of Angular, and which browser / OS are affected by this issue? Did this work in previous versions of Angular? Please also test with the latest stable and snapshot (https://code.angularjs.org/snapshot/) versions.
I am only aware of 1.5.9 being broken, though previous iterations might be too.
Chrome is affected.

Other information (e.g. stacktraces, related issues, suggestions how to fix)
None.

[Narretz]

Your plnkr throws an error (class constructors must be invoked with |new|), I guess this is not intended?

[jonathantyates]

I am not getting this error in Chrome 53. What browser are you using?

[Narretz]

FF 49. It's because you are using a class as a Service. This is currently not well supported, because we cannot detect this in Firefox, so we are not calling new on it.

[jonathantyates]

Since I don't have to worry about FF, here are some Chrome solutions:

1 ) If you change it to a one way binding '<', the scoping is corrected.
2) If you set a flag inside of $onChanges (this.changes_ = changes), you can use it inside of $doCheck when it is called later in the digest cycle.

So my current plan is just to swap to one-way bindings. Do you have any other recommendations for using ES6 classes? Also, why would '<' work while '@' wouldn't?

[jonathantyates]

Also, being able to reference the same instance in $onChanges and $doCheck even though they have different attributes makes no sense.

[gkalpak]

I need to investigate more the exact roots of the problem, but it is related to the combination of pre-assigning bindings and classes. E.g. if you use a function (as the controller), it works as expected.

In 1.5.9 and 1.6+ you will be able to disable bindings pre-assignment, which also gets rid of the problem. (Demo)

Also, being able to reference the same instance in $onChanges and $doCheck even though they have different attributes makes no sense.

I am not sure what you mean by this 😕

[gkalpak]

So, it is a bug:

During compilation, we call initializeDirectiveBindings(), which (among other things) sets up $watchers and $observers for updating the bindings.

Under certain circumstances, we need to re-apply the bindings to a different object later, in which case we must un-register the previous watchers/observers. initializeDirectiveBindings() handily returns a function to do exactly that.

The bug is that we fail to clean up the @-binding observers, so it keeps firing $onChanges on a temporary, useless object.

[gkalpak]

@jonathantyates, since you were the one that found this out, would you be interested in working on a fix?
_{(It should be pretty straight-forward and short. Maybe a couple of lines and a testcase.)}

[jonathantyates]

Thanks @gkalpak for pointing out what was failing. I have created a PR that fixes the issue. (though I am unsure if I am allowed to use ES6 classes in unit tests. Let me know if I need to change it)

[gkalpak]

Awesome! Taking a look.
_{Spoiler: No, you are not allowed to use classes 😃}