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

Treat plain JS as undefined-returning AMD modules and don't be grouchy #164

Closed dvdotsenko closed 11 years ago

dvdotsenko commented 11 years ago

Example:

require(['plain/js/file'], function(){})

as an alternative to

require(['js!plain/js/file.js'], function(){})

From practical stand point, there is no difference between an AMD module with the following define:

define(function(){return undefined})

and a plain JS file loaded over "js!plain/jsfile.js"

Both effectively load only once, are cached and return undefined

However, CurlJS complains about plain JS files loaded using regular module ID references require(['path/to/plain/jsfile'],function(){/*do something*/}) with "define() missing" error message.

There is a good argument for allowing treatment of plain JS files as if they were undefined-returning AMD modules:

unscriptable commented 11 years ago

Hey Daniel!

The negative: parse errors or 404s won't get caught in IE 6-8 and Opera (modules or plain js).

RequireJS handles this situation via a timeout (setTimeout), which is impossible to configure correctly for all users. If the timeout set to something very high, such as 60 seconds, there's little risk that the timeout could trigger too early. However, if the url 404s, the user will give up before she/he sees an error.

My conclusion (2 years ago) was that a timeout was not an acceptable solution. Failing silently in old IE and Opera is not acceptable imho, either. This may be worse! It suceeds with a bogus value.

Thoughts?

-- John

dvdotsenko commented 11 years ago

Excellent point about the old browsers. It's unfortunate 2 years since and you still have to deal with old IE, old Opera quirks. Grr...

I see that James put in place a switch for enforcing presence of at least one define. I don't see a "switch" as a best solution to this, as it just breeds more confusion. ( I don't see switches or settings of any kind as best solutions :) )

dvdotsenko commented 11 years ago

Reason this conversation started:

In presence of the following config:

paths: {
    libs: 'stuff'
}

Reference to this resource:

'js!libs/plain_old.js' // on CurlJS or RequireJS with js plugin https://groups.google.com/d/msg/requirejs/_kjtTyRvKe8/o_h6Vh_kEIQJ

or

'libs/plain_old.js' // (on RequireJS because "js!" is autodetected)

Is resolved to baseURL + "libs/plain_old.js"' on RequireJS and tobaseURL + "stuff/plain_old.js"' on CurlJS.

In the mean time, RequireJS's "libs/plain_old" worked seemingly exactly like "js!libs/plain_old.js" on CurlJS...

Seeing that James makes the rules. I am sure he will have an explanation for the different ways of resolving the module IDs against the paths. Since I did not find RequireJS's way to resolve "blah.js" paths to be desirable, I was looking at his other way of loading plain JS - using module ID-like strings - as a good common denominator behavior between CurlJS and RequireJS.

unscriptable commented 11 years ago

What do you think about this instead? https://github.com/cujojs/curl/blob/auto-define/src/curl.js#L941-L949

1) succeeds if the id ends in ".js" and the file didn't call define() 2) fails if the id ends in ".js" and the file called define() 3) catches 404 and parse errors except in IE6-8 and Opera (devs can use js! plugin if they want to detect those)

-- J

dvdotsenko commented 11 years ago

I took a look at proposed auto-define patch. I have added a commit on top of it with tests demonstrating why I am not a fan of the way RequireJS implemented handling of resources with ".js" ending.

https://github.com/dvdotsenko/curl/tree/auto-define

In short, RequireJS does not subject the resource ID to resolution through paths etc. = Broken in my book.

My end goal was NOT to "make CurlJS like RequireJS." I am very happy with how CurlJS resolves the paths for js!resourse.js resources. I am NOT happy with how RequireJS resolves [js!]resourse.js.

My end goal was to find a common behavior between CurlJS and RequireJS that I actually like. Both CurlJS and RequireJS resolve ".js"-LESS resources exactly the same way. Hence my focus was specifically on avoiding the use of ".js" endings and just stretching the acceptance of these define-less "modules" to CurlJS.

So, while your auto-define patch technically works and feels like what RequireJS does, it does not address the actual issue and does not actually do what RequireJS does.

Besides, the magic behavior of treating ".js" ending resources is just plain wrong. What if I want to load "blah.js.js" file? (I know it's contrived, but still...) I am very happy you stayed away from these magic gimmics.

Let's do this: 1 Accept that the CurlJS's present handling of plain JS resource resolution is more correct that RequireJS's 2 Lobby James to change RequireJS to include in some future major version paths etc into resolution of plain JS resources requested with ".js" ending and then contemplate adding magic handling of ".js$" files in CurlJS 3 In year or two if by some stroke of magic the share of IE6-8 falls to some abysmal, unimportant share where you will not need to rely on lack of define call as indicator of error, you may finally add auto-def for all resources that load with module-like IDs (lacking ".js" endings)

In other words, seeing that ## 2 and 3 are unlikely to happen, doing nothing in this regard is probably the best course of action for now. :)

dvdotsenko commented 11 years ago

I pitched an alternative approach to James.

https://groups.google.com/d/msg/requirejs/_kjtTyRvKe8/C2V276eYW5oJ

Instead of stretching CurlJS to common handling of plain JS files, it seems it's much easier to stretch RequireJS to desired handling of plain JS resource IDs by bundling a 56-byte "js!" plugin. I hope this happens and there will be a "cross-AMD-loader-compatible" way to load (resolve) plain JS resources.