feat($rootScope): allow $watch to be triggered by an event

[odedniv]

Note: I did not yet add documentation/tests because I first want to know if this actually makes sense to do this.

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
feat

What is the current behavior? (You can also link to an open issue here)
Watch expressions are always evaluated on every digest.

What is the new behavior (if this is a feature change)?
Allow $watch to be evaluated when an event is triggered (using $on), with a new syntax "eventName: watchExpression".

Example:

HTML:

<div ng-if="sessionChanged: currentUser">
  <!-- Below doesn't really work yet since I don't know where that is -->
  Hi, {{sessionChanged: currentUser.username}}
</div>

JavaScript:

$rootScope.currentUser = { username: "..." };
$rootScope.$broadcast('sessionChanged');

Does this PR introduce a breaking change?
No.

Other information:
This is actually more of a suggestion/question: I read everywhere that watchers are bad, but there is no true solution for this apart from writing directives that basically destroy the templating concept. I understand that using events ($on) should be better, and so I figured it might be a good idea to patch into $watch (which is used everywhere, ng-if etc) the ability to be triggered by an event rather than adding it to the digest cycle.

[gkalpak]

This seems similar to #6354.

[odedniv]

@gkalpak they seem to offer a different implementation, asking the community if this makes sense, plus I have a simple implementation ready 😄

[lgalfaso]

I would like to know if it is necessary for this to be part of the core. Can this be done thru a third party module? Eg https://github.com/kseamon/fast-bind (or many others)

[odedniv]

Well to do this I had to override $rootScope.$watch. Do third party modules usually do this? Kind of compatibility scary.

I already implemented it as a patch first and only then made the angular pull request, so either way is fine by me. Most important for me is hearing your thoughts about it: should it really make it better? Is it implemented properly?

[odedniv]

• I don't really understand the reference (fast-bind), there is no documentation of what it does or how it works, and it looks like they just copied the entire angular code base and left it for 2 years...

This is a good of example of something no-one should do when creating a third party plug in, I don't want to do that... Do you have any other example?

[lgalfaso]

If you look into http://ngmodules.org/ there are several solutions for this specific issue. Many of these have some shortcomings, and this is why there are some existing PRs that try to solve this issue in different ways. There is #10096 that would be a more generic implementation of what it is proposed here and there is #13524 that would allow a different implementation thru a third party library as this (quite old) RFC

[odedniv]

