SlexAxton / yepnope.js

A Script Loader For Your Conditional Builds
http://yepnopejs.com/
BSD 3-Clause "New" or "Revised" License
2.51k stars 323 forks source link

Imbricked calls #13

Closed itechnology closed 13 years ago

itechnology commented 13 years ago

Not sure if it's me that's doing something wrong but here goes:

// somepage
console.log("loading file1");
yepnope([{ load: 'file1.js', callback: function () {
        console.log("loaded file1");
    }
}]);

// file1.js
console.log("loading file2");
yepnope([{ load: 'file2.js', callback: function () {
        console.log("loaded file2");
    }
}]);

// file2.js
console.log("finished");

Result: loading file1 loading file2

And in firebug net panel, i see that both files have correctly been loaded, however the callbacks and console.log("finished") does not seem to fire.

SlexAxton commented 13 years ago

Is there any way you can put up an example of this happening on a live webpage?

We have a test suite that does this about 50 times, and it's passing in every browser that we support, as far as I can tell, but perhaps you're doing something that I haven't thought of. It'd be hard for me to tell without seeing it though, so if you could get me access, that'd be awesome.

Thanks!

itechnology commented 13 years ago

Using yepnope 0.2.3, works fine /test/index.htm (deleted)

Using yepnope 1.0 rc2, works as described above Same behavior if using modernizr 2.0 beta with yepnope included /test/index-rc.htm (deleted)

However while putting up the sample i noticed that 0.2.3 includes the minified version of $lab inside it, where the rc and modernizr version does not, and i did not include labjs in any of my pages.

Oh, and congrats on joining the modernizr team!

ralphholzmann commented 13 years ago

This is indeed a bug in our loading logic. Great job on finding this. I've fixed this in my branch ( https://github.com/ralphholzmann/yepnope.js ) and Alex should be pulling the fix in shortly. We'll make sure to add this specific use case into our test suite. Thank you.

ralphholzmann commented 13 years ago

Test case for this specific issue has been added to the test suite with this commit:

https://github.com/ralphholzmann/yepnope.js/commit/76c6ba386d2d3364c5d6fc1dda8cee36887d98f5

SlexAxton commented 13 years ago

Moved it to master, thanks for the find! It looks like the problem occurred when you try to use yepnope inside of a file that you load with yepnope. We hadn't considered that use-case. (I actually would advise against it if you can avoid it, but if you're trying the hack i showed you in the google maps stuff, that'd be a good reason to do it :) ).

Thanks for the catch!

We moved away from LABjs in the 1.x version of yepnope so we could do some things that aren't possible (while obviously making other sacrifices) with LAB. This also helps to keep our file size down, which is priority #1 with script loaders.

Thanks for the congratz. Let me know if you run into anything else!

P.S. thanks for the fix ralph

itechnology commented 13 years ago

Actually i've been wanting to split my js into modules since quite a while now, and yesterday when i came across https://github.com/rmurphey/jquery-module

i thought i'd try something similar based on html5 data attributes, and then use yepnope as the dependency loader, where each module might require additional dependencies and load them as needed, but ran across this issue.

Google maps is different issue where we noticed that it could take ages to load, and block other js from executing on our site, which is pretty reliant on js/ajax for updating the UI. (especially for users with ie6/7/ff2: which allow only 2 same domain tcp connections)

So for now i just moved the google loader jsapi reference to the login page, like this the users cache is already primed when he hits the actual application page, and then use googles native load function if a client ever needs to access it.

Thank's for the updates, will surely be integrating them with modernizr 2 :)