paolochiodi / htmlcompressor

A work in progress port of google's htmlcompressor.jar
Other
152 stars 26 forks source link

Compress specific path #35

Closed ndvbd closed 6 years ago

ndvbd commented 8 years ago

Now you can add in the routes.rb file, if you want, for specific routes the option:

:compress => false or :compress => true

And then the HtmlCompressor will act accordingly.

Another option was added to DEFAULT_OPTIONS: :compress_by_default => true

paolochiodi commented 8 years ago

Hi @nadavb, thanks for submitting this PR (and thanks @marvindanig for reviewing it).

I'm a bit concerned on merging this PR because it will break the middleware for people using it without rails. I'm undecided on what is best

What do you think?

marvindanig commented 8 years ago

I think a separate thing -- a 'htmlcompressor_rails' gem would be better. It's unnecessary to mix up everything for everyone into one gem.

ndvbd commented 8 years ago

If you gonna separate it, you'd have to maintain two codebases, wouldn't you? The question is how many people are using this gem without rails... My "specific compress" feature is based on Rails Routes, so maybe you can wrap the feature in a guard...

marvindanig commented 8 years ago

@nadavb :+1:

I prefer rails over other alternatives because after a while all 'em arrive to what rails provides by default.

Waiting for an update of this gem so that I don't have to shake the internals of my gemset manually.

paolochiodi commented 8 years ago

I agree that the best solution would be to have a separate gem, but unfortunately at the moment I don't have the time (and honestly the interest) to work on that. If one of you is interested in doing that I'll be happy to advertise such a gem here.

In the meantime @nadavb can you make two small fixes to the PR:

I'll merge and publish after that

Thank you all for the collaboration

ndvbd commented 8 years ago

Hi @paolochiodi, unfortunately I'm not available in the following weeks to do it... Sorry for that.

marvindanig commented 8 years ago

I'm happy to volunteer for the said gem but before that let me grab a fork and see its internals. brb.

marvindanig commented 8 years ago

Okay, I now have the gem html_compressor_rails in place, essentially a fork with changes that @nadavb suggested on his pr. Since it is a separate gem now, I do not think we need a guard for rails_env anymore.

Would be nice if you guys can give it a quick look / tip / see credits before I publish it to rubygems.org

paolochiodi commented 8 years ago

@marvindanig I'd suggest to depend on this gem for the actual compressor (instead of using a fork) and only provide a different rack middleware.

This way the core functionality won't be duplicated and you will be easily able to import any changes to this repo (just updating the gem version in you gemfile)

marvindanig commented 8 years ago

Hmm, makes sense. Let me take this out first.

ndvbd commented 8 years ago

So, is it in somewhere?

paolochiodi commented 8 years ago

looks like @marvindanig never published the gem. @marvindanig can we help with that?

ndvbd commented 8 years ago

Thanks guys. If you can also assist me with http://stackoverflow.com/questions/35557946/access-routes-in-rails-middleware - that would be lovely.

ndvbd commented 8 years ago

Folks, I've used this Gem for around 6 months, and just discovered that it added ~3 seconds to the response time of my server. This is really bad. Please see this as a warning - if you think to try it - make sure you measure your overall response time of the server (whether if it's apache or else). I removed it completely and now the website is really fast.

The right solution should be else. Either doing the compression in a low level language, to add speed, or, better, doing the compression not in real-time. Something like assets pipeline for the views and partials.