So... can someone advise whether this is a good performance change at all? (whether it's core or third party)

[gkalpak]

I think this would improve performance in specific usecases. But @lgalfaso is right, this is not something that should be incorporated into core, since this is easily implemented indepently and actually it doesn't need to be tied to scope watchers.

[odedniv]

@gkalpak how can it be implemented without patching scope watches? Can you give an example? All I can think of is a directive that recompiles the entire element when an event triggers...

[gkalpak]

@odedniv , this is not relying on anything that is not available outside core.

Basically, all you need is an extra method on the Scope prototype that takes an eventName, an expression to $parse, a listener to call when the expression value changes and optionally a 4th argument if you need to support "deep-watching".

Here is a simplistic POC.

[petebacondarwin]

Closing as this can be be achieved outside core easily.

[odedniv]

@petebacondarwin this can only be achieved by overriding $watch, not really "outside core" and not really "easily"... unless you know of another way to do it.

I want ng-if to not add a watcher, but subscribe to a broadcast, do you have an example of how to do it without rewriting ng-if (and a ton of other directives).

[petebacondarwin]

@odedniv - perhaps you are right that it is difficult to prevent the watch functions from running on every digest in this way without changing core (or at least patching core). But then decorating RootScope.prototype.$watch is not outside of the scope of what we allow developers to do, hence the module.decorate() method.

[odedniv]

@petebacondarwin $watch is not a factory but a scope function, so it cannot be decorated with module.decorator (if that's what you meant). Also, if you'll check the way I implemented it, I need some non-trivial code which in my app I now have to pretty much copy and hope it won't change (like the watchEquals I extracted for reuse, and the initWatchVal).

This is how my app's code looks now (since this PR I added multi-events functionality, CoffeeScript generated):

angular.module('myApp').run([
    '$rootScope', '$parse', function($rootScope, $parse) {
      var $watch, WATCH_ON_PATTERN;
      WATCH_ON_PATTERN = /^ *[a-zA-Z][a-zA-Z ]+:/;
      $watch = $rootScope.$watch;
      return $rootScope.$watch = function(watchExp, listener, objectEquality) {
        var evaluate, eventNames, events, get, initialValue, last, statement;
        if (typeof watchExp === 'string' && watchExp.match(WATCH_ON_PATTERN)) {
          if (!angular.isFunction(listener)) {
            listener = angular.noop;
          }
          statement = watchExp.split(':');
          eventNames = statement[0];
          get = $parse(statement.slice(1).join(':'));
          last = initialValue = {};
          evaluate = (function(_this) {
            return function() {
              var reallyLast, value;
              value = get(_this);
              if (value !== last && !(objectEquality ? angular.equals(value, last) : typeof value === 'number' && typeof last === 'number' && isNaN(value) && isNaN(last))) {
                reallyLast = last;
                last = objectEquality ? angular.copy(value, null) : value;
                return listener(value, (reallyLast === initialValue ? value : reallyLast), _this);
              }
            };
          })(this);
          evaluate();
          events = [];
          eventNames.split(' ').forEach((function(_this) {
            return function(eventName) {
              eventName = eventName.trim();
              if (eventName) {
                return events.push(_this.$on(eventName, evaluate));
              }
            };
          })(this));
          return (function(_this) {
            return function() {
              return events.forEach(function(event) {
                return event();
              });
            };
          })(this);
        } else {
          return $watch.apply(this, arguments);
        }
      };
    }
  ])

Not exactly pretty on the eyes...

[petebacondarwin]

It doesn't have to be that bad - especially if you don't rely on CoffeeScript :-) - and you can put it in a decorator that can be distributed as a separate module.

See http://plnkr.co/edit/YJuqVXfj66kBw2jm0xyO?p=preview

[odedniv]

That looks just as bad... the copy-pasted logic is my issue with it, not the function declarations. My CoffeeScript version looks fine (I don't care what the translated JavaScript looks like), I just didn't want to properly translate it to prettier JavaScript for the sake of the conversation.

angular.module('myApp').run(['$rootScope', '$parse', ($rootScope, $parse) ->
    # Starts with a letter, then only letters and spaces until :
    # Copied to: directives/alias.coffee
    WATCH_ON_PATTERN = /^ *[a-zA-Z][a-zA-Z ]+:/

    $watch = $rootScope.$watch
    $rootScope.$watch = (watchExp, listener, objectEquality) ->
      if typeof watchExp == 'string' and watchExp.match(WATCH_ON_PATTERN)
        listener = angular.noop if not angular.isFunction(listener)

        statement = watchExp.split(':')
        eventNames = statement[0]
        get = $parse(statement.slice(1).join(':'))

        last = initialValue = {}
        evaluate = =>
          value = get(@)

          if value != last and not (if objectEquality then angular.equals(value, last) else (typeof value == 'number' && typeof last == 'number' and isNaN(value) and isNaN(last)))
            reallyLast = last # before updating
            last = if objectEquality then angular.copy(value, null) else value

            listener(value, (if (reallyLast == initialValue) then value else reallyLast), @)
        evaluate()

        events = []
        eventNames.split(' ').forEach (eventName) =>
          eventName = eventName.trim()
          events.push(@$on(eventName, evaluate)) if eventName
        => events.forEach((event) => event())
      else
        $watch.apply(@, arguments)

[petebacondarwin]

The complexity of the actual code is not that big a deal if it is packaged up as a 3rd party module. There is actually not that much copy-pasted logic - only really the if statement in the evaluate function.

I'll reopen this PR but I'm going to leave it in the Ice Box since it is functionality that "can" be achieved outside of core and doesn't help Angular 1 move semantically closer to Angular 2.

[Narretz]

We are planning to add a different way of stopping watchers: #10658

[odedniv]

@Narretz this PR doesn't "stop watchers". This is a syntax to let "watchers" subscribe to events instead of watching for changes.