avoiding reentering the $digest cycle

[Godmar Back]

Here's a weirdness I encountered recently. When angular instantiates a controller, it is in a $digest cycle. Attempts to call $apply will fail.

In my application, I have a retrieveModel function that may or may not complete synchronously, depending on the browser and depending on whether data is in the cache or not.  If it does complete synchronously, calling $apply would trigger an exception since $digest is in progress.  If it doesn't completely synchronously, I must of course wrap the model assignment in $apply.  So the code will be:

function Controller($scope) {

    var inController = true;

    retrieveModel(function (model) {

        function setModel() {

            $scope.model = model;

        }

        if (inController) {

            // retrieveModel completed synchronously, we are already in a $digest cycle,

            // don't trigger another one

            setModel();

        } else {

            $scope.$apply(setModel());

        }

    });

    inController = false;

}

That's awkward and tricky, to say the least - what's the recommended pattern?

 - Godmar

[Andy Joslin]

I would do it this way:

setModel();

if (!$scope.$$phase) $scope.$apply();

$scope.$$phase is set to either '$digest' or null, depending on whether digestion is happening or not.

It is undocumented though, so use it at your own risk.

[Godmar Back]

Not only undocumented, but would also break encapsulation, as you point out.

I'm wondering why $apply() throws this error, anyway. Is it to keep track of the $digest cycle count?  $apply, when called within a $digest cycle, might as well consider the evaluation of the current expression part of the same cycle. Or so one would think - I'm sure there's something I'm missing. Angular is throwing the "already in progress" exception at the user, of course without documenting it at http://docs.angularjs.org/api/ng.$rootScope.Scope or explaining the rationale for why they've decided to make $apply non-reentrant, nor giving advice on how to avoid it :-(

 - Godmar

 

[Peter Bacon Darwin]

The easiest (and documented / supported) way to avoid this sort of thing is to simply defer execution until you know you are not in an $digest:

function Controller($scope, $timeout) {

    retrieveModel(function (model) {

        $timeout(function() {

            $scope.model = model;

        });

    });

}

--
You received this message because you are subscribed to the Google Groups "AngularJS" group.
To post to this group, send email to ang...@googlegroups.com.
To unsubscribe from this group, send email to angular+u...@googlegroups.com.
Visit this group at http://groups.google.com/group/angular?hl=en.
 
 

[Wizek]

+Peter Bacon Darwin

Wouldn't using timeout introduce UI-flicker?

[Peter Bacon Darwin]

Without a delay parameter it runs in the next process loop.  I guess there is a chance that the browser will jump in and render unwanted content in that gap but in practice I haven't encountered this happening.

Pete

[Wizek]

Even if that was true, I believe it goes against the spirit of angular models: $digest until we have consistency across models. Calling timeout will trick angular into thinking that the models are consistent.

Don't you agree?

M.

[Peter Bacon Darwin]

No, I don't agree.  We have asked for a model (from some service that may or may not be asynchronous - we are just forcing it to be asynchronous.

It doesn't try to update the model until the $timeout times-out.  At that point the digest is run (via $apply) so there is no trick.

By the way, in the case that the service IS asynchronous, you have the same problem of potential display of unwanted content.

Pete

[Wizek]

How about a way which does not introduce unnecessary (therefore often times undesired) asynchronicity?

[Peter Bacon Darwin]

Use Andy's unsupported method.

Pete
...from my mobile.

[Wizek]

How about a factory?

.factory('applyIfApplicable', function ($rootScope) {                 

  return function (fnToApply) {                                       

    if ($rootScope.$$phase) {                                         

      return fnToApply()                                              

    } else {                                                          

      return $rootScope.$apply(fnToApply)                             

    }                                                                 

  }                                                                   

})                                                                    

Also, shouldn't something like be a built-in? Like, $apply to do it internally? :) Would there be any harm with that?

[Peter Bacon Darwin]

Originally multiple calls to $digest were quietly disregarded, but since, in general, this is an indication of a programming error, it was decided that actually it should throw an error.

I agree, though, that when working with external libraries, which annoyingly often have methods that may or may not call their callbacks synchronously or asynchronously, it would be reasonable to have a built in alternative to $apply, which explicitly states that you do not know whether the call is within an $digest or not.

Perhaps an extra parameter (which defaults to false) for this...

$apply(exp[, mayAlreadyBeInDigest])

 mayAlreadyBeInDigest{optional=false} - {boolean=} - allow developer to explicitly state that it is not determined whether the callback is in an $apply or not.

--

[Wizek]

I like the optional-argument suggestion, though I dislike the pattern for specifying arguments after a callback, as it can be easily overlooked.

I don't quite have a much better idea, though. Maybe a mirror function, like `.$applyMaybe()` (with a much better and/or more descriptive -- but still succinct name).

What do you guys think?

