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

curl throws error if global "require" object already exists #121

Closed tmaslen closed 11 years ago

tmaslen commented 11 years ago

Hi,

I appreciate that it is a feature of curl to error if "require" is already available but we have an edge case for when this would be okay.

We load curl asynchronously, but we have other teams that can only include their AMD modules directly into the body of the page. To counter this I've made a small bit of code that collects calls to require and define. In the onload function that fires when curl is added into the page, these collected require and define calls are refired and get picked up by curl.

This used to work with a previous version of curl. Is this a viable usage case to have an option that stops curl from erroring if there is already a global require?

Thanks as always, Tom.

Sample code:

// before curl is loaded into the page
(function(global){
    global.globalAmdCalls = [];
    global.define = function() {
        var args = arguments;
        globalAmdCalls.push(function() {
            define.apply(global, args);
        });
    };
    global.require = function() {
        var args = arguments;
        globalAmdCalls.push(function() {
            require.apply(global, args);
        });
    };
})(window);

// in curl's asynchronously loaded script onload callback
for(var q=0, len = window.globalAmdCalls.length; q < len; q++) {
    globalAmdCalls[q]();
}
unscriptable commented 11 years ago

Heh. I put that "feature" in there because it seemed like the right thing to do! :) Lately, lots of ppl have been embedding curl in their widgets as a micro-loader. I figured it was better to throw than to clobber. But I guess curl could silently fail or attempt to log a message? Hm. Logging a message probably isn't what you want either, huh?

Maybe I'll just make it fail silently and add loud failure to the curl/debug (which I've been meaning to update big time).

tmaslen commented 11 years ago

It's definitely an edge case, so I wouldn't mind giving curl a command to not fail. Though the silent fail would work for me too.

unscriptable commented 11 years ago

I think it makes sense if curl silently allows overwriting an existing function if the dev namespaces the curl api (curl()) or AMD api (define()) via config options: apiContext or defineContext. I would hope that the dev knows what they're doing if they specify either of those. This seems like the friendliest way to do it.

This means you guys could just specify apiContext: window to stop the exception, but that seems klunky. Therefore, I'm adding an overwriteApi: true option, too.

Sound good?

unscriptable commented 11 years ago

I take that back. overwriteApi: true is always required. :) This is fixed in the dev branch. Feel free to try it.

tmaslen commented 11 years ago

Thanks John. Will take a look.

tmaslen commented 11 years ago

Hey John, it only took me 3 months but I got round to taking a look at this and it all worked really well. Thanks.

unscriptable commented 11 years ago

Awesome. did you try it with the new 0.7.3 code? Or 0.7.2?

tmaslen commented 11 years ago

0.7.2. I just pulled down what was in master (I think).