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

feat($timeout): overload service API. #9723

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 19 additions & 3 deletions src/ng/timeout.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
function $TimeoutProvider() {
this.$get = ['$rootScope', '$browser', '$q', '$$q', '$exceptionHandler',
function($rootScope, $browser, $q, $$q, $exceptionHandler) {
var deferreds = {};

var deferreds = {};


/**
Expand All @@ -19,20 +20,35 @@ function $TimeoutProvider() {
* The return value of registering a timeout function is a promise, which will be resolved when
* the timeout is reached and the timeout function is executed.
*
* Additionally, the wrapper function can also be used without a target 'fn' function in order
* to generate a delayed promise.
*
* To cancel a timeout request, call `$timeout.cancel(promise)`.
*
* In tests you can use {@link ngMock.$timeout `$timeout.flush()`} to
* synchronously flush the queue of deferred functions.
*
* @param {function()} fn A function, whose execution should be delayed.
* @param {function()} fn A function, whose execution should be delayed. This argument is
* optional and can be skipped if required.
* @param {number=} [delay=0] Delay in milliseconds.
* @param {boolean=} [invokeApply=true] If set to `false` skips model dirty checking, otherwise
* will invoke `fn` within the {@link ng.$rootScope.Scope#$apply $apply} block.
* @returns {Promise} Promise that will be resolved when the timeout is reached. The value this
* promise will be resolved with is the return value of the `fn` function.
*
*/
function timeout(fn, delay, invokeApply) {
function timeout() {
var fn, delay, invokeApply;
if (isFunction(arguments[0])) {
fn = arguments[0],
delay = arguments[1],
invokeApply = arguments[2];
} else {
fn = noop,
delay = arguments[0],
invokeApply = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

if invokeApply is defined when calling $timeout, then it should be respected. BTW, if the default changes when defining or not a function, then the documentation needs to be updated

Copy link
Contributor

Choose a reason for hiding this comment

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

if the default changes, then we're not landing it --- jus saying, that is a really pointless breaking change. But it shouldn't change if your comment is addressed

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, @vasileorza please don't do the arguments[0] etc thing --- use named formal parameters instead, it is so much easier to read

}

var skipApply = (isDefined(invokeApply) && !invokeApply),
deferred = (skipApply ? $$q : $q).defer(),
promise = deferred.promise,
Expand Down
32 changes: 30 additions & 2 deletions test/ng/timeoutSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,17 @@ describe('$timeout', function() {
}));


it('should NOT call $apply if no callback function is used', inject(function($timeout, $rootScope) {
var applySpy = spyOn($rootScope, '$apply').andCallThrough();

$timeout().then(function() {});
expect(applySpy).not.toHaveBeenCalled();

$timeout.flush();
expect(applySpy).not.toHaveBeenCalled();
}));


it('should NOT call $evalAsync or $digest if invokeApply is set to false',
inject(function($timeout, $rootScope) {
var evalAsyncSpy = spyOn($rootScope, '$evalAsync').andCallThrough();
Expand All @@ -69,6 +80,10 @@ describe('$timeout', function() {
$timeout(noop, 123);
expect(defer.callCount).toEqual(1);
expect(defer.mostRecentCall.args[1]).toEqual(123);

$timeout(456);
expect(defer.callCount).toEqual(2);
expect(defer.mostRecentCall.args[1]).toEqual(456);
}));


Expand All @@ -81,6 +96,14 @@ describe('$timeout', function() {

$timeout.flush();
expect(log).toEqual(['timeout', 'promise success: buba']);

var promise = $timeout();

promise.then(function(value) { log('promise success'); }, log.fn('promise error'));
expect(log).toEqual(['timeout', 'promise success: buba']);

$timeout.flush();
expect(log).toEqual(['timeout', 'promise success: buba', 'promise success']);
}));


Expand Down Expand Up @@ -165,19 +188,24 @@ describe('$timeout', function() {
var task1 = jasmine.createSpy('task1'),
task2 = jasmine.createSpy('task2'),
task3 = jasmine.createSpy('task3'),
promise1, promise3;
task4 = jasmine.createSpy('task4'),
promise1, promise3, promise4;

promise1 = $timeout(task1);
$timeout(task2);
promise3 = $timeout(task3, 333);
promise4 = $timeout(333);
promise3.then(task4);

$timeout.cancel(promise3);
$timeout.cancel(promise1);
$timeout.cancel(promise3);
$timeout.cancel(promise4);
$timeout.flush();

expect(task1).not.toHaveBeenCalled();
expect(task2).toHaveBeenCalledOnce();
expect(task3).not.toHaveBeenCalled();
expect(task4).not.toHaveBeenCalled();
}));


Expand Down