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

Commit d557ec1

Browse files
committed
fix($timeout/$interval): do not trigger a digest on cancel
Previously `.catch(noop)` was used on the rejected timeout/interval to prevent an unhandled rejection error. 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. Fixes #16057
1 parent e58bcfa commit d557ec1

File tree

7 files changed

+42
-10
lines changed

7 files changed

+42
-10
lines changed

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

+3-2
Original file line numberDiff line numberDiff line change
@@ -449,7 +449,7 @@ describe('ngModelOptions', function() {
449449
});
450450

451451

452-
it('should not trigger digest while debouncing', function() {
452+
it('should not trigger digest while debouncing', inject(function($browser) {
453453
var inputElm = helper.compileInput(
454454
'<input type="text" ng-model="name" name="alias" ' +
455455
'ng-model-options="{ debounce: 10000 }"' +
@@ -459,11 +459,12 @@ describe('ngModelOptions', function() {
459459
$rootScope.$watch(watchSpy);
460460

461461
helper.changeInputValueTo('a');
462+
expect(function() {$browser.defer.flush();}).toThrowError('No deferred tasks to be flushed');
462463
expect(watchSpy).not.toHaveBeenCalled();
463464

464465
$timeout.flush(10000);
465466
expect(watchSpy).toHaveBeenCalled();
466-
});
467+
}));
467468

468469

469470
it('should allow selecting different debounce timeouts for each event',

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)