Should RailsHelper really overwrite stylesheet_path/javascript_path? #3417

Closed
raroni opened this Issue Oct 24, 2011 · 5 comments

Comments

Projects
None yet
3 participants

raroni commented Oct 24, 2011

In my 3.1.1 app stylesheet_path('application') throws ActionView::Template::Error (application isn't precompiled). stylesheet_include_tag works fine, but in this case I need the path for the stylesheet.

I think this happens because Sprockets::Helpers::RailsHelper.stylesheet_path calls asset_path(source) without specifying file extensions, asset type or anything.

stylesheet_tag_helpers.rb seems to be doing it the proper way:

def stylesheet_path(source)
  asset_paths.compute_public_path(source, 'stylesheets', :ext => 'css', :protocol => :request)
end

This method is included in Sprockets::Helpers::RailsHelper (through ActionView::Helpers::AssetTagHelper), but overridden afterwards with:

def stylesheet_path(source)
  asset_path(source)
end

If I remove the stylesheet_path method from RailsHelper, everything works as expected.

With my level of understanding of Rails and Sprockets, it would seem that both javascript_path and stylesheet_path should be removed from RailsHelper as they are given through AssetTagHelper.

I'd be glad if anyone with a greater insight would comment on this. Thank you!

Owner

spastorino commented Dec 4, 2011

Is this still an issue? can you provide an app that reproduces it? Thank you!!!

raroni commented Dec 30, 2011

Sorry for the slow response. I've now created a test app that exposes this. In order to reproduce, do the following:

  • git@github.com:rasmusrn/rails_asset_helper_test.git
  • cd rails_asset_helper_test
  • bundle install
  • bundle exec rake assets:precompile
  • rails s -eproduction
  • open http://localhost:3000/

Hopefully, you'll get a application isn't precompiled. I would've expected it to print /assets/application-1c9609bc58b1e77c196de29021583379.css or something like that.

If you try changing the contents of app/views/welcome/index.html.erb to <%= stylesheet_link_tag('application') %> it works fine.

@ghost ghost assigned guilleiguaran Jan 3, 2012

Owner

spastorino commented Jan 3, 2012

Ok I see what the issue is ... we have to add here the extension https://github.com/rails/rails/blob/master/actionpack/lib/sprockets/helpers/rails_helper.rb#L72.
The same way we pass here https://github.com/rails/rails/blob/master/actionpack/lib/sprockets/helpers/rails_helper.rb#L46

We need that and a failing test, same thing from javascript_path

@spastorino spastorino closed this in cbe9180 Jan 3, 2012

Owner

spastorino commented Jan 3, 2012

@rasmusrn I've pushed a fix please try it out and let me know if everything works fine now

raroni commented Jan 4, 2012

It works. Thanks! :)

carlosantoniodasilva pushed a commit to carlosantoniodasilva/rails that referenced this issue Jan 5, 2012

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