Also, how can we make sure that a solution (whichever we agree on) gets added to Angular sooner than later?

[Peter Bacon Darwin]

I think overlooking the argument would be a good thing :-)

By the way, that follows the same convention as the delay argument to the $timeout service.

Pete

[Wizek]

I meant overlooking from a readability standpoint, not a documentational one: I dislike seeing things like this in my code:

        }, 1000)
    }
}, true)

It is much nicer to have every argument, even optional ones before the callback/passed fn.

M.

[Godmar Back]

On Sat, Sep 15, 2012 at 4:25 PM, Peter Bacon Darwin <pe...@bacondarwin.com> wrote:

The easiest (and documented / supported) way to avoid this sort of thing is to simply defer execution until you know you are not in an $digest:

What's documented about $timeout?  Angular's doc [1] say it's a wrapper for window.setTimeout(, 0). There are no guarantees as to when setTimeout(, 0) callbacks are executed, at least there haven't been traditionally.  There may well be a rendering cycle in between, leading to flicker.

This reminds me of a discussion with John Resig in 2008 where I claimed that JavaScript guarantees that 0-timeouts execute after the current event handler completes [2]; John's experiments - at the time - indicated that this wasn't actually true in practice (for the browsers of that time).

 - Godmar

[1] http://docs.angularjs.org/api/ng.$timeout

[2] https://groups.google.com/d/msg/jquery-dev/bTXJZEpvYWY/hMlRY-nqDH0J

 

[Godmar Back]

On Sun, Sep 16, 2012 at 4:39 AM, Peter Bacon Darwin <pe...@bacondarwin.com> wrote:

Originally multiple calls to $digest were quietly disregarded, but since, in general, this is an indication of a programming error, it was decided that actually it should throw an error.

How so?  A programming error is a violation of a documented API.  I haven't found documentation or even a description of a rationale for why $apply isn't reentrant.

 - Godmar

 

[Peter Bacon Darwin]

I'm afraid I'm not much of a computer scientist so I'm not in a position to argue the case for $apply not being reentrant.

The $timeout service is documented in the sense that you can read about it in the docs, which is not true for $$phase.

The reason for this discussion thread is that one doesn't know whether a function has been called asynchronously or not.  So I think it is fair to assume that at some point it will be asynchronous.  In that case you are going to have to deal with the situation you describe (a render cycle occuring, leading to flicker) at some point anyway.  This is whether or not you use $timeout.  If you know for sure that it is synchronous then you don't need to worry about $apply.

Pete

--

[grabstein...@gmail.com]

The best way of handling this problem would be by using the $http or $resource service to fetch the data. These takes care of calling $apply for you. If you for some reason cannot use $http or $resource, the recommended approach would be to call $apply in whatever other method that fetches the data from the server.

[Daniel Demmel]

This would be too complicated even if it worked, why don't you set a $q.defer() promise as $scope.model and resolve it from a service for example?

[Wizek]

I think I have a solution to please everyone!

Make the error a warning. Let me explain: As far as I understand this error is only caused by api which may or may not be async. (Or is there any other situation?) Because if the api is always sync, you don't need apply, and the warning will notify you about that. If the api is always async, you'll need apply all the time, and the warning will never come.

If it's the sometimes-sometimes-not situation, you'll still get a warning (to the console, and not disrupting your code with exceptions), to notify if the api you are building upon might have issues. If it is in your reach, you can then modify your api to get rid of the design issue. But if it is external, and you still need it to be sometimes sync, you may add the last optional boolean parameter, which surpasses the warning.

What do you think?

Possible factory implementation:

.factory('apply', function ($log, $rootScope, suppress) {
  return function apply(fn) {
    if ($rootScope.$$phase) {
      fn()
      if (suppress) {
        $log.warn('$digest was in progress. This might indicate a design error either in your code, ' +
          'or more likely in the external api you are using.')
      }
    } else {
      $rootScope.$apply(function () {
        fn()
      })
    }
  }
})

[Peter Bacon Darwin]

Hi Wizek,

Thanks for adding to the discussion :-) I think you meant the code to be?

.factory('apply', function ($log, $rootScope) {
  return function apply(fn, suppress) {

    if ($rootScope.$$phase) {
      fn()
      if (!suppress) {

        $log.warn('$digest was in progress. This might indicate a design error either in your code, ' +
          'or more likely in the external api you are using.')
      }
    } else {
      $rootScope.$apply(function () {
        fn()
      })
    }
  }
}) 

I think that I would make it more explicit, if you know that the code may run synch but you have no control over that then you should make this clear in the call to apply:

.factory('apply', function ($rootScope) {
  return function apply(fn, suppress) {

    if ( !suppress || !$rootScope.$$phase) {

      $rootScope.$apply(fn);

    } else {

      fn()
    }
  }
}) 

apply(function() { /* Stuff that may or may not be synch */ }, true);

Pete