patternjavascriptMinor
Lazily Load CSS and JS
Viewed 0 times
loadandcsslazily
Problem
I have written this piece of JS and CSS loading code and I would like some advice on it. Anything some of the JavaScript gurus could possibly point out would be much appreciated. The code works, but I have not done extensive testing, because I am concerned about replacing functions in this manner.
A single JavaScript file containing JQuery as well as the below code will be included on all the pages. We write all the components in house (which means no Require JS) and keep them very modular separated into their own folder with the corresponding JS and CSS. You can imagine starting to use for instance a dropown, dialog and a datepicker on one page would require us to add 6 includes and this quite frankly is annoying, because I want the dependencies to resolve automatically and using JSP includes could possibly make multiple calls to the same resources.
Below is the source to load a single datepicker lazily:
```
;(function($){
//All Lazily loaded components go here
$.fn.datepicker = function(settings){
console.log("This should only be displayed once");
loadCSS("/res/component/datepicker/datepicker.css");
var elem = this;
return loadJS("/res/component/datepicker/datepicker.js",
function(){return elem.datepicker(settings)});//After Load Completion the $.fn.datepicker is replaced
//by the proper working implementation, execute it and return it so we maintain the chain
};
}(jQuery));
function loadCSS(absoluteUrl){
if(loadCSS[absoluteUrl])
return;//Css already loaded
$('')
.appendTo('head')
.attr({type : 'text/css', rel : 'stylesheet'})
.attr('href', absoluteUrl);//Appending entire element doesn't load in IE, but setting the href in this manner does
loadCSS[absoluteUrl] = true;//Memoize
}
function loadJS(absoluteUrl, onComplete){
if(loadJS[absoluteUrl])
return;//Script already loaded
A single JavaScript file containing JQuery as well as the below code will be included on all the pages. We write all the components in house (which means no Require JS) and keep them very modular separated into their own folder with the corresponding JS and CSS. You can imagine starting to use for instance a dropown, dialog and a datepicker on one page would require us to add 6 includes and this quite frankly is annoying, because I want the dependencies to resolve automatically and using JSP includes could possibly make multiple calls to the same resources.
Below is the source to load a single datepicker lazily:
```
;(function($){
//All Lazily loaded components go here
$.fn.datepicker = function(settings){
console.log("This should only be displayed once");
loadCSS("/res/component/datepicker/datepicker.css");
var elem = this;
return loadJS("/res/component/datepicker/datepicker.js",
function(){return elem.datepicker(settings)});//After Load Completion the $.fn.datepicker is replaced
//by the proper working implementation, execute it and return it so we maintain the chain
};
}(jQuery));
function loadCSS(absoluteUrl){
if(loadCSS[absoluteUrl])
return;//Css already loaded
$('')
.appendTo('head')
.attr({type : 'text/css', rel : 'stylesheet'})
.attr('href', absoluteUrl);//Appending entire element doesn't load in IE, but setting the href in this manner does
loadCSS[absoluteUrl] = true;//Memoize
}
function loadJS(absoluteUrl, onComplete){
if(loadJS[absoluteUrl])
return;//Script already loaded
Solution
No JavaScript guru, but I do have some thoughts on this.
As you yourself have point out in comments, this will be a problem when
As
even before the URL is really loaded,
subsequent calls to the method will return early.
By moving the
somewhere after the
the problem will be mitigated.
In that case,
the worst thing that can happen is that the URL will be loaded twice,
which is not as bad as the rest of the code happily executing thinking that initialization has successfully completed when in fact it hasn't yet.
Update: ok maybe that's not the worst thing that can happen (as you pointed out in comments).
For example, if the same JS file is loaded by multiple
the functions in the JS file will get overwritten.
I don't know how that works, and what happens if the function is being overwritten while it's also being executed at the same time.
To avoid such issues,
you would need to implement locking in
It seems a bad idea to use the same name for objects and functions,
for
-
Can be confusing and lead to nasty bugs (see more on that in my update)
-
-
Since CSS and JS URLs are distinct, a single cache will be enough
So how about renaming the cache, and sharing between CSS and JS:
Another thing,
I think you should make the behavior consistent: either return
Update
As you asked, a bit more info why not to use the same name for different objects.
As a general principle, programs should be as clear as possible.
Using the same name for different objects is really just asking for trouble,
for no apparent benefit, at least in this example.
The handling of and behavior of this feature in JavaScript can be confusing,
and may also be browser-dependent.
Take a look at the example snippets in this other answer.
Predicting the output of these examples is far from trivial:
Having such elements in the code that force me to think hard would be a really unnecessary obstacle.
Even though in your code there is no such confusion,
that might creep in later as you (or somebody else) add more features later.
It also sets a bad example that others might follow.
Unless there is a really good reason to do this,
and real advantages,
don't call different things by the same name,
don't make me think,
keep things perfectly clear.
As you yourself have point out in comments, this will be a problem when
loadJS is called with the same URL a 2nd time before the 1st call has completed:function loadJS(absoluteUrl, onComplete){
if(loadJS[absoluteUrl])
return;//Script already loaded
loadJS[absoluteUrl] = true;//Memoize
// ...As
loadJS[absoluteUrl] gets set to true immediately,even before the URL is really loaded,
subsequent calls to the method will return early.
By moving the
loadJS[absoluteUrl] = true; line further down in the method,somewhere after the
onComplete() call,the problem will be mitigated.
In that case,
the worst thing that can happen is that the URL will be loaded twice,
which is not as bad as the rest of the code happily executing thinking that initialization has successfully completed when in fact it hasn't yet.
Update: ok maybe that's not the worst thing that can happen (as you pointed out in comments).
For example, if the same JS file is loaded by multiple
loadJS calls in parallel,the functions in the JS file will get overwritten.
I don't know how that works, and what happens if the function is being overwritten while it's also being executed at the same time.
To avoid such issues,
you would need to implement locking in
loadJS to prevent subsequent calls while a URL is being loaded.It seems a bad idea to use the same name for objects and functions,
for
loadCSS and loadJS, for many reasons:-
Can be confusing and lead to nasty bugs (see more on that in my update)
-
loadCSS and loadJS are not good names for objects that act essentially as a cache of url -> flag, indicating whether a URL was already loaded or not-
Since CSS and JS URLs are distinct, a single cache will be enough
So how about renaming the cache, and sharing between CSS and JS:
function loadCSS(absoluteUrl) {
if (alreadyLoadedUrls[absoluteUrl])
return; // Css already loaded
// ...
}
function loadJS(absoluteUrl, onComplete) {
if (alreadyLoadedUrls[absoluteUrl])
return; // Script already loaded
// ...
}Another thing,
loadJS returns different things when the URL was already loaded or not:- If already loaded, returns
null
- If called for the first time, it returns
onComplete()
I think you should make the behavior consistent: either return
null always, or onComplete() always.Update
As you asked, a bit more info why not to use the same name for different objects.
As a general principle, programs should be as clear as possible.
Using the same name for different objects is really just asking for trouble,
for no apparent benefit, at least in this example.
The handling of and behavior of this feature in JavaScript can be confusing,
and may also be browser-dependent.
Take a look at the example snippets in this other answer.
Predicting the output of these examples is far from trivial:
(function (foo) {
var foo; // doesn't affect the existing `foo` identifier
return typeof foo;
function foo () {}
})('string'); // yields "function"
(function (foo) {
var foo = {};
return typeof foo;
function foo () {}
})('string'); // yields "object"!!Having such elements in the code that force me to think hard would be a really unnecessary obstacle.
Even though in your code there is no such confusion,
that might creep in later as you (or somebody else) add more features later.
It also sets a bad example that others might follow.
Unless there is a really good reason to do this,
and real advantages,
don't call different things by the same name,
don't make me think,
keep things perfectly clear.
Code Snippets
function loadJS(absoluteUrl, onComplete){
if(loadJS[absoluteUrl])
return;//Script already loaded
loadJS[absoluteUrl] = true;//Memoize
// ...function loadCSS(absoluteUrl) {
if (alreadyLoadedUrls[absoluteUrl])
return; // Css already loaded
// ...
}
function loadJS(absoluteUrl, onComplete) {
if (alreadyLoadedUrls[absoluteUrl])
return; // Script already loaded
// ...
}(function (foo) {
var foo; // doesn't affect the existing `foo` identifier
return typeof foo;
function foo () {}
})('string'); // yields "function"
(function (foo) {
var foo = {};
return typeof foo;
function foo () {}
})('string'); // yields "object"!!Context
StackExchange Code Review Q#62806, answer score: 3
Revisions (0)
No revisions yet.