search

jeremyruppel / walrus

A bolder kind of mustache
http://documentup.com/jeremyruppel/walrus/
MIT License
215 stars 10 forks source link

AMD Support with UMD Pattern #21

Closed steckel closed 12 years ago

steckel commented 12 years ago

Looks like running a cake compile added more changes than I originally intended. I can submit another pull-request with an interactive git commit to the walrus.coffee so that you could do the compile on your end if you'd prefer.

We've been utilizing AMD modules recently for async browser loading so I added AMD support to walrus.js with the UMD pattern mentioned here: https://github.com/umdjs/umd/blob/master/returnExports.js

All the tests pass but, I haven't manually tested anything but walrus.js so far.

Let me know your thoughts. :+1:

jeremyruppel commented 12 years ago

The other changes are ones I've seen come up before during compilation on different systems. Has to do with slightly different versions of jison. I'm fine with keeping those in the pull because they're automatic and will be overwritten again at some point anyways. I'll take a look at this today and get back to you. Thanks!

jeremyruppel commented 12 years ago

Looks like this does break backwards compatibility if the developer isn't using AMD in the browser. I've set up a fiddle here pointing to the compiled walrus.js on your fork so you can test it out if you want.

However, since this is very AMD-specific behavior, I'm feeling like this might belong in its own compiled file, similar to how mustache does it. Take a look at their approach and let me know what you think.

steckel commented 12 years ago

Did a quick amend to that commit since I botched the export name space. The JSFiddle should work now.

I think that this approach will be more clean than mustache's wrappers if it can past the acceptance tests/ manual tinkering.

jeremyruppel commented 12 years ago

Agreed. I don't like what mustache had to do to support all those different require patterns, but they had to do something because they're super popular. Walrus isn't, so f that noise.

Thanks for this. Merged, I'll update this when I've published to npm.

jeremyruppel commented 12 years ago

Release 0.6.1 is on npm.

steckel commented 12 years ago

:+1: on "f that noise" and :+1: on "0.6.1 on npm"