Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Commit

Permalink
feat(Scope): async auto-flush $evalAsync queue when outside of $digest
Browse files Browse the repository at this point in the history
This change causes a new $digest to be scheduled in the next tick if
a task was was sent to the $evalAsync queue from outside of a $digest
or an $apply.

While this mode of operation is not common for most of the user code,
this change means that $q promises that utilze $evalAsync queue to
guarantee asynchronicity of promise apis will now also resolve outside
of a $digest, which turned out to be a big pain point for some developers.

The implementation ensures that we don't do more work than needed and
that we coalese as much work as possible into a single $digest.

The use of $browser instead of setTimeout ensures that we can mock out
and control the scheduling of "auto-flush", which should in theory
allow all of the existing code and tests to work without negative
side-effects.

Closes #3539
Closes #2438
  • Loading branch information
IgorMinar committed Aug 26, 2013
1 parent 42af8ea commit 6b91aa0
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 6 deletions.
23 changes: 18 additions & 5 deletions src/ng/rootScope.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ function $RootScopeProvider(){
return TTL;
};

this.$get = ['$injector', '$exceptionHandler', '$parse',
function( $injector, $exceptionHandler, $parse) {
this.$get = ['$injector', '$exceptionHandler', '$parse', '$browser',
function( $injector, $exceptionHandler, $parse, $browser) {

/**
* @ngdoc function
Expand Down Expand Up @@ -666,20 +666,33 @@ function $RootScopeProvider(){
*
* The `$evalAsync` makes no guarantees as to when the `expression` will be executed, only that:
*
* - it will execute in the current script execution context (before any DOM rendering).
* - at least one {@link ng.$rootScope.Scope#$digest $digest cycle} will be performed after
* `expression` execution.
* - it will execute after the function that schedule the evaluation is done running (preferably before DOM rendering).
* - at least one {@link ng.$rootScope.Scope#$digest $digest cycle} will be performed after `expression` execution.
*
* Any exceptions from the execution of the expression are forwarded to the
* {@link ng.$exceptionHandler $exceptionHandler} service.
*
* __Note:__ if this function is called outside of `$digest` cycle, a new $digest cycle will be scheduled.
* It is however encouraged to always call code that changes the model from withing an `$apply` call.
* That includes code evaluated via `$evalAsync`.
*
* @param {(string|function())=} expression An angular expression to be executed.
*
* - `string`: execute using the rules as defined in {@link guide/expression expression}.
* - `function(scope)`: execute the function with the current `scope` parameter.
*
*/
$evalAsync: function(expr) {
// if we are outside of an $digest loop and this is the first time we are scheduling async task also schedule
// async auto-flush
if (!$rootScope.$$phase && !$rootScope.$$asyncQueue.length) {
$browser.defer(function() {
if ($rootScope.$$asyncQueue.length) {
$rootScope.$digest();
}
});
}

this.$$asyncQueue.push(expr);
},

Expand Down
2 changes: 1 addition & 1 deletion src/ng/timeout.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ function $TimeoutProvider() {
var deferred = $q.defer(),
promise = deferred.promise,
skipApply = (isDefined(invokeApply) && !invokeApply),
timeoutId, cleanup;
timeoutId;

timeoutId = $browser.defer(function() {
try {
Expand Down
51 changes: 51 additions & 0 deletions test/ng/rootScopeSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -705,6 +705,57 @@ describe('Scope', function() {
expect(isolateScope.$$asyncQueue).toBe($rootScope.$$asyncQueue);
expect($rootScope.$$asyncQueue).toEqual(['rootExpression', 'childExpression', 'isolateExpression']);
}));


describe('auto-flushing when queueing outside of an $apply', function() {
var log, $rootScope, $browser;

beforeEach(inject(function(_log_, _$rootScope_, _$browser_) {
log = _log_;
$rootScope = _$rootScope_;
$browser = _$browser_;
}));


it('should auto-flush the queue asynchronously and trigger digest', function() {
$rootScope.$evalAsync(log.fn('eval-ed!'));
$rootScope.$watch(log.fn('digesting'));
expect(log).toEqual([]);

$browser.defer.flush(0);

expect(log).toEqual(['eval-ed!', 'digesting', 'digesting']);
});


it('should not trigger digest asynchronously if the queue is empty in the next tick', function() {
$rootScope.$evalAsync(log.fn('eval-ed!'));
$rootScope.$watch(log.fn('digesting'));
expect(log).toEqual([]);

$rootScope.$digest();

expect(log).toEqual(['eval-ed!', 'digesting', 'digesting']);
log.reset();

$browser.defer.flush(0);

expect(log).toEqual([]);
});


it('should not schedule more than one auto-flush task', function() {
$rootScope.$evalAsync(log.fn('eval-ed 1!'));
$rootScope.$evalAsync(log.fn('eval-ed 2!'));

$browser.defer.flush(0);
expect(log).toEqual(['eval-ed 1!', 'eval-ed 2!']);

expect(function() {
$browser.defer.flush(0);
}).toThrow('No deferred tasks with delay up to 0ms to be flushed!');
});
});
});


Expand Down

9 comments on commit 6b91aa0

@erudianart
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am wondering @IgorMinar why you never created a digestAsync? Why does it only run on rootScope and not an individual scope?

@gkalpak
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what a $digestAsync would be about. You can call $digest whenever you want (e.g. async 😃). And you can run $digest on a specific scope (if you know what you are doing).

@erudianart
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the problem I am having is that I am not always in an angular context, like for instance, my HTTP calls are not using $http but some other data service, so whenever we get data we need to call $digest on the scope to update it. However, we have lots and lots and lots of directives (with isolated scopes) so sometimes we are double digesting - so ideally we have some mechanism to handle that like evalAsync does. We don't want to trigger a $rootScope.$digest because that is terrible for performance.

@gkalpak
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. How about $applyAsync() then ? 😃

@erudianart
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't want to run a $rootScope.$digest - ideally I would like to just run $scope.$digest and have all the nice stuff that evalAsync has in terms of phase/digest resolution

@erudianart
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For context I am working on a MASSIVE angular app, like .. MASSIVE, so we have a lot of watchers, a lot of directives, etc - so a $rootScope.$digest can be really expensive hence why trying to localize things to $scope.$digest();

@gkalpak
Copy link
Member

@gkalpak gkalpak commented on 6b91aa0 Mar 1, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, basically you need a debounced/throttled $scope.$digest or an $applyAsync that only runs $digest on the local scope (not on $rootScope). It should be fairly easy to implement (although not as straight-forwarded). Quick and hacky example

Considering this is a corner case (massive app doing stuff outside of the Angular context), I don't think we are too keen on adding that to core, because running $digest on a specific scope might have unexpected effects (e.g. not propagate changes to all bindings of the app or propagate changes unexpectedly), so we wouldn't want to encourage that.

But that's just my opinion 😃
You could try opening a feature request issue and see what other people think.

@erudianart
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I totally think its specific to us, its a result of a lot of bad practices in our data (i know i know), but more than willing to share my code its essentially just evalAsync swapping out $rootScope.$digest() with $scope.$digest().

@gkalpak
Copy link
Member

@gkalpak gkalpak commented on 6b91aa0 Mar 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like I said, I don't think it's a good fit for core (but maybe other have different opinions).
If you think it might be useful for more people, feel free to submit a PR or open a feature request to kick off some discussion 😃

Please sign in to comment.