Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AMD support for runtime.js #634

Closed
phated opened this issue May 11, 2012 · 12 comments
Closed

AMD support for runtime.js #634

phated opened this issue May 11, 2012 · 12 comments

Comments

@phated
Copy link

phated commented May 11, 2012

I am currently using https://github.com/wearefractal/jaded to compile my jade templates to AMD, but have to use a custom runtime.js to keep jade from being globally defined.

Would it be possible to add that functionality into the main project?

Thanks

@tj
Copy link
Contributor

tj commented May 11, 2012

meh -1 from me, im not a fan of AMD

@monteslu
Copy link

+1 from me. I love AMD. Is there any real downside to adding this?

@tj
Copy link
Contributor

tj commented May 11, 2012

im fine with it if it can be a separate compile step thing, forget what the AMD signature looks like for registering

@phated
Copy link
Author

phated commented May 22, 2012

It just needs to be wrapped like so:

define([], function(){
var exports = {};
// Augment exports here
return exports;
});

I issued a pull request but if you don't like my naming or whatever, it would just be cool to have this. Thanks.

@tj
Copy link
Contributor

tj commented May 22, 2012

I kinda feel like all these require type things are approached wrong. Everything should be module.exports / exports, and then the web "framework" or whatever build-system should wrap accordingly, there's a lot of fragmentation and inconsistency in the community right now

@monteslu
Copy link

There's really only two formats with any mindshare right now, AMD and CommonJS. Both are useful, and are only somewhat competing. CommonJS is more typical on the server while AMD is more typical on the client. AMD is supported by jQuery, Dojo, Mootools, Firebug, and other things.

The pull request doesn't harm the project at all, and adds something very useful for client side MV*.

@evdb
Copy link

evdb commented May 23, 2012

To solve this issue for myself I've created jade-amd: https://github.com/mysociety/node-jade-amd

It has a cli that wraps the runtime.js and compiled templates in AMD semantics and is intended to play well with the RequireJS build process. There is an example app that exercises all the features.

Perhaps the way to close this ticket is to add some docs to Jade that points to other solutions like jaded and jade-amd. I tend to agree that it is a build process task.

@phated
Copy link
Author

phated commented May 26, 2012

I understand that there are ways to work around this with custom solutions (such is my solution - https://github.com/phated/grunt-jade), but I figured this would be helpful to keep upstream since runtime.js is a client-side file, and AMD is pretty much the standard for client-side modules right now.

@yocontra
Copy link

@evdb - Wrote something similar (https://github.com/wearefractal/jaded). Would be nice if this was supported out of the box but using a 3rd party CLI/runtime isn't too bad.

@monteslu
Copy link

So if at least three different projects are having to wrap the jade runtime in AMD themselves, why not just fix it upstream?

@ryanflorence
Copy link

runtime.js doesn't need to be wrapped, RequireJS 2.0 has the "shim" config that works perfectly with the global pattern:

paths: {
  jade: '../node_modules/jade/runtime'
},
shim: {
  jade: {
    exports: 'jade'
  }
}

And you're done. If you're one of the few people who actually MUST remove the global variable then yeah, runtime.js should implement a noConflict (and while you're at it remove the built-in shims :\ ...) and then you can change the exports config on jade like so:

exports: function() {
  return jade.noConflict()
}

It's still RequireJS's responsibility to be the adapter.

@ForbesLindesay
Copy link
Member

AMD should just be built on top of a common-js version. And should be done so by people who use AMD, not in the library itself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants