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

1.5.0 and 1.5.1 can no longer find assets in production #98

Closed
shepmaster opened this issue Jun 18, 2019 · 16 comments
Closed

1.5.0 and 1.5.1 can no longer find assets in production #98

shepmaster opened this issue Jun 18, 2019 · 16 comments

Comments

@shepmaster
Copy link

shepmaster commented Jun 18, 2019

I am still debugging this, but I figured I'd open an issue earlier in case other people have the same problem.

We were previously using 1.4.0 with seemingly no issues, but on upgrading to 1.5.0 (or 1.5.1) we now get:

<!-- SVG file not found: 'logo-black.svg' -->

The corresponding view is pretty basic:

= inline_svg "logo-black.svg", class: "welcome-logo"

What's extra strange is that this only appears when we've deployed to Heroku, not in local development.


Which version of Rails are you using?

Ruby 2.6.2, Rails 5.2.3

Are you using Webpack at all?

Yes, via webpacker

Are you using Sprockets or Webpacker? Or both?

Both

Are you using your own custom asset_finder?

No

Could you paste your InlineSvg.configure block, if any

Not using one.

@shepmaster
Copy link
Author

Strangely, explicitly forcing the old asset finder seems to work (we are double checking):

Rails.application.config.after_initialize do |app|
  InlineSvg.configure do |config|
    config.asset_finder = app.instance_variable_get(:@assets)
  end
end

@jamesmartin
Copy link
Owner

@shepmaster thanks for opening this issue and sorry about the problems you're having with 1.5x.

Strangely, explicitly forcing the old asset finder seems to work (we are double checking):

This is very strange because the Railtie should automatically configure that for you during App initialization.

if assets = app.instance_variable_get(:@assets)
# In default Rails apps, this will be a fully operational
# Sprockets::Environment instance
config.asset_finder = assets

I guess this must also mean that your app's assets are not being precompiled by Heroku on deployment, which is the default behavior. If your assets were being precompiled then there would be no Sprockets @assets instance variable and the gem would fallback to the StaticAssetFinder… 🤔

@beauraF
Copy link

beauraF commented Jun 19, 2019

Hi @jamesmartin, I had the same problem when I went from 1.3.1 to 1.5.0.
I couldn't reproduce it locally. Only in production.

We use Sprockets and Webpacker. But it is still Sprockets that manages the compilation of SVG. Of course we precompile our assets in production.

I'm trying to investigate. I'll come back to you if I have any further information.

@igorkasyanchuk
Copy link

+1 for this issue. maybe because of this line: v1.5.0...v1.5.1#diff-f77654af79391ea695bddcf725119364R21

if you have webpacker it will use it for asset finder

@shepmaster
Copy link
Author

This is very strange because the Railtie should automatically configure that for you during App initialization.

Exactly why I think it's so very strange!

I guess this must also mean that your app's assets are not being precompiled by Heroku on deployment, which is the default behavior.

We certainly intend for the assets to be precompiled. The Heroku log states:

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

If your assets were being precompiled then there would be no Sprockets @assets instance variable and the gem would fallback to the StaticAssetFinder

Wouldn't it fallback to the Webpacker loader, since we also have it?

if assets = app.instance_variable_get(:@assets)
# In default Rails apps, this will be a fully operational
# Sprockets::Environment instance
config.asset_finder = assets
elsif defined?(Webpacker)
# Use Webpacker when it's available
config.asset_finder = InlineSvg::WebpackAssetFinder
else
# Fallback to the StaticAssetFinder if all else fails.
# This will be used in cases where assets are precompiled and other
# production settings.
config.asset_finder = InlineSvg::StaticAssetFinder

Said another way, why wouldn't Webpacker be available?

@shepmaster
Copy link
Author

shepmaster commented Jun 19, 2019

Using this initializer also appears to work:

Rails.application.config.after_initialize do |app|
  InlineSvg.configure do |config|
    config.asset_finder = InlineSvg::StaticAssetFinder
  end
end

@shepmaster
Copy link
Author

And forcing the loader to be WebpackAssetFinder causes my development environment to exhibit the same problem as production, so this certainly seems like the right area to investigate.

config.asset_finder = InlineSvg::WebpackAssetFinder

@igorkasyanchuk
Copy link

Ideal is to have both finders work together. Of course check if webpack if available, if not only static finder works

@shepmaster
Copy link
Author

check if webpack if available

I think that may be a key problem for our case.

Our app uses both the asset pipeline and Webpack (via Webpacker) as we slowly migrate from the former to the latter. The two tools process completely independent sets of assets for us.

As I understand the problem, app.instance_variable_get(:@assets) returns nil in production because the assets have been precompiled. However, because we have Webpack, the WebpackAssetFinder is enabled. However, Webpack doesn't know about these assets, so when the SVG is requested the finder fails.

