Support both Sprockets and Webpacker?

[kylefox]

First off, thanks for this great gem 👍

We're migrating our Rails app from Sprockets to Webpacker, but we're doing it in gradual steps. We need to find SVGs in both the asset pipeline and webpacker.

Would you be open to a PR that adds functionality for configuring multiple asset finders? I'm thinking something like this:

InlineSvg.configure do |config|
  config.asset_finders = [
    InlineSvg::WebpackAssetFinder,
    InlineSvg::SprocketsAssetFinder
  ]
end

The idea would be that inline_svg would first attempt to find the asset in webpacker, and if not found would fallback to finding with sprockets. The array syntax could allow people to invert this behavior by swapping the order of the finders if they prefer to search the asset pipeline first.

Thoughts?

[jamesmartin]

@kylefox thanks for the suggestion. I've been thinking about the upgrade path for existing Rails apps and I think your suggestion makes sense. Definitely open to the idea of multiple asset finders. 👍

[kylefox]

Thinking about it more, I wonder if it makes sense to introduce a new helper explicitly for webpacker — similar to what Rails/Webpacker itself has done. 🤔

Rails has image_tag for images in the asset pipeline, and image_pack_tag for images in webpacker.

What do you think about replacing inline_svg with inline_svg_tag for the asset pipeline and inline_svg_pack_tag for webpacker?

I think those helpers more closely match the Rails helpers. inline_svg could be an alias for inline_svg_tag or inline_svg_pack_tag (with a deprecation warning) depending on which asset_finder is currently configured.

[jamesmartin]

What do you think about replacing inline_svg with inline_svg_tag for the asset pipeline and inline_svg_pack_tag for webpacker?

I'm not against this. I think it would have to be a major version bump as it would completely break the v1.x interface. Sounds like v2.0 might be the "Webpacker" release. 😆

[kylefox]

Yeah, I think for sure it would need to be a major version bump. Just wanted to confirm if you'd be open to a change like that (and the resulting major version bump) before I explored that route. Thanks!

[jamesmartin]

Just wanted to confirm if you'd be open to a change like that (and the resulting major version bump) before I explored that route.

I think the interface change you proposed will clear up confusion and make things more consistent with similar Rails internal helpers, so I'm definitely open to this and a major version bump as a result. 👍

[kylefox]

As an experiment, I tried implementing inline_svg_tag and inline_svg_pack_tag in a somewhat lazy way (#103), but it actually seems to be working fairly well. Plus, as far as I can tell, it's backwards-compatible.

Let me know if you think this implementation approach looks acceptable, or if I should pursue another approach!

[jamesmartin]

Let me know if you think this implementation approach looks acceptable, or if I should pursue another approach!

Thanks for opening #103, @kylefox. I like the approach. Let's try to get this into shape for a 1.6.0 release.

[kylefox]

Hey @jamesmartin can you let me know if there's anything else needed before #103 can be merged and released? Thanks!

[jamesmartin]

can you let me know if there's anything else needed before #103 can be merged and released?

Looks good. 👍 I've merged #103 and will make a new release including your changes.

[jamesmartin]

Released in v1.6.0. Thanks again for your help, @kylefox! ✨

[kylefox]

No problem, thanks for giving responsive and helpful feedback 👍