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

'on' method missing on require for full dojo compatibility #158

Closed qvuilliot closed 10 years ago

qvuilliot commented 11 years ago

Just to let you know that I encountered an issue on a dojo/dijit project (using trunk versions) when trying to use curl as the loader. In dojo/ready.js, require.on() is called, what caused on error since this method does not exist with curl.

So I added a fake 'on' method on require. Not sure whether this is the best way to tackle this issue, but it works for me. This is more a Dojo issue I suppose, not being compatible enough with AMD.

unscriptable commented 11 years ago

Hey Quentin!

Thanks! I'll take a peek at your PR after the thanksgiving holiday. Do you know what the on function does? If we can shim it to behave correctly that'd be best.

-- John

Sent from planet earth

On Nov 20, 2012, at 12:53 PM, Quentin notifications@github.com wrote:

Just to let you know that I encountered an issue on a dojo/dijit project (using trunk versions) when trying to use curl as the loader. In dojo/ready.js, require.on() is called, what caused on error since this method does not exist with curl.

So I added a fake 'on' method on require. Not sure whether this is the best way to tackle this issue, but it works for me.

This is more a Dojo issue I suppose, not being compatible enough with AMD.

You can merge this Pull Request by running:

git pull https://github.com/qvuilliot/curl master

Or view, comment on, or merge it at:

https://github.com/cujojs/curl/pull/158 Commit Summary

File Changes

Patch Links

qvuilliot commented 11 years ago

From what I have seen, the 'on' method is part of dojo loader's "micro event API" (http://livedocs.dojotoolkit.org/loader/amd#the-micro-event-api). I couldn't find any file in dojo depending on it except dojo/ready.js, which is marked as deprecated for Dojo 2.0. So it might not be worth spending more time on this... It's your call :)

unscriptable commented 11 years ago

Hey @qvuilliot, seems like we can close this now that dojo 1.8.2 has removed require.on()? -- J

qvuilliot commented 11 years ago

Did dojo removed require.on() ?? From what I can see, it's still there in 1.8.2: https://github.com/dojo/dojo/blob/1.8.2/ready.js#L48

I tried anyway with a trunk dojo and original curl.js today, and I still get the same error "require.on undefined"...

unscriptable commented 10 years ago

Thanks @qvuilliot! You were right that we could just stub this function. This will be fixed by the new dojo18 shim in 0.7.6!