patternjavascriptMinor
Dynamically load assets (CSS+images+controllers) on route resolve
Viewed 0 times
routedynamicallycsscontrollersresolveassetsloadimages
Problem
I wrote a little factory to dynamically load assets in angular resolve. Is it well structured and can it be made prettier?
```
app.factory('loadAssets', function ($q) {
var jsPath = "scripts/",
cssPath = "css/",
imagesPath = "images/",
head = document.getElementsByTagName("head")[0]; // define starting paths for each file tile
return {
startLoad: function (url) {
var fileType = url.split(".");
fileType = fileType[fileType.length - 1]; // we must get (length - 1) because filenames like jquery.min.js
/* prevent duplicate loading -
check if url exist in already loaded assets
1. If asset exist, return true and quit.
2. If asset doesn't exist - add it.
3. If loadedAssets is undefined (first time you load something) - create this array.
*/
if (this.loadedAssets == undefined || this.loadedAssets == null) {
this.loadedAssets = [];
}
if (this.loadedAssets.indexOf(url) >= 0) { // note that indexOf supported only in ie9+ , add fallback if you want to support legacy browsers.
return true;
} else {
this.loadedAssets.push.apply(this.loadedAssets, [url]);
// load js files
if (fileType == "js") {
var jsFile = document.createElement("script");
jsFile.src = jsPath + url;
head.appendChild(jsFile);
var waitforload = $q.defer();
jsFile.onload = function () {
waitforload.resolve(jsFile);
};
jsFile.onerror = function (e) {
waitforload.reject(e);
console.log("Could not load " + jsFile.src
```
app.factory('loadAssets', function ($q) {
var jsPath = "scripts/",
cssPath = "css/",
imagesPath = "images/",
head = document.getElementsByTagName("head")[0]; // define starting paths for each file tile
return {
startLoad: function (url) {
var fileType = url.split(".");
fileType = fileType[fileType.length - 1]; // we must get (length - 1) because filenames like jquery.min.js
/* prevent duplicate loading -
check if url exist in already loaded assets
1. If asset exist, return true and quit.
2. If asset doesn't exist - add it.
3. If loadedAssets is undefined (first time you load something) - create this array.
*/
if (this.loadedAssets == undefined || this.loadedAssets == null) {
this.loadedAssets = [];
}
if (this.loadedAssets.indexOf(url) >= 0) { // note that indexOf supported only in ie9+ , add fallback if you want to support legacy browsers.
return true;
} else {
this.loadedAssets.push.apply(this.loadedAssets, [url]);
// load js files
if (fileType == "js") {
var jsFile = document.createElement("script");
jsFile.src = jsPath + url;
head.appendChild(jsFile);
var waitforload = $q.defer();
jsFile.onload = function () {
waitforload.resolve(jsFile);
};
jsFile.onerror = function (e) {
waitforload.reject(e);
console.log("Could not load " + jsFile.src
Solution
From a once over:
-
This
could be
-
This
could be
this will assign
-
This
should be
The comment is on top which reads easier, but more importantly there is no
-
I would encode all the image extensions from
All in all, I like the code.
`
-
This
var fileType = url.split(".");
fileType = fileType[fileType.length - 1]; // we must get (length - 1) because filenames like jquery.min.jscould be
//Get the last . separated string
var fileType = url.split(".").slice(-1);.slice(-1) retrieves the last element in an array-
This
if (this.loadedAssets == undefined || this.loadedAssets == null) {
this.loadedAssets = [];
}could be
this.loadedAssets == this.loadedAssets || [];this will assign
[] if this.loadedAssets evaluates to false (by being null or undefined)-
This
if (this.loadedAssets.indexOf(url) >= 0) { // note that indexOf supported only in ie9+ , add fallback if you want to support legacy browsers.
return true;
} else {should be
// indexOf suppor only in ie9+ , add fallback if you want to support legacy browsers.
if (this.loadedAssets.indexOf(url) >= 0) {
return true;
}The comment is on top which reads easier, but more importantly there is no
else. The else was superfluous since you exit the function anyway if the url was already loaded-
I would encode all the image extensions from
if (fileType == "jpg" || fileType == "jpeg" || fileType == "png" || fileType == "gif") { into an array and use indexOf.All in all, I like the code.
`
Code Snippets
var fileType = url.split(".");
fileType = fileType[fileType.length - 1]; // we must get (length - 1) because filenames like jquery.min.js//Get the last . separated string
var fileType = url.split(".").slice(-1);if (this.loadedAssets == undefined || this.loadedAssets == null) {
this.loadedAssets = [];
}this.loadedAssets == this.loadedAssets || [];if (this.loadedAssets.indexOf(url) >= 0) { // note that indexOf supported only in ie9+ , add fallback if you want to support legacy browsers.
return true;
} else {Context
StackExchange Code Review Q#68251, answer score: 2
Revisions (0)
No revisions yet.