Do not call reaction functions with stale values

[jbdeboer]

@tbosch identified this issue in Angular 2.0 / watchtower.js.

Since we batch reaction functions, we may be calling reaction functions with stale values. This leads to unnecessary digests, or in the worst case, infinite (bounded by TTL) loops.

Instead, during the reaction phase, immediately before calling the reaction function, we should recompute the value. If the value has changed back to the old value, do not call the reaction function.

As part of this change, we should add an intermediate "assert" change which counts the number of times this situation occurs. We can run this assert in apps to understand the impact of this change.

cc @mhevery

[jbdeboer]

Should we even be batching reaction functions at all?

[naomiblack]

Separating reaction functions out makes it easier to understand their cost within the digest loop. We originally made this change to be most consistent with object.observe.

[jbdeboer]

@naomiblack There may be other ways to understand the cost.

e.g. #1119 (UserTags)
e.g. count the last digest (stable) as the field + eval cost and then subtract out the reaction time from the total.

[caitp]

@jbdeboer is it possible that TC39 had actual good reasons to avoid passing the current value to listeners? Maybe it's the right thing to do. I'm interested in reading their notes on it, but I haven't been able to find discussions on that yet. If something is too big of a performance or complexity hit for the platform, maybe it stands to reason that it doesn't make much sense in Angular, either

[IgorMinar]

I think that this issue should be renamed to something more generic. Like "Separating dirty-checks and reactions functions results in doing extra work"

The issues that we see are:

• stale values passed to reaction functions result in reaction functions running several times
• we need to perform several more dirty checks because after each reaction, we need to verify that nothing has changed, but because of the stale value issue, we end up doing dirty check just to realize that some value passed into reaction fn was stale. afterwards we need to dirty-check again.
• several reaction functions deregister watches, and usually these reaction functions belong to watches that fire before the watches that are being deregistered (think ng-include or ng-if containing templates with watches). This means that with the current model, if an ng-include that is being cleared out in the current apply contains 1000 watches, we first realize that the ng-include watch is dirty, then we dirty check all the nested watches and only then we deregister the watches. This is wasteful.