Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

undefined method `find_asset' for nil:NilClass in asset_pipeline_loader #145

Closed
javierjulio opened this issue Dec 18, 2015 · 21 comments
Closed

Comments

@javierjulio
Copy link
Contributor

Today I updated the sprockets-rails gem which is now at 3.0.0 and noticed an error occurring after updating: "NoMethodError·undefined method `find_asset' for nil:NilClass". I'm using latest premailer-rails 1.8.2. The error is is happening here: https://github.com/fphilipe/premailer-rails/blob/master/lib/premailer/rails/css_loaders/asset_pipeline_loader.rb#L10

Noticed the issue reported on sprockets-rails rails/sprockets-rails#237 back in April for the 3.0 version. I didn't see what the fix was but it was mentioned that it was resolved in react-rails. I figure the same issue that happened there is probably happening here.

@javierjulio
Copy link
Contributor Author

It seems the OP at rails/sprockets-rails#237 also posted about it at: reactjs/react-rails#241 I haven't been able to look into this yet as I need to revert back but wanted to mention the issue while I noticed it and had all this info.

@fphilipe
Copy link
Owner

Thanks for reporting. Does this error pop up only in production or also in development? I'm currently not using this gem and thus haven't come across this.

I guess the fix would be to augment the check in asset_pipeline_present? so that it returns false in that case. I'm happy to accept a PR 😉

@stevestmartin
Copy link

I am experiencing this issue in production and wasn't able to reproduce it in development. I haven't had time to write a pull request with proper tests and to maintain compatibility with sprockets 2, I do have a branch as a work around that is working for us in production if anyone else needs an immediate fix or needs a starting point to a more proper fix.

https://github.com/stevestmartin/premailer-rails/tree/sprockets3

@jnimety
Copy link

jnimety commented Dec 22, 2015

