Monday, October 21, 2013

Angular JS - you probably shouldn't use $watch in your controllers.

The problem with $watch isn’t so much that it doesn’t work. It definitely works. The problems are two-fold:
  1. It’s hard/hackish to test effectively.
  2. It’s inefficient.

Inefficiency: Adding complexity to your $digest

As I discussed in my other post, a $digest must occur to update the view from the model or the model from the view. This happens in Angular with great frequency. Whenever a digest occurs you it must evaluate all of your registered $watches. To make matters worse, whatever is altering that value you’re $watching, probably already has a $watch associated to it or an $apply you can leverage to update your value in your view.

Bad: $watch a value changed by user input

What do I mean? Suppose you had some sort of input that was changing a value you were watching:
<input type="text" ng-model="foo"/>
And in your controller you were watching foo for some reason so you could update some other value:
$scope.$watch('foo', function (val){
    switch(val) {
        if(val === 'test') {
            $scope.bar = 'foo is testing me';
            } else if (val === 'blah') {
                $scope.bar = 'foo seems indifferent';
            } else {
                $scope.bar = 'I do not understand foo';
        }
   }
});
and what would the test around this look like? UGLY.
$scope.foo = 'test';
$scope.$apply(); //don't forget the magic step!
expect($scope.bar).toBe('foo is testing me'); //makes sense right? 


Better: Use ng-change

Here we can simplify this greatly by just leveraging ng-change on the <input/>:
<input type="text" ng-model="foo" ng-change="updateBar(foo)"/>
In our controller we’d have a a nice, easy to read, easy to test function:
$scope.updateBar = function(foo) {
    if(val === 'test') {
        scope.bar = 'foo is testing me';
    } else if (val === 'blah') {
        $scope.bar = 'foo seems indifferent';
    } else {
            $scope.bar = 'I do not understand foo';
    }
};
And our test is much clearer and cleaner:
$scope.updateBar('test');
expect($scope.bar).toBe('foo is testing me');

Bad: Use $watch to update a value after [some event here]

In this case you might be doing something like getting a value via Ajax, and you think, “Man, $watches are SWEET, I’m going to use one of these bad boys to watch my value”:
app.controller('WidgetCtrl', function($scope, widgetService) {
    $scope.$watch('widgets', function(val) {
        var count = (val && val.length ? val.length : 0);
        $scope.theCountSays = count + ', ' + count + ' widgets! ah! ah!';
    });

    $scope.updateWidgets = function(widgets) {
        $scope.widgets = result;
    };

    $scope.getWidgets = function() {
        widgetService.get().then($scope.updateWidgets);
    };
});
Now look! theCountSays is updated automagically! Isn’t that awesome? No, I say. No it is not.
Look at our test related to it:
$scope.widgets = [1,2,3,4,5];
$scope.$apply(); //Weee! magic!
expect($scope.theCountSays).toBe(5);


Better: Use the event that triggered the change to update your value!

This is really the case for every single use of $watch in a controller… if you need to update something, update it when you need to, not in some catch all $watch.
app.controller('WidgetCtrl', function($scope, widgetService) {
    $scope.updateWidgets = function(widgets) {
       $scope.widgets = result;
       var count = (val && val.length ? val.length : 0);
       $scope.theCountSays = count + ', ' + count + ' widgets! ah! ah!';
    };

    $scope.getWidgets = function() {
        widgetService.get().then($scope.updateWidgets);
    };
});
and the tests are clear again:
$scope.updateWidgets([1,2,3,4,5]);
expect($scope.theCountSays).toBe(5);


Conclusion: Watches are almost never really necessary in a controller.

I realize the examples I gave above are contrived, but I’m happy to take on more specific examples. In the end, the only thing you need to remember when trying to avoid watches is: “What is triggering the change I’m worried about?” and subscribe to that. $watch is really meant to facilitate two-way binding between the model and the DOM as part of constructing directives.

4 comments:

  1. Hi Ben, my understanding of MVC is you do want to be able to listen/watch for model updates as needed and carry out actions based on that.
    From wikipedia MVC: "A model notifies its associated views and controllers when there has been a change in its state. This notification allows the views to produce updated output, and the controllers to change the available set of commands."
    I understand using $watch is ok but the callback must be efficient and follow angular js guidance for usage.

    As complexity increases and more diverse watchers could be interested in a given model change then $watch seems an appropriate way for each listener to monitor an expression and do what they need rather than adding more and more code say to updateWidgets() in the example above.

    Thanks for info on ngChange, must look into that

    ReplyDelete
  2. Thanks a lot for explaining this. I've read your blogs on watch, appy and digest andthe current blog, and what I understood is that perhaps it's better to use ng-change directive most of the times than to use $watch. Am I thinking the Angular way?

    ReplyDelete
    Replies
    1. Yes, I think you are. It's just a lot more efficient and "clean" to update your model in an event driven way, rather than relying on a consistent $digest to monitor changes.

      Delete
  3. Hey Ben! Just want to share my thoughts and explain why I don't agree with what you say about watches in controllers.

    Having a $watch on a property means that you want to keep an invariant true in the specific scope (at least eventually). That is, a way to define calculated properties that depend on other ones. In the first example you wrote, an ng-change could be a possible solution to THAT particular problem, but what happens if later I see that foo is updated from other place other than the input? The ngChange directive would just execute the expression whenever the input is changed, not when the model changes from outside. On the testing side, I don't think the first way of testing is ugly. Actually I think it's more natural, since the idea is that you're testing behavior on the scope, so whenver a property changes, the invariants are still true, so the properties that depend on that modified property change as well. I shouldn't need to remember to call an extra function just to update stuff if there is a dependency between properties. Otherwise that would lead to inconsistent state in the scope.

    On the other hand, I agree on trying to reduce watches if there are better alternatives, like the one you put in the last example, the ajax request. But I wouldn't expose any internal functions, like updateWidgets there, just to test that. I would test directly the getWidgets function, since that's the real API from the controller.

    To sum up, I think watches are actually good, and help you write your controllers in a more event driven way, since you react on changes from other properties and the rules are always true in the scope.

    ReplyDelete

This form allows some basic HTML. It will only create links if you wrap the URL in an anchor tag (Sorry, it's the Blogger default)