Hi Eric, this is a very nice piece of code. We did have a couple issues, however, that we quickly patched. Might not be the ideal fixes, but they work for our purposes.
Safari 3 was throwing several errors trying the load an "undefined" script. I altered the createScriptEl function to check the url var before execution of the body. This issue also caused Chrome to crash. It iss possible that there is something in my code passing undefined to .load function, but so far I haven't seen any errors in IE or Firefox that validates any consistency.
function createScriptEl(url, srcSetObj, iteration) { if (url != "undefined") {....
Added a path property to the tag object to allow easy setting of a path variable without having to manually append it to the name property or depend on just 'http://' in the name string.
In .getSrcToLoad prototype.... if (tagName.indexOf("http://") > -1) { var filePath = tagName; } else if (tag.path) { var filePath = tag.path + tagName + '.js'; } else { var filePath = (path ? path : "") + tagName + '.js'; }
Regards,
Patrick Dillon Integrify
- jsload.js 9.24KB
Comment #1
Posted on Jan 26, 2009 by Happy GiraffeHello, Patrick. Thanks for your patches, and for writing up your bugs! I'm going to keep this code maintained, and your input really helps.
The second suggestion has been implemented in the repository. As for the first bug, I was unable to reproduce it. I tried passing undefined vars in as the tagName, but couldn't get the same error as what you're describing. Do you have a live example or an example file you could send me to help me debug?
Thanks, Eric
Comment #2
Posted on Apr 2, 2009 by Massive BearDear Eric,
First of all, thank you very much for such an excellent and useful piece of code. This does everything we were previously using YUI loader for -- so much more elegantly.
I've also run into the issue that Patrick describes in number 1. If you want a test case, check out your own http://ericnguyen.com/jsload/test/index.html .. and look at the once loaded. JSLoad is pulling in "undefined" javascript source files.
Attached is a stupidly simple patch that actually fixes the issue (for me, at least) rather than simply works around it. All tests still pass.
Regards, Nick Stenning, currently with Open Knowledge Foundation [http://okfn.org] hat on.
Comment #3
Posted on Apr 3, 2009 by Grumpy CatEric, sorry I missed your previous comment and never responded. Nick, this fix looks
good to me. I noticed that I was incorrect before and it was happening in all
browsers so I expanded my orignal url filter in the createScriptEl function from
if (url != "undefined") {....
to
if (url.indexOf("undefined") == -1) {...
instead of getting into the code. Makes sense now how it was getting "undefined" from the next line without the return statement you added. Good catch.
Thanks, Pat
Comment #4
Posted on Oct 8, 2009 by Happy GiraffeThanks for your comments and patches, guys, and apologies for the long delay in keeping the repository up to date. I'm cleaning house, finally, and the fix is in, as of r11, with some additional cleanup.
Nick, I also folded in many of the JSLint edits you made in your Github repository (http://github.com/nickstenning/jsload/commit/5d6e91139c3496a19d98e130f3d4582e38ba2f82). The dependency on Prototype/jQuery is something that I plan to make a cleaner, so that change is upcoming, along with a few other improvements.
Status: Fixed
Labels:
Type-Defect
Priority-Medium