sprockets-rails no longer sets Rails.application.assets if Rails.application.config.assets.compile is false (rails/sprockets-rails#294). I'm having early success in production by simply setting Rails.application.config.assets.compile = true. We use a CDN, so this isn't strictly necessary, but it lets premailer-rails use assets in production.

@DannyBen
Copy link

DannyBen commented Jan 6, 2016

Thanks for the quick fix. Just came here with the same issue (production only), saw it was fixed, updated the gem to 1.9.0 (was 1.8.2) and it seems to be ok.

@glebm
Copy link

glebm commented Mar 3, 2016

Still broken! This fix makes it so the assets can no longer be found in production. Looking at rails/sprockets-rails#311, it seem this gem should use Rails.application.assets_manifest.assets[filename] to get the correct path to the compiled file in production.

A workaround for now (in the email template), works correctly in production and development:

link[rel='stylesheet' type='text/css'
  href="/assets/#{Rails.application.config.assets.compile ? 'email.css' : assets_manifest.assets['email.css']}"]

@fphilipe
Copy link
Owner

@glebm ::Rails.application.assets_manifest.assets[filename] should be used instead of ::Rails.application.assets.find_asset(file)?

@glebm
Copy link

glebm commented Mar 21, 2016

@fphilipe It should use assets_manifest.assets[filename] but only if config.assets.compile is false (i.e. the assets have been precompiled). If config.assets.compile is true, it should use a path such as /assets/filename.css.

@DannyBen
Copy link

Maybe this helps?

def asset_exist?(path)
  if Rails.configuration.assets.compile
    Rails.application.precompiled_assets.include? path
  else
    Rails.application.assets_manifest.assets[path].present?
  end
end

@glebm
Copy link

glebm commented Mar 21, 2016

@DannyBen That may incorrectly report true if an asset has been precompiled, but then removed. I think premailer-rails should not attempt to use precompiled assets if compile is true, as that may result in using older versions of assets or assets that no longer exist.

@glebm
Copy link

glebm commented Apr 11, 2016

Seeing as this is still an issue, can this be re-opened?

@fphilipe fphilipe reopened this Apr 11, 2016
zdennis added a commit to Aperta-project/Aperta that referenced this issue Apr 12, 2016
where it was trying to call "find_asset" on Rails.application.assets when it was nil.

Related: fphilipe/premailer-rails#145

This may resolve the issue in Aperta (in production environment) where inlining the CSS was failing.

APERTA-6543
egh added a commit to Aperta-project/Aperta that referenced this issue Apr 12, 2016
@fphilipe
Copy link
Owner

@glebm

Still broken! This fix makes it so the assets can no longer be found in production. Looking at rails/sprockets-rails#311, it seem this gem should use Rails.application.assets_manifest.assets[filename] to get the correct path to the compiled file in production.

A workaround for now (in the email template), works correctly in production and development:

link[rel='stylesheet' type='text/css'
  href="/assets/#{Rails.application.config.assets.compile ? 'email.css' : assets_manifest.assets['email.css']}"]

I've looked further into this issue. I don't understand why anyone would want to use the asset pipeline in production. Why don't you simply use stylesheet_link_tag, which takes care of this? Then premailer-rails will find your CSS in either public/assets or, if you have them on a CDN, request it. Am I missing something?

@javierjulio
Copy link
Contributor Author

javierjulio commented Jun 13, 2016

@fphilipe yes I believe what you're missing is that this happens with stylesheet_link_tag. I'm using that to link to a SASS file in my email layout that caused the error I reported. This is what I specify in my email layout file:

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<meta name="viewport" content="width=device-width"/>
<link href="https://fonts.googleapis.com/css?family=Lato:300,400,700" rel="stylesheet" type="text/css" />
<%= stylesheet_link_tag 'email' %>
<style type="text/css" data-premailer="ignore">
@media only screen and (max-width: 596px) {
...

@fphilipe
Copy link
Owner

@javierjulio I don't follow.

As far as I can tell, in development it will pick up the asset from the asset pipeline.

In production the asset pipeline won't be available (this will be nil) and thus, it will look for the compiled asset in public/. Since rails' stylesheet_link_tag will output something like http://www.example.com/assets/email-fingerprint.css, it will look for public/assets/email-fingerprint.css. If it can't find that (e.g. when you don't have the compiled assets in the app, but on a CDN), it will request http://www.example.com/assets/email-fingerprint.css.

Are you using the asset pipeline in production?

@glebm
Copy link

glebm commented Jun 14, 2016

@fphilipe In production, even when using a CDN, the local file is often available (e.g. if using a caching proxy such as CloudFront), so it should pull the local file instead of making a network request to CloudFront. So the following order:

  1. Rails.application.config.assets.compile is true => email.css.
  2. Otherwise, assets_manifest.assets['email.css'].
  3. Only if 2 is nil, fall back to making a request.

@fphilipe
Copy link
Owner

But stylesheet_link_tag will output the fingerprinted file name, which happens to be assets_manifest.assets['email.css']. It will look that file up in public/ and thus not make a request.

@glebm Are you saying that it should use the asset pipeline strategy in production? I think it's not considered good practice to have that enabled as it is rather slow. Maybe I'm misunderstanding what you mean in point 2. Can you elaborate?

@javierjulio
Copy link
Contributor Author

@fphilipe I have to be honest, I'm not entirely sure how it all works under the hood. I went with this approach as its what I expect with Rails as if I were in a normal browser requested view. I did forget to point out that I do specify my email stylesheet in the config/initializers/assets.rb:

# ...
# Precompile additional assets.
# application.js, application.css, and all non-JS/CSS in app/assets folder are already added.
Rails.application.config.assets.precompile += %w(
  email.css
)

@glebm
Copy link

glebm commented Jun 14, 2016

@fphilipe For some reason that wasn't working when I last tried it, but will check again.

@phlegx
Copy link

phlegx commented Jun 14, 2016

I use sprockets-rails version > 3.0.0 and this line (requires gem compass-rails):

CompassRails.sprockets.find_asset(logical_file_path).to_s

instead of:

Rails.application.assets.find_asset(logical_file_path).to_s

and it works for me!

If you don't use gem compass-rails you can get the asset with:

(Rails.application.assets || ::Sprockets::Railtie.build_environment(Rails.application)).find_asset(logical_file_path).to_s

@giedriusr
Copy link

Is this the proper way to get that logo?

@goulvench
Copy link

goulvench commented Aug 28, 2018

@phlegx's last solution works out-of-the-box, using Rails 4.2.1.

I've packaged it in a helper method in my app:

# app/helpers/application_helper.rb
# Returns the contents of the compiled asset (CSS, JS, etc) or an empty string
def asset_body(name)
   (Rails.application.assets || ::Sprockets::Railtie.build_environment(Rails.application)).find_asset(name).to_s
end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants