Should sprockets be a regular gem dependency?
andyshinn opened this issue · 5 comments
At https://github.com/polleverywhere/firehose/blob/master/lib/firehose/assets.rb#L1 we require sprockets
. But it is included only as a development dependency, which breaks deployment when running Bundler without development
or test
groups.
Should sprockets be a regular gem dependency? I'll submit a pull request for this trivial change if this is indeed the case.
This should only load if there's a reference to Firehose::Assets
. Could you let me know where you're seeing the error so I can track down where that may be happening?
A common idiom for this is
begin
require 'sprockets'
rescue LoadError
puts "continuuing without sprockets"
end
or to wrap that in a method
It is happening in another project requiring firehose
and using the client to publish. The same error happens right out of the example client usage:
require 'firehose'
require 'json'
json = {'hello'=> 'world'}.to_json
firehose = Firehose::Client::Producer::Http.new('//127.0.0.1:7474')
firehose.publish(json).to("/my/messages/path")
$ bundle exec ruby test.rb
/Users/andy/.rbenv/versions/2.0.0-p451/lib/ruby/gems/2.0.0/gems/firehose-1.2.12/lib/firehose/assets.rb:1:in `require': cannot load such file -- sprockets (LoadError)
from /Users/andy/.rbenv/versions/2.0.0-p451/lib/ruby/gems/2.0.0/gems/firehose-1.2.12/lib/firehose/assets.rb:1:in `<top (required)>'
from /Users/andy/.rbenv/versions/2.0.0-p451/lib/ruby/gems/2.0.0/gems/firehose-1.2.12/lib/firehose.rb:22:in `<top (required)>'
from test.rb:1:in `require'
from test.rb:1:in `<main>'
Well, looks like this is a circular dependency-type bug. Thanks to your stack trace
https://github.com/polleverywhere/firehose/blob/master/lib/firehose.rb#L22
# Detect if Sprockets is loaded. If it is, lets configure Firehose to use it!
Firehose::Assets::Sprockets.auto_detect
So, maybe it should check const_defined? or something?
I'd rather move this detection code into something like firehose/assets
and require anything concerned about firehose JS assets to require "firehose/assets"
. This is more inline with splitting out firehose into several different gems.