Skip to content

Commit cdaa6a9

Browse files
jbedardNarretz
authored andcommitted
fix($timeout/$interval): do not trigger a digest on cancel
Previously, `.catch(noop)` was used on a rejected timeout/interval to prevent an unhandled rejection error (introduced in #c9dffde1cb). However this would schedule a deferred task to run the `noop`. If the cancelling was outside a digest this could cause a new digest such as with the ng-model `debounce` option. For unit testing, this means that it's no longer necessary to use `$timeout.flush()` when a `$timeout` has been cancelled outside of a digest. Previously, this was necessary to execute the deferred task added by `.catch(noop). There's an example of such a change in this commit's changeset in the file `/test/ngAnimate/animateCssSpec.js`. Fixes angular#16057 Closes angular#16064
1 parent bf0af6d commit cdaa6a9

File tree

8 files changed

+48
-8
lines changed

8 files changed

+48
-8
lines changed

src/.eslintrc.json

+3
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,9 @@
168168
/* ng/compile.js */
169169
"directiveNormalize": false,
170170

171+
/* ng/q.js */
172+
"markQExceptionHandled": false,
173+
171174
/* ng/directive/directives.js */
172175
"ngDirective": false,
173176

src/ng/interval.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ function $IntervalProvider() {
190190
interval.cancel = function(promise) {
191191
if (promise && promise.$$intervalId in intervals) {
192192
// Interval cancels should not report as unhandled promise.
193-
intervals[promise.$$intervalId].promise.catch(noop);
193+
markQExceptionHandled(intervals[promise.$$intervalId].promise);
194194
intervals[promise.$$intervalId].reject('canceled');
195195
$window.clearInterval(promise.$$intervalId);
196196
delete intervals[promise.$$intervalId];

src/ng/q.js

+14-4
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,7 @@ function qFactory(nextTick, exceptionHandler, errorOnUnhandledRejections) {
351351
state.pending = undefined;
352352
try {
353353
for (var i = 0, ii = pending.length; i < ii; ++i) {
354-
state.pur = true;
354+
markQStateExceptionHandled(state);
355355
promise = pending[i][0];
356356
fn = pending[i][state.status];
357357
try {
@@ -378,8 +378,8 @@ function qFactory(nextTick, exceptionHandler, errorOnUnhandledRejections) {
378378
// eslint-disable-next-line no-unmodified-loop-condition
379379
while (!queueSize && checkQueue.length) {
380380
var toCheck = checkQueue.shift();
381-
if (!toCheck.pur) {
382-
toCheck.pur = true;
381+
if (!isStateExceptionHandled(toCheck)) {
382+
markQStateExceptionHandled(toCheck);
383383
var errorMessage = 'Possibly unhandled rejection: ' + toDebugString(toCheck.value);
384384
if (isError(toCheck.value)) {
385385
exceptionHandler(toCheck.value, errorMessage);
@@ -391,7 +391,7 @@ function qFactory(nextTick, exceptionHandler, errorOnUnhandledRejections) {
391391
}
392392

393393
function scheduleProcessQueue(state) {
394-
if (errorOnUnhandledRejections && !state.pending && state.status === 2 && !state.pur) {
394+
if (errorOnUnhandledRejections && !state.pending && state.status === 2 && !isStateExceptionHandled(state)) {
395395
if (queueSize === 0 && checkQueue.length === 0) {
396396
nextTick(processChecks);
397397
}
@@ -671,3 +671,13 @@ function qFactory(nextTick, exceptionHandler, errorOnUnhandledRejections) {
671671

672672
return $Q;
673673
}
674+
675+
function isStateExceptionHandled(state) {
676+
return !!state.pur;
677+
}
678+
function markQStateExceptionHandled(state) {
679+
state.pur = true;
680+
}
681+
function markQExceptionHandled(q) {
682+
markQStateExceptionHandled(q.$$state);
683+
}

src/ng/timeout.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ function $TimeoutProvider() {
8585
timeout.cancel = function(promise) {
8686
if (promise && promise.$$timeoutId in deferreds) {
8787
// Timeout cancels should not report an unhandled promise.
88-
deferreds[promise.$$timeoutId].promise.catch(noop);
88+
markQExceptionHandled(deferreds[promise.$$timeoutId].promise);
8989
deferreds[promise.$$timeoutId].reject('canceled');
9090
delete deferreds[promise.$$timeoutId];
9191
return $browser.defer.cancel(promise.$$timeoutId);

test/ng/directive/ngModelOptionsSpec.js

+6
Original file line numberDiff line numberDiff line change
@@ -459,8 +459,14 @@ describe('ngModelOptions', function() {
459459
$rootScope.$watch(watchSpy);
460460

461461
helper.changeInputValueTo('a');
462+
$timeout.flush(2000);
463+
expect(watchSpy).not.toHaveBeenCalled();
464+
465+
helper.changeInputValueTo('b');
466+
$timeout.flush(2000);
462467
expect(watchSpy).not.toHaveBeenCalled();
463468

469+
helper.changeInputValueTo('c');
464470
$timeout.flush(10000);
465471
expect(watchSpy).toHaveBeenCalled();
466472
});

test/ng/intervalSpec.js

+11
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,17 @@ describe('$interval', function() {
339339
inject(function($interval) {
340340
expect($interval.cancel()).toBe(false);
341341
}));
342+
343+
344+
it('should not trigger digest when cancelled', inject(function($interval, $rootScope, $browser) {
345+
var watchSpy = jasmine.createSpy('watchSpy');
346+
$rootScope.$watch(watchSpy);
347+
348+
var t = $interval();
349+
$interval.cancel(t);
350+
expect(function() {$browser.defer.flush();}).toThrowError('No deferred tasks to be flushed');
351+
expect(watchSpy).not.toHaveBeenCalled();
352+
}));
342353
});
343354

344355
describe('$window delegation', function() {

test/ng/timeoutSpec.js

+11
Original file line numberDiff line numberDiff line change
@@ -299,5 +299,16 @@ describe('$timeout', function() {
299299
$timeout.cancel(promise);
300300
expect(cancelSpy).toHaveBeenCalledOnce();
301301
}));
302+
303+
304+
it('should not trigger digest when cancelled', inject(function($timeout, $rootScope, $browser) {
305+
var watchSpy = jasmine.createSpy('watchSpy');
306+
$rootScope.$watch(watchSpy);
307+
308+
var t = $timeout();
309+
$timeout.cancel(t);
310+
expect(function() {$browser.defer.flush();}).toThrowError('No deferred tasks to be flushed');
311+
expect(watchSpy).not.toHaveBeenCalled();
312+
}));
302313
});
303314
});

test/ngAnimate/animateCssSpec.js

+1-2
Original file line numberDiff line numberDiff line change
@@ -1328,8 +1328,7 @@ describe('ngAnimate $animateCss', function() {
13281328
animator.end();
13291329

13301330
expect(element.data(ANIMATE_TIMER_KEY)).toBeUndefined();
1331-
$timeout.flush();
1332-
expect(function() {$timeout.verifyNoPendingTasks();}).not.toThrow();
1331+
$timeout.verifyNoPendingTasks();
13331332
}));
13341333

13351334
});

0 commit comments

Comments
 (0)