evanmoran / oj

Unified web templating for the people. Thirsty people.
MIT License
444 stars 7 forks source link

requirejs compatability #8

Closed dminkovsky closed 11 years ago

dminkovsky commented 11 years ago

It appears that OJ can't be used with requirejs. This is because it attempts to write to module.exports when require is available, but requirejs does not have module.exports (or module).

I would just add a guard that checks to see if module is not undefined before assigning to module.exports.

evanmoran commented 11 years ago

Working on bower support now. I'm glad to support this as well.

evanmoran commented 11 years ago

Can you try this out and let me know if it enables RequireJS support? Replace the relevant module code in https://github.com/ojjs/oj/blob/master/src/oj.js#L34-L39:

// Export as a module in Node
if (typeof module === 'object' && typeof module.exports === 'object')
  module.exports = oj
// Export into named AMD module for RequireJS
else if (typeof define === 'function' && define.amd)
  define('oj', [], function(){return oj})
// Export globally if window exists
if ( typeof window === "object" && typeof window.document === "object" )
  window.oj = oj

This follows the jQuery approach of defining a named module with define('oj', ..). I'm not very familiar with RequireJS support, is this named approach the right thing to do? If it works I'll put this in OJ v0.2.2

dminkovsky commented 11 years ago

Will do, thanks.

On Wed, Oct 23, 2013 at 5:41 PM, Evan Moran notifications@github.comwrote:

Can you try this out and let me know if it enables RequireJS support? Replace the relevant module code in https://github.com/ojjs/oj/blob/master/src/oj.js#L34-L39:

// Export as a module in Node if (typeof module === 'object' && typeof module.exports === 'object') module.exports = oj // Export into named AMD module for RequireJS else if (typeof define === 'function' && define.amd) define('oj', [], function(){return oj}) // Export globally if window exists if ( typeof window === "object" && typeof window.document === "object" ) window.oj = oj

This follows the jQuery approach of defining a named module with define('oj', ..). I'm not very familiar with RequireJS support, is this named approach the right thing to do? If it works I'll put this in OJ v0.2.2

— Reply to this email directly or view it on GitHubhttps://github.com/ojjs/oj/issues/8#issuecomment-26948295 .

dminkovsky commented 11 years ago

Yeah, this patch works.

dminkovsky commented 11 years ago

First day here, too, with Bower and RequireJS. With the above patch, I've got what I think is a good base setup for setting up OJ with Bower and Require:

Notice forked -amd Underscore and Backbone are required from bower.

Reference:

evanmoran commented 11 years ago

This is awesome. Thank you for checking it out and I'll get it in asap!

Also, I wanted to be sure you knew that the oj commandline tool (http://ojjs.org/docs.html#commandline) can do this sort of packaging using npm instead of bower. The oj tool acts like Browserify and can bring any modules you require in node to the client so server/client is seamless. And I'm currently making this happen work for express as well. Definitely reach out if you want to learn more=)

Example code for oj commandline for javascript or coffee-script:

https://github.com/ojjs/oj-examples/tree/master/server-side-commandline https://github.com/ojjs/oj-examples/tree/master/server-side-commandline-coffeescript (notice they just uses package.json for OJ plugins)

That said, bower is blowing up and I'm very glad you brought it up. With so many plugins it is a very natural fit for oj!

dminkovsky commented 11 years ago

Actually, I was wrong. This patch works, but stochastically, because the require and interpreting happen sometimes before, sometimes after the jQuery loads.

Check out the first few lines of https://github.com/amdjs/backbone/blob/master/backbone.js to see how they address this-- the define happens inside a wrapper.

Or just not shim this and see if the current codebase works with requirejs's shim.

dminkovsky commented 11 years ago

Oh, thanks. Going to check out the CLI tool, then!

evanmoran commented 11 years ago

@dminkovsky Ah I see. One issue is I've been trying to avoid a strict Backbone dependency. OJ is just the View in MVC, so really it should be able to handle any Model framework. Possibly even EmberJS or other big MVC frameworks.

Of course clearly Backbone makes it much better. Not sure on this. Is an optional dependency possible in AMD?

dminkovsky commented 11 years ago

Oh all you need here is jQuery dependency, not Backbone. I linked to the shimmed AMD-style Backbone to show how they shim it without modifying original Backbone. I'm going to try this approach with OJ after attempting to shim with RequireJS's shim

evanmoran commented 11 years ago

Taking a look at this: https://github.com/umdjs/umd/blob/master/returnExports.js

evanmoran commented 11 years ago

I commited a version using this new "factory" approach. Let me know if it works for you...

https://github.com/ojjs/oj/blob/master/src/oj.js

evanmoran commented 11 years ago

OJ 0.2.2 now supports require js / AMD.