documentcloud / jammit

Industrial Strength Asset Packaging for Rails
http://documentcloud.github.com/jammit/
MIT License
1.16k stars 197 forks source link

Adding Sinatra support to Jammit #61

Closed trydionel closed 13 years ago

trydionel commented 14 years ago

Hi guys,

Just wanted to get this fork into Github's new pull request tracker! Only a couple of new commits (small bugfixes) since my previous pull request.

Thanks, Jeff

jashkenas commented 14 years ago

Thanks for bumping this again. I'd still very much like to get this merged in soon ... just been busy.

matth commented 14 years ago

Bump 2, would love to see this in the official release :)

jashkenas commented 14 years ago

Hey Jeff. Some quick questions:

trydionel commented 14 years ago

Hey Jeremy,

From what I can tell, those two extensions are "official" -- Sinatra::StaticAssets is recommended on the Sinatra extensions page, and Sinatra::Cache is the only extension I found for caching.

As for caching -- I was mainly trying to mimic the methodology used in Rails -- only cache the assets if the app is caching. Since Sinatra only supports it with an extension, I focused on making caching work transparently when that extension is installed. In fact, asset caching can be turned on easily without Sinatra::Cache being present by simply following the same setup Sinatra::Cache requires:

class Foo < Sinatra::Base
  register Sinatra::Jammit
  # Jammit will now cache assets
  set :cache_enabled, true
  # Jammit will put assets here (defaults to public/assets if not specified)
  set :cache_output_dir, File.join('path', 'to', 'cache')
  ....

As for static assets -- Sinatra doesn't support the underlying view helpers at all (javascript_include_tag, stylesheet_link_tag). I didn't want to build in support for the view helpers considering that there was already an accepted and recommended solution. As best as I can tell, the only way to get around this dependency would be to rework the Jammit view helpers without using any Rails-isms. I'd be interested in helping you tackle this, as the Sinatra::StaticAssets extension isn't exactly quick and easy to set up.

jashkenas commented 14 years ago

So, the reason for using underlying rails helpers is because they support required features (in Rails at least), such as asset hosts, and asset paths. If you want to make the Sinatra version of Jammit avoid these features, that would be fine -- but the Rails integration should continue to use them.

matth commented 14 years ago

Hi there, hope you don't mind my input as both a Jammit and Sinatra fan!

There seems much better external caching options available if needed, such as Rack::Cache or even an upstream proxy server such as Squid.

Also StaticAssets seems to depend in turn on sinatra-url-for which hasn't been updated for a while, (still using gems.github.com).

In the context of Sinatra - where we work much closer to the metal anyway - these two dependancies are maybe not really needed. For me the real benefit of the patch is being able to use the packaging, minification and templating features of Jammit in my Sinatra app.

Just my two pence, thanks for all the hard work so far!

trydionel commented 14 years ago

Hey Matt,

Thanks for the input! Definitely happy to hear other dev opinions :) I think the biggest advantage for Sinatra::Cache is that it's actually integrated into your app, allowing for e.g., fragment caching. Also, I don't know how to easily turn Jammit caching on/off in response to upstream caching solutions.

I totally agree that Sinatra::StaticAssets isn't an ideal library, both because of it's dependency on url-for and some unexpected assumptions it makes (e.g., your output will be forced into XHTML mode). That's a big reason for why I didn't specify it as a required dependency. I too feel that Sinatra should be lightweight, and that the view helpers Jammit provides aren't strictly needed to get the real benefits. Thus, I only "turn them on" if the app is already using the required helpers.

Strictly speaking, you don't have to use StaticAssets to get the view helpers, only define stylesheet_link_tag and javascript_include_tag in a manner which matches Rails. I'll have to dig further into Jammit to see just which features it looks for in these two methods, and may spike a bit on cobbling together a Sinatra equivalent that doesn't use StaticAssets.

I'm going to be at Ruby Hoedown for the next few days, so I'll be a bit quiet in this channel. I'll pick it up if I get a chance, but it may be next week before I'm around again. Thanks guys!

jashkenas commented 13 years ago

This ticket's been idling for a while now, so I'm going to close it. If someone picks this up and wants to continue working on it, add a comment here, and we can open it again.

matth commented 13 years ago

Hi guys, I'd still like to see this integrated, am currently deploying from trydionel's sinatra branch!

End goal for me is to get some nice jammit integration with sinatra, not too bothered about the include tag helpers or cache layer, it's more attaching a sinatra route to the jammit packaging functionality.

Can have a go at finishing this branch off if everybody has moved on? Up for collaboration too!

jashkenas commented 13 years ago

Sure thing -- glad to hear you're willing to pick up the reins. Re-opening the ticket.

matth commented 13 years ago

Cool, have a big release mid month but should hopefully have made some progress by the end of November. Will update.

sstrudeau commented 13 years ago

@matth -- any progress on this front?

jashkenas commented 13 years ago

None that I've heard of.

matth commented 13 years ago

Apologies guys I have not been able to make any progress on this yet. We are still using Jammit + Sinatra off a slightly modified version of trydionel's branch.

Tidying, testing and merging the code back is definitely still on the todo list but if you guys want to close this issue for now then that's fine.

sstrudeau commented 13 years ago

@matth -- no worries. I unfortunately don't have the time to tie this up myself, but knowing the status of this patch helped me weigh my options. Thanks.

jashkenas commented 13 years ago

Closing this issue for the time being then...

alan-andrade commented 11 years ago

What happened ? I'm looking for Sinatra Jammit option.