cujojs / curl

curl.js is small, fast, extensible module loader that handles AMD, CommonJS Modules/1.1, CSS, HTML/text, and legacy scripts.
https://github.com/cujojs/curl/wiki
Other
1.89k stars 216 forks source link

Resolution callback being executed before dependency is actually loaded #119

Closed ghost closed 11 years ago

ghost commented 11 years ago

I don't have a simple test case boiled down for this, but what I'm seeing is that I end up here:

https://github.com/cujojs/curl/blob/master/src/curl.js#L1006

at which point deps has a length of 1 (expected) and that one "dep" is undefined (unexpected). Looking at the Network inspector in Safari, I see that the appropriate dependency was indeed requested from the server, but it is still in a pending state, so the code in my completion routine that expects that file to have loaded and run dies a miserable death because it hasn't actually shown up yet.

I'm not sure what causes this to happen, as plenty of other loads happen successfully in the same application, but the loads that fail this way always fail this way.

The file does eventually show up and all is well from that point forward, but my code has already crashed.

I'm using the js! plugin and the !order suffix if that is relevant.

ghost commented 11 years ago

After some further digging, what seems to be triggering the problem is multiple attempts to load the same file hitting curl in rapid succession. I haven't spent enough time tracing through the js! plugin to determine exactly how it handles this, but it seems to respond by fulfilling promises prematurely.

unscriptable commented 11 years ago

Interesting. I wonder if this is an issue using cjsm-wrapped modules with plugins. iirc, you are using the cjs exports object to declare modules?

ghost commented 11 years ago

My "modules" are just the standard patterns IcedCoffeeScript spits out. A typical file will look like:

(function() {
    ...
}).call(this);

I'm not using AMD for anything at all as I have my own lazy dependency management mechanisms orchestrated by a custom build system.

unscriptable commented 11 years ago

If you paste a code snippet of the curl configuration and loading, I may be able to spot something. The more code the better. A gist would be awesome, as well. :)

ghost commented 11 years ago

Run this and you'll get 99 undefineds logged by _loadSucceeded and not one failure.

stubs.js:

window.Bug = Date.now();

index.html:

<!DOCTYPE html>
<html>
<head>
    <title>Curl's Broken</title>

    <script type="text/javascript" src="/js/curl.js/dist/curl-for-jQuery/curl.js"> </script>

    <script type="text/javascript">

        (function() 
        {
            var _loadSucceeded = function ()
            {
                console.log(window.Bug);
            }

            var _loadFailed = function ()
            {
                console.error("Didn't load");
            }

            for (var i = 0; i < 100; i++)
            {
                curl({}, ["js!/stubs.js!order"]).then(_loadSucceeded, _loadFailed);
            }

        })();

    </script>

</head>

<body>
</body>
</html>
unscriptable commented 11 years ago

Thanks @khiltd!

I found the issue. It happens when the same file is loaded more than once (like you suggested).

It's fixed in the dev branch. I see you're using a "dist" version so I re-generated these. If you want to try it out, checkout the dev branch and grab the curl-for-jQuery/curl.js file.

Regards,

-- John

ghost commented 11 years ago

Cool; thanks. I wrote my own police code to prevent the situation from ever happening since it was totally avoidable on my end, but seemed like it was undesirable behavior. I'll give the new one a shot in the morning.

ghost commented 11 years ago

Looks like it's doing the right thing now. Thanks!