bytespider / jsOAuth

JavaScript implimentation of the OAuth protocol. Currently supports version 1.0 (RFC5849) of the specification. Node.js & CommonJS compatible.
http://bytespider.github.com/jsOAuth/
MIT License
556 stars 108 forks source link

Using requirejs breaks jsOAuth #32

Closed raoulmillais closed 12 years ago

raoulmillais commented 12 years ago

Hi,

I'm writing a chrome extension and currently chrome has no support for commonjs modules. I am using requirejs to manage my dependencies, but due to the shims to support CommonJS modules, jsOAuth willl not work (xhr is "require"d and require exists but does not have a module named xhr. Adding a check for define being a function and define.amd gets us someway to fixing the issue but we also need to the new up window.XMLHttpRtequest explicitly when define does exist. I have this working and would be happy to submit a pull request with tests if you would consider accepting this. Unless of course you thought there might be a better approach? I thought I would test the water given the currently highly politicised debate surrounding AMD vs CommonJS.

Regards,

Raoul

bytespider commented 12 years ago

Raoul,

What version of jsOAuth are you using?

I've not chosen a direction as regards to AMD as yet and as jsOAuth targets application platforms, those platforms tend to handle dependencies themselves.

I have started writing a 2.0 version of jsOAuth using requirejs and have found it very difficult to use for my cross platform requirements due to the fact I need it to work in CommonJS/Node, the browser and Platforms such as Appcelerator Titanium, all of which handle dependancies differently.

By all means write up how you added AMD support and I'll reference it in the Wiki and may come back to it when AMD is set in stone.

Rob

raoulmillais commented 12 years ago

Hi,

I started with 1.3.3, but made my changes based on HEAD. As you say, requirejs (AMD) can be a painful if you need to support both. It's a real shame chrome extensions don't come with CommonJS support :( I'll add a branch in my fork and put the changes there when I get back from work. In order to get my changes working needs a different {start,end}.js which doesn't redefine exports and wraps the code in this:

start: define(function (require, exports, module) {

end: exports.OAuth = OAuth; });

As an interim measure I guess I could add an extra build target using the AMD specific {start,end}.js That of course adds maintenance burden though.

karth2k1 commented 12 years ago

Hi Raoul,

Just this week, I encountered the exact same problem. Glad to see this issue/discussion. Please do let me know when you commit your changes to a new branch.

Thanks, -Karthik.