jamesmartin / inline_svg

Embed SVG documents in your Rails views and style them with CSS
MIT License
716 stars 72 forks source link

Add support for Propshaft #134

Closed martinzamuner closed 2 years ago

martinzamuner commented 2 years ago

Rails 7 adds the option of using a new asset pipeline library called Propshaft. It will most likely replace Sprockets in the future as the default asset pipeline for Rails as they move away from CSS compilers and JS bundlers.

This PR adds a new asset finder for Propshaft and it automatically loads it when appropriate. I've tried to make as few changes as possible because Sprockets is still the default and inline_svg supports many versions of Rails.

navidemad commented 2 years ago

Working well in my repository ⚡️

jamesmartin commented 2 years ago

@martinzamuner thanks for the PR! ✨

The code looks good to me. Tagging in @mjc-gh as they raised the issue of support for Rails 7 without using the Asset Pipeline in https://github.com/jamesmartin/inline_svg/issues/133. More 👀 on the approach taken in this PR would be helpful.

Also, note to self, I think we'll want to add Rails 7 (with and without Propshaft, if that's an option) as an integration test matrix item in: https://github.com/jamesmartin/inline_svg/blob/5e8e86de481f909b3d29d6455a7fc89d55311886/.github/workflows/integration_test.yml#L11

mjc-gh commented 2 years ago

Hey all, this PR look good to me overall. This Rails issue https://github.com/rails/rails/issues/43793 is generally related to the initial issue I brought up in #133 and to this PR as well. I think the gist of it all is either sprockets-rails or propshaft should be installed to actually serve assets on most Rails 7 apps and this gem can likely expect either or is present when Rails 7 is being used.

Presumably the existing asset pipeline/sprocket support still works so long as sprockets-rails is installed. Perhaps the additions to the integration test matrix should be for rails 7 with sprockets-rails and rails 7 with propshaft?

jamesmartin commented 2 years ago

I added Rails 7 to the integration tests and removed Rails 6 Webpacker, because it's become unreliable. I'm planning to drop support for Webpacker from this gem moving forward, as it seems to have fallen out of favor in the Rails world.