$css.preload may call the callback function too early #10
Description
Hi there,
I was reviewing this promising angular module and found a small algorithm issue in the $css.preload() function.
Here's the original function code:
/**
* Preload: preloads css via http request
**/
$css.preload = function (stylesheets, callback) {
// If no stylesheets provided, then preload all
if (!stylesheets) {
stylesheets = [];
// Add all stylesheets from custom directives to array
if ($directives.length) Array.prototype.push.apply(stylesheets, $directives);
// Add all stylesheets from ngRoute to array
if ($injector.has('$route')) Array.prototype.push.apply(stylesheets, $css.getFromRoutes($injector.get('$route').routes));
// Add all stylesheets from UI Router to array
if ($injector.has('$state')) Array.prototype.push.apply(stylesheets, $css.getFromStates($injector.get('$state').get()));
}
stylesheets = filterBy(stylesheets, 'preload');
angular.forEach(stylesheets, function(stylesheet, index) {
// Preload via ajax request
$http.get(stylesheet.href)
.success(function (response) {
$log.debug('preload response: ' + response);
if (stylesheets.length === (index + 1) && angular.isFunction(callback))
callback(stylesheets);
})
.error(function (response) {
$log.error('Incorrect path for ' + stylesheet.href);
});
});
};I can see that angular.forEach(stylesheets, ... will make the program download each stylesheet by ajax, and for each stylesheet download success, we'll run the callback function only when we downloaded the last stylesheet in the list.
For me, the problem is that we assumed that stylesheets will be loaded in sequential order.
However, due to the nature of ajax requests and the unpredictable response speed of servers, this cannot be guaranteed.
So I think this part of the algorithm should take advantage of promises to make sure that the callback function is called at the end when all stylesheets have been loaded successfully.
So here's my suggested code change:
// Assuming that $q is injected in the module's JS scope
var stylesheetLoadPromises = [];
angular.forEach(stylesheets, function(stylesheet) {
stylesheetLoadPromises.push(
// Preload via ajax request
$http.get(stylesheet.href)
.success(function (response) {
$log.debug('preload response: ' + response);
if (stylesheets.length === (index + 1) && angular.isFunction(callback))
callback(stylesheets);
})
.error(function (response) {
$log.error('Incorrect path for ' + stylesheet.href);
})
);
});
if (angular.isFunction(callback)) {
$q.all(stylesheetLoadPromises).then(function(){
callback(stylesheets);
});
}Please let me know what you think.
Regards,
David