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

Commit 3fa796f

Browse files
sjelinsjelin
committed
fix(ngRoute): make route.resolve count as a pending request
Protractor users were having a problem where if they had asynchonous code in a route.resolve variable, Protractor was not waiting for that code to complete before continuing. See angular/protractor#789 (comment) for details Also enhanced ngMock to wait for pending requests before calling callbacks from `$browser.notifyWhenNoOutstandingRequests` Potentially breaking change if someone was relying on route.resolve not blocking `$browser.notifyWhenNoOutstandingRequests`
1 parent 0694af8 commit 3fa796f

File tree

6 files changed

+144
-10
lines changed

6 files changed

+144
-10
lines changed

‎src/ngMock/angular-mocks.js

+20-8
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,26 @@ angular.mock.$Browser = function() {
3737
self.$$lastUrl = self.$$url; // used by url polling fn
3838
self.pollFns = [];
3939

40-
// TODO(vojta): remove this temporary api
41-
self.$$completeOutstandingRequest = angular.noop;
42-
self.$$incOutstandingRequestCount = angular.noop;
43-
40+
// Testability API
41+
42+
var outstandingRequestCount = 0;
43+
var outstandingRequestCallbacks = [];
44+
self.$$incOutstandingRequestCount = function() { outstandingRequestCount++; };
45+
self.$$completeOutstandingRequest = function() {
46+
outstandingRequestCount--;
47+
if (!outstandingRequestCount) {
48+
while (outstandingRequestCallbacks.length) {
49+
outstandingRequestCallbacks.pop()();
50+
}
51+
}
52+
};
53+
self.notifyWhenNoOutstandingRequests = function(callback) {
54+
if (outstandingRequestCount) {
55+
outstandingRequestCallbacks.push(callback);
56+
} else {
57+
callback();
58+
}
59+
};
4460

4561
// register url polling fn
4662

@@ -166,10 +182,6 @@ angular.mock.$Browser.prototype = {
166182

167183
state: function() {
168184
return this.$$state;
169-
},
170-
171-
notifyWhenNoOutstandingRequests: function(fn) {
172-
fn();
173185
}
174186
};
175187

‎src/ngRoute/route.js

+8-2
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
var isArray;
88
var isObject;
99
var isDefined;
10+
var noop;
1011

1112
/**
1213
* @ngdoc module
@@ -54,6 +55,7 @@ function $RouteProvider() {
5455
isArray = angular.isArray;
5556
isObject = angular.isObject;
5657
isDefined = angular.isDefined;
58+
noop = angular.noop;
5759

5860
function inherit(parent, extra) {
5961
return angular.extend(Object.create(parent), extra);
@@ -350,7 +352,8 @@ function $RouteProvider() {
350352
'$injector',
351353
'$templateRequest',
352354
'$sce',
353-
function($rootScope, $location, $routeParams, $q, $injector, $templateRequest, $sce) {
355+
'$browser',
356+
function($rootScope, $location, $routeParams, $q, $injector, $templateRequest, $sce, $browser) {
354357

355358
/**
356359
* @ngdoc service
@@ -776,6 +779,7 @@ function $RouteProvider() {
776779

777780
function resolveLocals(route) {
778781
if (route) {
782+
$browser.$$incOutstandingRequestCount();
779783
var locals = angular.extend({}, route.resolve);
780784
angular.forEach(locals, function(value, key) {
781785
locals[key] = angular.isString(value) ?
@@ -786,7 +790,9 @@ function $RouteProvider() {
786790
if (angular.isDefined(template)) {
787791
locals['$template'] = template;
788792
}
789-
return $q.all(locals);
793+
return $q.all(locals).finally(function(locals) {
794+
$browser.$$completeOutstandingRequest(noop);
795+
});
790796
}
791797
}
792798

‎test/e2e/fixtures/ng-route-promise/index.html

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<html ng-app="lettersApp">
2+
<head>
3+
<meta charset="utf-8">
4+
<title>Angular.js Example</title>
5+
<script src="angular.js"></script>
6+
<script src="angular-route.js"></script>
7+
<script src="script.js"></script>
8+
</head>
9+
<body>
10+
<div ng-view></div>
11+
</body>
12+
</html>

‎test/e2e/fixtures/ng-route-promise/script.js

+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
'use strict';
2+
3+
var lettersApp = angular.module('lettersApp', ['ngRoute']);
4+
5+
lettersApp.config(function($routeProvider) {
6+
$routeProvider.
7+
when('/', {
8+
template: '<ul><li ng-repeat="letter in letters">{{letter}}</li><ul>',
9+
controller: 'LettersCtrl',
10+
resolve: {
11+
letters: function($q) {
12+
var deferred = $q.defer();
13+
window.setTimeout(function() {
14+
deferred.resolve(['a', 'b', 'c', 'd', 'e']);
15+
}, 1000);
16+
return deferred.promise;
17+
}
18+
}
19+
}).
20+
otherwise({
21+
redirectTo: '/'
22+
});
23+
});
24+
25+
lettersApp.controller('LettersCtrl', function($scope, letters) {
26+
$scope.letters = letters;
27+
});

‎test/e2e/tests/ng-route-promise.spec.js

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
'use strict';
2+
3+
describe('ngRoute promises', function() {
4+
beforeEach(function() {
5+
loadFixture('ng-route-promise');
6+
});
7+
it('should wait for promises in resolve blocks', function() {
8+
expect(element.all(by.tagName('li')).count()).toBe(5);
9+
});
10+
it('should time out if the promise takes long enough', function() {
11+
// Don't try this at home kids, I'm a protractor dev
12+
browser.manage().timeouts().setScriptTimeout(1);
13+
browser.waitForAngular().then(function() {
14+
fail('waitForAngular() should have timed out, but didn\'t');
15+
}, function(error) {
16+
expect(error.message).toContain('Timed out waiting for asynchronous Angular tasks to finish after 0.001 seconds.');
17+
});
18+
});
19+
afterAll(function() {
20+
// Restore old timeout limit
21+
browser.manage().timeouts().setScriptTimeout(browser.getProcessedConfig().allScriptsTimeout);
22+
});
23+
});

‎test/ngRoute/routeSpec.js

+54
Original file line numberDiff line numberDiff line change
@@ -2082,4 +2082,58 @@ describe('$route', function() {
20822082
expect(function() { $route.updateParams(); }).toThrowMinErr('ngRoute', 'norout');
20832083
}));
20842084
});
2085+
2086+
describe('testability', function() {
2087+
it('should wait for route promises before calling callbacks', function() {
2088+
var deferred;
2089+
2090+
module(function($provide, $routeProvider) {
2091+
$routeProvider.when('/path', { template: '', resolve: {
2092+
a: function($q) {
2093+
deferred = $q.defer();
2094+
return deferred.promise;
2095+
}
2096+
} });
2097+
});
2098+
2099+
inject(function($location, $route, $rootScope, $httpBackend, $$testability) {
2100+
$location.path('/path');
2101+
$rootScope.$digest();
2102+
2103+
var callback = jasmine.createSpy('callback');
2104+
$$testability.whenStable(callback);
2105+
expect(callback).not.toHaveBeenCalled();
2106+
2107+
deferred.resolve();
2108+
$rootScope.$digest();
2109+
expect(callback).toHaveBeenCalled();
2110+
});
2111+
});
2112+
2113+
it('should call callback after route promises are rejected', function() {
2114+
var deferred;
2115+
2116+
module(function($provide, $routeProvider) {
2117+
$routeProvider.when('/path', { template: '', resolve: {
2118+
a: function($q) {
2119+
deferred = $q.defer();
2120+
return deferred.promise;
2121+
}
2122+
} });
2123+
});
2124+
2125+
inject(function($location, $route, $rootScope, $httpBackend, $$testability) {
2126+
$location.path('/path');
2127+
$rootScope.$digest();
2128+
2129+
var callback = jasmine.createSpy('callback');
2130+
$$testability.whenStable(callback);
2131+
expect(callback).not.toHaveBeenCalled();
2132+
2133+
deferred.reject();
2134+
$rootScope.$digest();
2135+
expect(callback).toHaveBeenCalled();
2136+
});
2137+
});
2138+
});
20852139
});

0 commit comments

Comments
 (0)