Prior to 1.5, the fallback would have gone to the static asset finder, which would find the precompiled assets .

@jamesmartin
Copy link
Owner

@shepmaster thanks for clarifying that you're using both Sprockets and Webpacker while you transition.

I added a note to the readme when I released 1.5.0 that was intended to help with any confusion, but it was not clear enough:

If you have both Sprockets and Webpacker enabled for some reason and you want Inline SVG to use Webpacker to find SVGs then you should configure the asset_finder appropriately

I'm going to add a note about using the StaticAssetFinder if you precompile assets in production.

In your case I think it's sufficient to just configure the StaticAssetFinder explicitly until you decide whether Webpacker will be bundling your image assets:

InlineSvg.configure do |config|
  config.asset_finder = InlineSvg::StaticAssetFinder
end

Given this is a breaking change, I probably should have bumped the major version to 2.0. Apologies again for the disruption. 🤔

Ideal is to have both finders work together. Of course check if webpack if available, if not only static finder works

That's kind of a complicated problem because Webpacker doesn't expose any reliable method to ask, "is Webpacker bundling images". We could check the Webpacker manifest file and start greping for paths that appear to be images, but that's probably not very reliable.

@jamesmartin
Copy link
Owner

I think I'm going to remove the auto-selection of Webpacker and make it "opt-in" for now. This is going to break too many Heroku Apps and it's not reasonable to make everybody do manual configuration to get back existing functionality.

@jamesmartin
Copy link
Owner

jamesmartin commented Jun 19, 2019

I've released v1.5.2, which completely reverts the automatic Webpacker detection and makes Webpack "opt-in" by default, meaning that if you're using Sprockets to serve assets (either dynamically, or precompiled), there should be no additional configuration required.

Please update to this version and let me know if you have any problems. Sorry for all the trouble these last couple of releases have caused.

This was referenced Jun 19, 2019
@shepmaster
Copy link
Author

@shepmaster thanks for clarifying that you're using both Sprockets and Webpacker while you transition.

Sorry, I thought I communicated that in the initial comment.

I added a note to the readme

Yep, but it wasn't part of the release notes or changelog that Dependabot provided for our upgrade.

Webpacker doesn't expose any reliable method to ask, "is Webpacker bundling images"

And in our case, webpack is bundling some images, just not those that we are using inline_svg for. In webpack land, we are using @svgr/webpack.

Sorry for all the trouble these last couple of releases have caused.

Not at all! Thank you for your time and maintenance of a useful library!

@jamesmartin
Copy link
Owner

@shepmaster I'm going to close this out now. Thanks again for taking the time to open up an issue and describe the problems you faced.

@richjdsmith
Copy link

Thought I would chime in and see if a bit of extra info could help.

I am using Rails 6.0rc1 - Ruby 2.5.3 and inline_svg v1.5.2 and I am only using webpacker (no sprockets).

When I use the InlineSvg::WebpackAssetFinder config, that finder only appears to find files that are precompiled (that is, I run $./bin/webpack before I start my server. If I do that, the finder find's my files no problem. However, it does not appear to use any assets found via webpack-dev-server or update changes based on if I remove or add files to my images folder.

This is the finder that I was using before (1.3) the 1.5.2 update:

class WebpackerAssetFinder
  class FoundAsset
    require 'open-uri'
    attr_reader :path

    def initialize(path)
      @path = path
    end

    def pathname
      if Webpacker.dev_server.running?
        begin
          asset = open(webpacker_dev_server_url)
          tempfile = Tempfile.new(path)
          tempfile.binmode
          tempfile.write(asset.read)
          tempfile.rewind
          tempfile
        rescue StandardError => e
          Rails.logger.error "Error creating tempfile: #{e}"
          raise
        end
      else
        Rails.application.root.join("public", path.gsub(/\A\//, ""))
      end
    end

    private

    def webpacker_dev_server_url
      "#{Webpacker.dev_server.protocol}://#{Webpacker.dev_server.host_with_port}#{path}"
    end
  end

  def find_asset(filename)
    if webpack_asset_path = Webpacker.manifest.lookup(filename)
      FoundAsset.new(webpack_asset_path)
    end
  end
end

# Override the Sprockets asset finder. This is my old homebrew method.
InlineSvg.configure do |config|
  config.asset_finder = WebpackerAssetFinder.new
end

and it still works fine with the 1.5.2 version plugin.

@jamesmartin
Copy link
Owner

@richjdsmith thanks for the information. As this seems to be a problem with the interaction between inline_svg and the Webpacker dev server would you mind opening a separate issue for this? Also, if you're motivated to help work on merging your working asset finder with the current version, that would be much appreciated.

Thanks 🙏

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

5 participants