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

Commit 04215ae

Browse files
matskomatsko
committed
feat($templateRequest): introduce the $templateRequest service
This handy service is designed to download and cache template contents and to throw an error when a template request fails. BREAKING CHANGE Angular will now throw a $compile minErr each a template fails to download for ngView, directives and ngMessage template requests. This changes the former behavior of silently ignoring failed HTTP requests--or when the template itself is empty. Please ensure that all directive, ngView and ngMessage code now properly addresses this scenario. NgInclude is uneffected from this change.
1 parent 3be00df commit 04215ae

File tree

11 files changed

+187
-42
lines changed

11 files changed

+187
-42
lines changed

Diff for: ‎angularFiles.js

+1
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ var angularFiles = {
3434
'src/ng/sanitizeUri.js',
3535
'src/ng/sce.js',
3636
'src/ng/sniffer.js',
37+
'src/ng/templateRequest.js',
3738
'src/ng/timeout.js',
3839
'src/ng/urlUtils.js',
3940
'src/ng/window.js',

Diff for: ‎src/AngularPublic.js

+2
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@
7878
$SceDelegateProvider,
7979
$SnifferProvider,
8080
$TemplateCacheProvider,
81+
$TemplateRequestProvider,
8182
$TimeoutProvider,
8283
$$RAFProvider,
8384
$$AsyncCallbackProvider,
@@ -227,6 +228,7 @@ function publishExternalAPI(angular){
227228
$sceDelegate: $SceDelegateProvider,
228229
$sniffer: $SnifferProvider,
229230
$templateCache: $TemplateCacheProvider,
231+
$templateRequest: $TemplateRequestProvider,
230232
$timeout: $TimeoutProvider,
231233
$window: $WindowProvider,
232234
$$rAF: $$RAFProvider,

Diff for: ‎src/ng/compile.js

+4-7
Original file line numberDiff line numberDiff line change
@@ -669,9 +669,9 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
669669
};
670670

671671
this.$get = [
672-
'$injector', '$interpolate', '$exceptionHandler', '$http', '$templateCache', '$parse',
672+
'$injector', '$interpolate', '$exceptionHandler', '$templateRequest', '$parse',
673673
'$controller', '$rootScope', '$document', '$sce', '$animate', '$$sanitizeUri',
674-
function($injector, $interpolate, $exceptionHandler, $http, $templateCache, $parse,
674+
function($injector, $interpolate, $exceptionHandler, $templateRequest, $parse,
675675
$controller, $rootScope, $document, $sce, $animate, $$sanitizeUri) {
676676

677677
var Attributes = function(element, attributesToCopy) {
@@ -1827,8 +1827,8 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
18271827

18281828
$compileNode.empty();
18291829

1830-
$http.get($sce.getTrustedResourceUrl(templateUrl), {cache: $templateCache}).
1831-
success(function(content) {
1830+
$templateRequest($sce.getTrustedResourceUrl(templateUrl))
1831+
.then(function(content) {
18321832
var compileNode, tempTemplateAttrs, $template, childBoundTranscludeFn;
18331833

18341834
content = denormalizeTemplate(content);
@@ -1903,9 +1903,6 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
19031903
childBoundTranscludeFn);
19041904
}
19051905
linkQueue = null;
1906-
}).
1907-
error(function(response, code, headers, config) {
1908-
throw $compileMinErr('tpload', 'Failed to load template: {0}', config.url);
19091906
});
19101907

19111908
return function delayedNodeLinkFn(ignoreChildLinkFn, scope, node, rootElement, boundTranscludeFn) {

Diff for: ‎src/ng/directive/ngInclude.js

+6-4
Original file line numberDiff line numberDiff line change
@@ -169,8 +169,8 @@
169169
* @description
170170
* Emitted when a template HTTP request yields an erronous response (status < 200 || status > 299)
171171
*/
172-
var ngIncludeDirective = ['$http', '$templateCache', '$anchorScroll', '$animate', '$sce',
173-
function($http, $templateCache, $anchorScroll, $animate, $sce) {
172+
var ngIncludeDirective = ['$templateRequest', '$anchorScroll', '$animate', '$sce',
173+
function($templateRequest, $anchorScroll, $animate, $sce) {
174174
return {
175175
restrict: 'ECA',
176176
priority: 400,
@@ -215,7 +215,9 @@ var ngIncludeDirective = ['$http', '$templateCache', '$anchorScroll', '$animate'
215215
var thisChangeId = ++changeCounter;
216216

217217
if (src) {
218-
$http.get(src, {cache: $templateCache}).success(function(response) {
218+
//set the 2nd param to true to ignore the template request error so that the inner
219+
//contents and scope can be cleaned up.
220+
$templateRequest(src, true).then(function(response) {
219221
if (thisChangeId !== changeCounter) return;
220222
var newScope = scope.$new();
221223
ctrl.template = response;
@@ -236,7 +238,7 @@ var ngIncludeDirective = ['$http', '$templateCache', '$anchorScroll', '$animate'
236238

237239
currentScope.$emit('$includeContentLoaded');
238240
scope.$eval(onloadExp);
239-
}).error(function() {
241+
}, function() {
240242
if (thisChangeId === changeCounter) {
241243
cleanupLastIncludeContent();
242244
scope.$emit('$includeContentError');

Diff for: ‎src/ng/templateRequest.js

+53
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
'use strict';
2+
3+
var $compileMinErr = minErr('$compile');
4+
5+
/**
6+
* @ngdoc service
7+
* @name $templateRequest
8+
*
9+
* @description
10+
* The `$templateRequest` service downloads the provided template using `$http` and, upon success,
11+
* stores the contents inside of `$templateCache`. If the HTTP request fails or the response data
12+
* of the HTTP request is empty then a `$compile` error will be thrown (the exception can be thwarted
13+
* by setting the 2nd parameter of the function to true).
14+
*
15+
* @param {string} tpl The HTTP request template URL
16+
* @param {boolean=} ignoreRequestError Whether or not to ignore the exception when the request fails or the template is empty
17+
*
18+
* @return {Promise} the HTTP Promise for the given.
19+
*
20+
* @property {number} totalPendingRequests total amount of pending template requests being downloaded.
21+
*/
22+
function $TemplateRequestProvider() {
23+
this.$get = ['$templateCache', '$http', '$q', function($templateCache, $http, $q) {
24+
function handleRequestFn(tpl, ignoreRequestError) {
25+
var self = handleRequestFn;
26+
self.totalPendingRequests++;
27+
28+
return $http.get(tpl, { cache : $templateCache })
29+
.then(function(response) {
30+
var html = response.data;
31+
if(!html || html.length === 0) {
32+
return handleError();
33+
}
34+
35+
self.totalPendingRequests--;
36+
$templateCache.put(tpl, html);
37+
return html;
38+
}, handleError);
39+
40+
function handleError() {
41+
self.totalPendingRequests--;
42+
if (!ignoreRequestError) {
43+
throw $compileMinErr('tpload', 'Failed to load template: {0}', tpl);
44+
}
45+
return $q.reject();
46+
}
47+
}
48+
49+
handleRequestFn.totalPendingRequests = 0;
50+
51+
return handleRequestFn;
52+
}];
53+
}

Diff for: ‎src/ngMessages/messages.js

+4-4
Original file line numberDiff line numberDiff line change
@@ -228,8 +228,8 @@ angular.module('ngMessages', [])
228228
* </file>
229229
* </example>
230230
*/
231-
.directive('ngMessages', ['$compile', '$animate', '$http', '$templateCache',
232-
function($compile, $animate, $http, $templateCache) {
231+
.directive('ngMessages', ['$compile', '$animate', '$templateRequest',
232+
function($compile, $animate, $templateRequest) {
233233
var ACTIVE_CLASS = 'ng-active';
234234
var INACTIVE_CLASS = 'ng-inactive';
235235

@@ -296,8 +296,8 @@ angular.module('ngMessages', [])
296296

297297
var tpl = $attrs.ngMessagesInclude || $attrs.include;
298298
if(tpl) {
299-
$http.get(tpl, { cache: $templateCache })
300-
.success(function processTemplate(html) {
299+
$templateRequest(tpl)
300+
.then(function processTemplate(html) {
301301
var after, container = angular.element('<div/>').html(html);
302302
angular.forEach(container.children(), function(elm) {
303303
elm = angular.element(elm);

Diff for: ‎src/ngRoute/route.js

+3-5
Original file line numberDiff line numberDiff line change
@@ -226,10 +226,9 @@ function $RouteProvider(){
226226
'$routeParams',
227227
'$q',
228228
'$injector',
229-
'$http',
230-
'$templateCache',
229+
'$templateRequest',
231230
'$sce',
232-
function($rootScope, $location, $routeParams, $q, $injector, $http, $templateCache, $sce) {
231+
function($rootScope, $location, $routeParams, $q, $injector, $templateRequest, $sce) {
233232

234233
/**
235234
* @ngdoc service
@@ -556,8 +555,7 @@ function $RouteProvider(){
556555
templateUrl = $sce.getTrustedResourceUrl(templateUrl);
557556
if (angular.isDefined(templateUrl)) {
558557
next.loadedTemplateUrl = templateUrl;
559-
template = $http.get(templateUrl, {cache: $templateCache}).
560-
then(function(response) { return response.data; });
558+
template = $templateRequest(templateUrl);
561559
}
562560
}
563561
if (angular.isDefined(template)) {

Diff for: ‎test/ng/directive/ngIncludeSpec.js

+1
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,7 @@ describe('ngInclude', function() {
199199
$rootScope.url = 'url2';
200200
$rootScope.$digest();
201201
$httpBackend.flush();
202+
202203
expect($rootScope.$$childHead).toBeFalsy();
203204
expect(element.text()).toBe('');
204205

Diff for: ‎test/ng/templateRequestSpec.js

+89
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
'use strict';
2+
3+
describe('$templateRequest', function() {
4+
5+
it('should download the provided template file',
6+
inject(function($rootScope, $templateRequest, $httpBackend) {
7+
8+
$httpBackend.expectGET('tpl.html').respond('<div>abc</div>');
9+
10+
var content;
11+
$templateRequest('tpl.html').then(function(html) { content = html; });
12+
13+
$rootScope.$digest();
14+
$httpBackend.flush();
15+
16+
expect(content).toBe('<div>abc</div>');
17+
}));
18+
19+
it('should cache the request using $templateCache to prevent extra downloads',
20+
inject(function($rootScope, $templateRequest, $templateCache) {
21+
22+
$templateCache.put('tpl.html', 'matias');
23+
24+
var content;
25+
$templateRequest('tpl.html').then(function(html) { content = html; });
26+
27+
$rootScope.$digest();
28+
expect(content).toBe('matias');
29+
}));
30+
31+
it('should throw an error when the template is not found',
32+
inject(function($rootScope, $templateRequest, $httpBackend) {
33+
34+
$httpBackend.expectGET('tpl.html').respond(404);
35+
36+
$templateRequest('tpl.html');
37+
38+
$rootScope.$digest();
39+
40+
expect(function() {
41+
$rootScope.$digest();
42+
$httpBackend.flush();
43+
}).toThrowMinErr('$compile', 'tpload', 'Failed to load template: tpl.html');
44+
}));
45+
46+
it('should throw an error when the template is empty',
47+
inject(function($rootScope, $templateRequest, $httpBackend) {
48+
49+
$httpBackend.expectGET('tpl.html').respond('');
50+
51+
$templateRequest('tpl.html');
52+
53+
$rootScope.$digest();
54+
55+
expect(function() {
56+
$rootScope.$digest();
57+
$httpBackend.flush();
58+
}).toThrowMinErr('$compile', 'tpload', 'Failed to load template: tpl.html');
59+
}));
60+
61+
it('should keep track of how many requests are going on',
62+
inject(function($rootScope, $templateRequest, $httpBackend) {
63+
64+
$httpBackend.expectGET('a.html').respond('a');
65+
$httpBackend.expectGET('b.html').respond('c');
66+
$templateRequest('a.html');
67+
$templateRequest('b.html');
68+
69+
expect($templateRequest.totalPendingRequests).toBe(2);
70+
71+
$rootScope.$digest();
72+
$httpBackend.flush();
73+
74+
expect($templateRequest.totalPendingRequests).toBe(0);
75+
76+
$httpBackend.expectGET('c.html').respond(404);
77+
$templateRequest('c.html');
78+
79+
expect($templateRequest.totalPendingRequests).toBe(1);
80+
$rootScope.$digest();
81+
82+
try {
83+
$httpBackend.flush();
84+
} catch(e) {}
85+
86+
expect($templateRequest.totalPendingRequests).toBe(0);
87+
}));
88+
89+
});

Diff for: ‎test/ngRoute/directive/ngViewSpec.js

+6-5
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ describe('ngView', function() {
5656
});
5757

5858

59-
it('should instantiate controller for empty template', function() {
59+
it('should not instantiate the associated controller when an empty template is downloaded', function() {
6060
var log = [], controllerScope,
6161
Ctrl = function($scope) {
6262
controllerScope = $scope;
@@ -70,11 +70,12 @@ describe('ngView', function() {
7070
inject(function($route, $rootScope, $templateCache, $location) {
7171
$templateCache.put('/tpl.html', [200, '', {}]);
7272
$location.path('/some');
73-
$rootScope.$digest();
7473

75-
expect(controllerScope.$parent).toBe($rootScope);
76-
expect(controllerScope).toBe($route.current.scope);
77-
expect(log).toEqual(['ctrl-init']);
74+
expect(function() {
75+
$rootScope.$digest();
76+
}).toThrowMinErr('$compile', 'tpload', 'Failed to load template: /tpl.html');
77+
78+
expect(controllerScope).toBeUndefined();
7879
});
7980
});
8081

Diff for: ‎test/ngRoute/routeSpec.js

+18-17
Original file line numberDiff line numberDiff line change
@@ -671,35 +671,36 @@ describe('$route', function() {
671671
});
672672

673673

674-
it('should drop in progress route change when new route change occurs and old fails', function() {
675-
module(function($routeProvider) {
674+
it('should throw an error when a template is empty or not found', function() {
675+
module(function($routeProvider, $exceptionHandlerProvider) {
676+
$exceptionHandlerProvider.mode('log');
676677
$routeProvider.
677678
when('/r1', { templateUrl: 'r1.html' }).
678-
when('/r2', { templateUrl: 'r2.html' });
679+
when('/r2', { templateUrl: 'r2.html' }).
680+
when('/r3', { templateUrl: 'r3.html' });
679681
});
680682

681-
inject(function($route, $httpBackend, $location, $rootScope) {
682-
var log = '';
683-
$rootScope.$on('$routeChangeError', function(e, next, last, error) {
684-
log += '$failed(' + next.templateUrl + ', ' + error.status + ');';
685-
});
686-
$rootScope.$on('$routeChangeStart', function(e, next) { log += '$before(' + next.templateUrl + ');'; });
687-
$rootScope.$on('$routeChangeSuccess', function(e, next) { log += '$after(' + next.templateUrl + ');'; });
688-
683+
inject(function($route, $httpBackend, $location, $rootScope, $exceptionHandler) {
689684
$httpBackend.expectGET('r1.html').respond(404, 'R1');
690-
$httpBackend.expectGET('r2.html').respond('R2');
691-
692685
$location.path('/r1');
693686
$rootScope.$digest();
694-
expect(log).toBe('$before(r1.html);');
695687

688+
$httpBackend.flush();
689+
expect($exceptionHandler.errors.pop().message).toContain("[$compile:tpload] Failed to load template: r1.html");
690+
691+
$httpBackend.expectGET('r2.html').respond('');
696692
$location.path('/r2');
697693
$rootScope.$digest();
698-
expect(log).toBe('$before(r1.html);$before(r2.html);');
699694

700695
$httpBackend.flush();
701-
expect(log).toBe('$before(r1.html);$before(r2.html);$after(r2.html);');
702-
expect(log).not.toContain('$after(r1.html);');
696+
expect($exceptionHandler.errors.pop().message).toContain("[$compile:tpload] Failed to load template: r2.html");
697+
698+
$httpBackend.expectGET('r3.html').respond('abc');
699+
$location.path('/r3');
700+
$rootScope.$digest();
701+
702+
$httpBackend.flush();
703+
expect($exceptionHandler.errors.length).toBe(0);
703704
});
704705
});
705706

0 commit comments

Comments
 (0)