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

Commit

Permalink
feat($templateRequest): introduce the $templateRequest service
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
matsko committed Aug 28, 2014
1 parent 3be00df commit a70e283
Show file tree
Hide file tree
Showing 11 changed files with 187 additions and 42 deletions.
1 change: 1 addition & 0 deletions angularFiles.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ var angularFiles = {
'src/ng/sanitizeUri.js',
'src/ng/sce.js',
'src/ng/sniffer.js',
'src/ng/templateRequest.js',
'src/ng/timeout.js',
'src/ng/urlUtils.js',
'src/ng/window.js',
Expand Down
2 changes: 2 additions & 0 deletions src/AngularPublic.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@
$SceDelegateProvider,
$SnifferProvider,
$TemplateCacheProvider,
$TemplateRequestProvider,
$TimeoutProvider,
$$RAFProvider,
$$AsyncCallbackProvider,
Expand Down Expand Up @@ -227,6 +228,7 @@ function publishExternalAPI(angular){
$sceDelegate: $SceDelegateProvider,
$sniffer: $SnifferProvider,
$templateCache: $TemplateCacheProvider,
$templateRequest: $TemplateRequestProvider,
$timeout: $TimeoutProvider,
$window: $WindowProvider,
$$rAF: $$RAFProvider,
Expand Down
11 changes: 4 additions & 7 deletions src/ng/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -669,9 +669,9 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
};

this.$get = [
'$injector', '$interpolate', '$exceptionHandler', '$http', '$templateCache', '$parse',
'$injector', '$interpolate', '$exceptionHandler', '$templateRequest', '$parse',
'$controller', '$rootScope', '$document', '$sce', '$animate', '$$sanitizeUri',
function($injector, $interpolate, $exceptionHandler, $http, $templateCache, $parse,
function($injector, $interpolate, $exceptionHandler, $templateRequest, $parse,
$controller, $rootScope, $document, $sce, $animate, $$sanitizeUri) {

var Attributes = function(element, attributesToCopy) {
Expand Down Expand Up @@ -1827,8 +1827,8 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {

$compileNode.empty();

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

content = denormalizeTemplate(content);
Expand Down Expand Up @@ -1903,9 +1903,6 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
childBoundTranscludeFn);
}
linkQueue = null;
}).
error(function(response, code, headers, config) {
throw $compileMinErr('tpload', 'Failed to load template: {0}', config.url);
});

return function delayedNodeLinkFn(ignoreChildLinkFn, scope, node, rootElement, boundTranscludeFn) {
Expand Down
10 changes: 6 additions & 4 deletions src/ng/directive/ngInclude.js
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,8 @@
* @description
* Emitted when a template HTTP request yields an erronous response (status < 200 || status > 299)
*/
var ngIncludeDirective = ['$http', '$templateCache', '$anchorScroll', '$animate', '$sce',
function($http, $templateCache, $anchorScroll, $animate, $sce) {
var ngIncludeDirective = ['$templateRequest', '$anchorScroll', '$animate', '$sce',
function($templateRequest, $anchorScroll, $animate, $sce) {
return {
restrict: 'ECA',
priority: 400,
Expand Down Expand Up @@ -215,7 +215,9 @@ var ngIncludeDirective = ['$http', '$templateCache', '$anchorScroll', '$animate'
var thisChangeId = ++changeCounter;

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

currentScope.$emit('$includeContentLoaded');
scope.$eval(onloadExp);
}).error(function() {
}, function() {
if (thisChangeId === changeCounter) {
cleanupLastIncludeContent();
scope.$emit('$includeContentError');
Expand Down
53 changes: 53 additions & 0 deletions src/ng/templateRequest.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
'use strict';

var $compileMinErr = minErr('$compile');

/**
* @ngdoc service
* @name $templateRequest
*
* @description
* The `$templateRequest` service downloads the provided template using `$http` and, upon success,
* stores the contents inside of `$templateCache`. If the HTTP request fails or the response data
* of the HTTP request is empty then a `$compile` error will be thrown (the exception can be thwarted
* by setting the 2nd parameter of the function to true).
*
* @param {string} tpl The HTTP request template URL
* @param {boolean=} ignoreRequestError Whether or not to ignore the exception when the request fails or the template is empty
*
* @return {Promise} the HTTP Promise for the given.
*
* @property {number} totalPendingRequests total amount of pending template requests being downloaded.
*/
function $TemplateRequestProvider() {
this.$get = ['$templateCache', '$http', '$q', function($templateCache, $http, $q) {
function handleRequestFn(tpl, ignoreRequestError) {
var self = handleRequestFn;
self.totalPendingRequests++;

return $http.get(tpl, { cache : $templateCache })
.then(function(response) {
var html = response.data;
if(!html || html.length === 0) {

This comment has been minimized.

Copy link
@caitp

caitp Aug 28, 2014

Contributor

why do we care?

This comment has been minimized.

Copy link
@HeberLZ

HeberLZ Sep 3, 2014

Contributor

because otherwise empty responses would be getting cached

This comment has been minimized.

Copy link
@zygimantus

zygimantus May 3, 2017

So how to disable the cache using templateRequest?

return handleError();
}

self.totalPendingRequests--;
$templateCache.put(tpl, html);
return html;
}, handleError);

function handleError() {
self.totalPendingRequests--;
if (!ignoreRequestError) {
throw $compileMinErr('tpload', 'Failed to load template: {0}', tpl);
}
return $q.reject();
}
}

handleRequestFn.totalPendingRequests = 0;

return handleRequestFn;
}];
}
8 changes: 4 additions & 4 deletions src/ngMessages/messages.js
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,8 @@ angular.module('ngMessages', [])
* </file>
* </example>
*/
.directive('ngMessages', ['$compile', '$animate', '$http', '$templateCache',
function($compile, $animate, $http, $templateCache) {
.directive('ngMessages', ['$compile', '$animate', '$templateRequest',
function($compile, $animate, $templateRequest) {
var ACTIVE_CLASS = 'ng-active';
var INACTIVE_CLASS = 'ng-inactive';

Expand Down Expand Up @@ -296,8 +296,8 @@ angular.module('ngMessages', [])

var tpl = $attrs.ngMessagesInclude || $attrs.include;
if(tpl) {
$http.get(tpl, { cache: $templateCache })
.success(function processTemplate(html) {
$templateRequest(tpl)
.then(function processTemplate(html) {
var after, container = angular.element('<div/>').html(html);
angular.forEach(container.children(), function(elm) {
elm = angular.element(elm);
Expand Down
8 changes: 3 additions & 5 deletions src/ngRoute/route.js
Original file line number Diff line number Diff line change
Expand Up @@ -226,10 +226,9 @@ function $RouteProvider(){
'$routeParams',
'$q',
'$injector',
'$http',
'$templateCache',
'$templateRequest',
'$sce',
function($rootScope, $location, $routeParams, $q, $injector, $http, $templateCache, $sce) {
function($rootScope, $location, $routeParams, $q, $injector, $templateRequest, $sce) {

/**
* @ngdoc service
Expand Down Expand Up @@ -556,8 +555,7 @@ function $RouteProvider(){
templateUrl = $sce.getTrustedResourceUrl(templateUrl);
if (angular.isDefined(templateUrl)) {
next.loadedTemplateUrl = templateUrl;
template = $http.get(templateUrl, {cache: $templateCache}).
then(function(response) { return response.data; });
template = $templateRequest(templateUrl);
}
}
if (angular.isDefined(template)) {
Expand Down
1 change: 1 addition & 0 deletions test/ng/directive/ngIncludeSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ describe('ngInclude', function() {
$rootScope.url = 'url2';
$rootScope.$digest();
$httpBackend.flush();

expect($rootScope.$$childHead).toBeFalsy();
expect(element.text()).toBe('');

Expand Down
89 changes: 89 additions & 0 deletions test/ng/templateRequestSpec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
'use strict';

describe('$templateRequest', function() {

it('should download the provided template file',
inject(function($rootScope, $templateRequest, $httpBackend) {

$httpBackend.expectGET('tpl.html').respond('<div>abc</div>');

var content;
$templateRequest('tpl.html').then(function(html) { content = html; });

$rootScope.$digest();
$httpBackend.flush();

expect(content).toBe('<div>abc</div>');
}));

it('should cache the request using $templateCache to prevent extra downloads',
inject(function($rootScope, $templateRequest, $templateCache) {

$templateCache.put('tpl.html', 'matias');

var content;
$templateRequest('tpl.html').then(function(html) { content = html; });

$rootScope.$digest();
expect(content).toBe('matias');
}));

it('should throw an error when the template is not found',
inject(function($rootScope, $templateRequest, $httpBackend) {

$httpBackend.expectGET('tpl.html').respond(404);

$templateRequest('tpl.html');

$rootScope.$digest();

expect(function() {
$rootScope.$digest();
$httpBackend.flush();
}).toThrowMinErr('$compile', 'tpload', 'Failed to load template: tpl.html');
}));

it('should throw an error when the template is empty',
inject(function($rootScope, $templateRequest, $httpBackend) {

$httpBackend.expectGET('tpl.html').respond('');

$templateRequest('tpl.html');

$rootScope.$digest();

expect(function() {
$rootScope.$digest();
$httpBackend.flush();
}).toThrowMinErr('$compile', 'tpload', 'Failed to load template: tpl.html');
}));

it('should keep track of how many requests are going on',
inject(function($rootScope, $templateRequest, $httpBackend) {

$httpBackend.expectGET('a.html').respond('a');
$httpBackend.expectGET('b.html').respond('c');
$templateRequest('a.html');
$templateRequest('b.html');

expect($templateRequest.totalPendingRequests).toBe(2);

$rootScope.$digest();
$httpBackend.flush();

expect($templateRequest.totalPendingRequests).toBe(0);

$httpBackend.expectGET('c.html').respond(404);
$templateRequest('c.html');

expect($templateRequest.totalPendingRequests).toBe(1);
$rootScope.$digest();

try {
$httpBackend.flush();
} catch(e) {}

expect($templateRequest.totalPendingRequests).toBe(0);
}));

});
11 changes: 6 additions & 5 deletions test/ngRoute/directive/ngViewSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ describe('ngView', function() {
});


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

expect(controllerScope.$parent).toBe($rootScope);
expect(controllerScope).toBe($route.current.scope);
expect(log).toEqual(['ctrl-init']);
expect(function() {
$rootScope.$digest();
}).toThrowMinErr('$compile', 'tpload', 'Failed to load template: /tpl.html');

expect(controllerScope).toBeUndefined();
});
});

Expand Down
35 changes: 18 additions & 17 deletions test/ngRoute/routeSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -671,35 +671,36 @@ describe('$route', function() {
});


it('should drop in progress route change when new route change occurs and old fails', function() {
module(function($routeProvider) {
it('should throw an error when a template is empty or not found', function() {
module(function($routeProvider, $exceptionHandlerProvider) {
$exceptionHandlerProvider.mode('log');
$routeProvider.
when('/r1', { templateUrl: 'r1.html' }).
when('/r2', { templateUrl: 'r2.html' });
when('/r2', { templateUrl: 'r2.html' }).
when('/r3', { templateUrl: 'r3.html' });
});

inject(function($route, $httpBackend, $location, $rootScope) {
var log = '';
$rootScope.$on('$routeChangeError', function(e, next, last, error) {
log += '$failed(' + next.templateUrl + ', ' + error.status + ');';
});
$rootScope.$on('$routeChangeStart', function(e, next) { log += '$before(' + next.templateUrl + ');'; });
$rootScope.$on('$routeChangeSuccess', function(e, next) { log += '$after(' + next.templateUrl + ');'; });

inject(function($route, $httpBackend, $location, $rootScope, $exceptionHandler) {
$httpBackend.expectGET('r1.html').respond(404, 'R1');
$httpBackend.expectGET('r2.html').respond('R2');

$location.path('/r1');
$rootScope.$digest();
expect(log).toBe('$before(r1.html);');

$httpBackend.flush();
expect($exceptionHandler.errors.pop().message).toContain("[$compile:tpload] Failed to load template: r1.html");

$httpBackend.expectGET('r2.html').respond('');
$location.path('/r2');
$rootScope.$digest();
expect(log).toBe('$before(r1.html);$before(r2.html);');

$httpBackend.flush();
expect(log).toBe('$before(r1.html);$before(r2.html);$after(r2.html);');
expect(log).not.toContain('$after(r1.html);');
expect($exceptionHandler.errors.pop().message).toContain("[$compile:tpload] Failed to load template: r2.html");

$httpBackend.expectGET('r3.html').respond('abc');
$location.path('/r3');
$rootScope.$digest();

$httpBackend.flush();
expect($exceptionHandler.errors.length).toBe(0);
});
});

Expand Down

0 comments on commit a70e283

Please sign in to comment.