Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Regression in v3.2.5: precompiled assets aren't correctly found when using assets that have more than one extension #6598

Closed
fj opened this Issue · 12 comments

7 participants

@fj
fj commented

I upgraded from Rails 3.2.3 to Rails 3.2.5. When doing so, Heroku now complains that one of my assets, ending in ".min.js", isn't compiled, and errors out.

My config/application.rb contains:

    # JavaScript files to include in :defaults expansion
    config.action_view.javascript_expansions[:defaults] = %w{rails modernizr-1.7.min}

    config.assets.initialize_on_precompile = false
    config.assets.precompile += [
# ...
      "screen/libs/modernizr-1.7.min.js",
# ...
    ]

When I hit any page in my application, I get:

ActionView::Template::Error (screen/libs/modernizr-1.7.min isn't precompiled):
    12: 
    13:   = stylesheet_link_tag "screen"
    14:
    15:   = javascript_include_tag "screen/libs/modernizr-1.7.min"
    16: 

Part of the Heroku deployment process already runs rake assets:precompile, though:

-----> Preparing app for Rails asset pipeline
       Running: rake assets:precompile

and nothing's changed about that in my 3.2.5 upgrade. Additionally, the precompiled assets are there on disk in the expected locations, and those locations are the same as they were before.

This is likely to be a 3.2.5 regression.

@homakov

do you have public/assets/screen/libs/modernizr-1.7.min.js file?

@drogus
Collaborator

I've checked that and it's not even heroku specific error. This won't also work when you try it on localhost. The quick workaround is to remove min part, so the file is named modernizr-1.7.js, then it will work.

@drogus
Collaborator

Actually I can't reproduce it anymore locally (probably I did something else wrong), but it still fails on heroku. I will look into it closer.

@drogus
Collaborator

Ok, the real problem is not the compile phase, it's include tag. Just add extension to your javascript_include_tag, so it looks like that:

= javascript_include_tag "screen/libs/modernizr-1.7.min.js"
@route

@drogus well I think the cause is #6423

@drogus
Collaborator

@route yup, I've already found that out and have the fix ready. I will push in a few minutes

@drogus drogus was assigned
@drogus drogus referenced this issue from a commit in drogus/rails
@drogus drogus Fix asset tags for files with more than one dot
After the fix done in 39f9f02, there are cases that will not work
correctly. If you have file with "2 extensions", like foo.min.js and you
reference the file without extension, like:

    javascript_include_tag "foo.min"

it will fail because sprockets finds foo.min.js with foo.min argument.

This commit fixes this case and will get the right file even when
referrencing it without extension.

(closes #6598)
10537ba
@drogus
Collaborator

I've pushed a fix, but this seems like another hack that should be probably fixed in sprockets. I'm cool with such implementation, but it would be nice to get some feedback - if it's not good for others I can look if it can be fixed in a better way in sprockets (probably in Asset, to get proper logical_path)

@josevalim @guilleiguaran @rafaelfranca @carlosantoniodasilva what do you guys think about that: 10537ba

@fj
fj commented

I updated the title and description of this issue to reflect the actual problem (i.e. that it's really about the precompiled assets not being found correctly, not that they were never precompiled).

I can confirm that the workaround of putting the full path in the javascript_include_tag resolves the issue pending a fix. Thanks for your help, @drogus, @route, and @homakov.

@carlosantoniodasilva

@drogus the fix looks ok to me. A similar commit was added to sprockets-rails, so it'd probably need to be fixed there as well. Thanks :)

@Vanuan

Shouldn't that method be merged with this: https://github.com/rails/rails/blob/master/actionpack/lib/action_view/helpers/asset_tag_helpers/asset_paths.rb#L30
? Is there any behaviour difference? IMO, the behaviour of tag helpers' rewrite extension method shouldn't depend on config.assets.enabled. See also #6310

@rafaelfranca
Owner

This looks fine to me too.

@drogus
Collaborator

@Vanuan I agree that we should look into merge'ing those methods somehow, but I will leave this fix like it is for 3-2-stable. It was extracted into sprockets-rails anyway, so I will take a look at it on master and if it makes sense backport it (but since it will be refactoring, not a bug fix, I would rather do it on master only).

@drogus drogus closed this issue from a commit
@drogus drogus Fix asset tags for files with more than one dot
After the fix done in 39f9f02, there are cases that will not work
correctly. If you have file with "2 extensions", like foo.min.js and you
reference the file without extension, like:

    javascript_include_tag "foo.min"

it will fail because sprockets finds foo.min.js with foo.min argument.

This commit fixes this case and will get the right file even when
referrencing it without extension.

(closes #6598)
5b0a891
@drogus drogus closed this in 5b0a891
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.