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

Caching assets at boot time doesn't map asset names to fingerprinted filenames #107

Open
agrobbin opened this issue Dec 1, 2019 · 3 comments

Comments

@agrobbin
Copy link

agrobbin commented Dec 1, 2019

I could definitely be doing something wrong here, but I've been unable to get the CachedAssetFile to work out of the box when using Webpacker.

Given this scenario:

  • A file in app/javascript/images/ named foo.svg, and
  • A template file in app/views/ calling inline_svg 'foo.svg' or inline_svg_pack_tag 'foo.svg', and
  • Configuration of config.asset_file = InlineSvg::CachedAssetFile.new(paths: [Rails.root.join('public', 'packs', 'media')], filters: /\.svg/)

This does not work out of the box, as the assets in public/packs/media/ are fingerprinted, so foo.svg does not exist (it's something like foo-[hex(16)].svg).

I've been able to work around this by defining my own custom cached asset file class like so:

class CachedWebpackAssetFile < InlineSvg::CachedAssetFile
  def named(asset_name)
    super(Webpacker.manifest.lookup(asset_name))
  end
end

However, I would expect this kind of thing to work out of the box.

I thought for a bit about configuring the InlineSvg::CachedAssetFile to load the app/javascript/images/ folder rather than the public/packs/media/ folder, but that doesn't feel like the right thing to do either, since something in the Webpack world could generate an SVG that doesn't exist on disk until after compilation. I also thought about calling inline_svg or inline_svg_pack_tag with 'foo' rather than 'foo.svg', but that doesn't play well in non-cached situations, which require the asset name to include the file extension to find the correct asset.

@jamesmartin
Copy link
Owner

Thanks for opening this issue. ✨ Apologies that the cached asset file is not working as expected.

When using this in production I always use paths containing non-fingerprinted, "static", assets. When caching the assets in memory at boot time it's unnecessary for them to be fingerprinted because the browser will never request them directly and so there's no need for any cache-busting. 🤷‍♂

But… I can definitely see why this is inconvenient, as it means assets must be either "static" or fingerprinted but can't be both. 😞

I doubt this works for non-Webpacker environments either.

I've been able to work around this by defining my own custom cached asset file class like so:

class CachedWebpackAssetFile < InlineSvg::CachedAssetFile
  def named(asset_name)
    super(Webpacker.manifest.lookup(asset_name))
  end
end

Thanks for finding a workaround. Seems like there's a couple of options for how to fix this, but it's probably going to involve two lookups: first using the "static" name of the asset and falling back to the "fingerprinted" name (via Webpacker.manifest.lookup or whatever). The other option, to avoid doing two lookups, is to allow this to be configured by users (maybe an option like use_fingerprinted_asset_names: true) but I'm less happy about making users configure the library.

@agrobbin
Copy link
Author

agrobbin commented Dec 2, 2019

Yeah I'm honestly not sure what the ideal option is here. The workaround we have makes sense in an entirely Webpacker-world, but since the gem needs to support both Webpacker + Sprockets, it might need to get a bit "smarter" in its logic.

@jamesmartin
Copy link
Owner

I'm throwing this open to anybody that wants to help. I probably won't be able to get to this quickly and it doesn't seem critical, so PRs are welcome. 🙇

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

No branches or pull requests

2 participants