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

Change asset finder configuration #155

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 30 additions & 16 deletions lib/inline_svg.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
require 'active_support/core_ext/module/delegation'

require "inline_svg/version"
require "inline_svg/action_view/helpers"
require "inline_svg/asset_file"
require "inline_svg/cached_asset_file"
require "inline_svg/finds_asset_paths"
require "inline_svg/propshaft_asset_finder"
require "inline_svg/sprockets_asset_finder"
require "inline_svg/static_asset_finder"
require "inline_svg/webpack_asset_finder"
require "inline_svg/transform_pipeline"
Expand All @@ -19,7 +22,15 @@ module InlineSvg
class Configuration
class Invalid < ArgumentError; end

attr_reader :asset_file, :asset_finder, :custom_transformations, :svg_not_found_css_class
attr_reader :asset_file, :custom_transformations, :svg_not_found_css_class

# Asset finders are ordered by priority. Unless configured otherwise, the
# first asset finder that matches will be used.
ASSET_FINDERS = [
InlineSvg::SprocketsAssetFinder,
InlineSvg::PropshaftAssetFinder,
InlineSvg::StaticAssetFinder
].freeze

def initialize
@custom_transformations = {}
Expand All @@ -41,18 +52,16 @@ def asset_file=(custom_asset_file)
end
end

def asset_finder=(finder)
@asset_finder = if finder.respond_to?(:find_asset)
finder
elsif finder.class.name == "Propshaft::Assembly"
InlineSvg::PropshaftAssetFinder
else
# fallback to a naive static asset finder
# (sprokects >= 3.0 && config.assets.precompile = false
# See: https://github.com/jamesmartin/inline_svg/issues/25
InlineSvg::StaticAssetFinder
end
asset_finder
def asset_finder=(asset_finder)
if asset_finder.respond_to?(:find_asset)
@asset_finder = asset_finder
else
raise InlineSvg::Configuration::Invalid.new("asset_finder should implement the #find_asset method")
end
end

def asset_finder
@asset_finder ||= matching_asset_finder
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here, we could raise if no asset finder is set or detected.

end

def svg_not_found_css_class=(css_class)
Expand All @@ -68,9 +77,7 @@ def add_custom_transformation(options)
@custom_transformations.merge!(Hash[ *[options.fetch(:attribute, :no_attribute), options] ])
end

def raise_on_file_not_found=(value)
@raise_on_file_not_found = value
end
attr_writer :raise_on_file_not_found

def raise_on_file_not_found?
!!@raise_on_file_not_found
Expand All @@ -82,6 +89,13 @@ def incompatible_transformation?(klass)
!klass.is_a?(Class) || !klass.respond_to?(:create_with_value) || !klass.instance_methods.include?(:transform)
end

def matching_asset_finder
ASSET_FINDERS.find do |klass|
asset_finder = klass.new
break asset_finder if asset_finder.match?
rescue NameError
end
end
end

@configuration = InlineSvg::Configuration.new
Expand Down
35 changes: 28 additions & 7 deletions lib/inline_svg/propshaft_asset_finder.rb
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The goal is to have these classes behave like lazy, auto-detect adapters. Also, to draw a line between the asset finder and the asset result.

Original file line number Diff line number Diff line change
@@ -1,16 +1,37 @@
module InlineSvg
class PropshaftAssetFinder
def self.find_asset(filename)
new(filename)
class Asset
attr_reader :asset_finder, :filename

def initialize(filename, asset_finder)
@asset_finder = asset_finder
@filename = filename
end

delegate :assets, to: :asset_finder

def pathname
asset_path = assets.load_path.find(@filename)
asset_path&.path
end
end

attr_reader :assets

def initialize(assets = ::Rails.application.assets)
@assets = assets
end

class << self
delegate :find_asset, to: :new
end

def initialize(filename)
@filename = filename
def find_asset(filename)
Asset.new(filename, self)
end

def pathname
asset_path = ::Rails.application.assets.load_path.find(@filename)
asset_path.path unless asset_path.nil?
def match?
defined?(::Propshaft) && assets.instance_of?(Propshaft::Assembly)
end
end
end
13 changes: 0 additions & 13 deletions lib/inline_svg/railtie.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,5 @@ class Railtie < ::Rails::Railtie
include InlineSvg::ActionView::Helpers
end
end

config.after_initialize do |app|
InlineSvg.configure do |config|
# Configure the asset_finder:
# Only set this when a user-configured asset finder has not been
# configured already.
if config.asset_finder.nil?
# In default Rails apps, this will be a fully operational
# Sprockets::Environment instance
config.asset_finder = app.instance_variable_get(:@assets)
end
end
end
end
end
19 changes: 19 additions & 0 deletions lib/inline_svg/sprockets_asset_finder.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
module InlineSvg
class SprocketsAssetFinder
attr_reader :assets

def initialize(assets = ::Rails.application.assets)
@assets = assets
end

class << self
delegate :find_asset, to: :new
end

delegate :find_asset, to: :assets

def match?
assets.respond_to?(:find_asset)
end
end
end
42 changes: 27 additions & 15 deletions lib/inline_svg/static_asset_finder.rb
Copy link
Contributor Author

@xymbol xymbol Oct 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could name this "Rails static asset finder" as it depends on a Rails configuration, i.e. it cannot work stand-alone or with a non-Rails environment.

Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,37 @@
# https://github.com/jamesmartin/inline_svg/commit/661bbb3bef7d1b4bd6ccd63f5f018305797b9509
module InlineSvg
class StaticAssetFinder
def self.find_asset(filename)
new(filename)
end
class Asset
attr_reader :filename

def initialize(filename)
@filename = filename
end
def initialize(filename, _asset_finder)
@filename = filename
end

def pathname
if ::Rails.application.config.assets.compile
asset = ::Rails.application.assets[@filename]
Pathname.new(asset.filename) if asset.present?
else
manifest = ::Rails.application.assets_manifest
asset_path = manifest.assets[@filename]
unless asset_path.nil?
::Rails.root.join(manifest.directory, asset_path)
def pathname
if ::Rails.application.config.assets.compile
asset = ::Rails.application.assets[@filename]
Pathname.new(asset.filename) if asset.present?
else
manifest = ::Rails.application.assets_manifest
asset_path = manifest.assets[@filename]
unless asset_path.nil?
::Rails.root.join(manifest.directory, asset_path)
end
end
end
end

class << self
delegate :find_asset, to: :new
end

def find_asset(filename)
Asset.new(filename, self)
end

def match?
defined?(::Rails)
end
end
end
6 changes: 3 additions & 3 deletions lib/inline_svg/webpack_asset_finder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ def self.find_asset(filename)
def initialize(filename)
@filename = filename
manifest_lookup = Webpacker.manifest.lookup(@filename)
@asset_path = manifest_lookup.present? ? URI(manifest_lookup).path : ""
@asset_path = manifest_lookup.present? ? URI(manifest_lookup).path : ""
end

def pathname
Expand All @@ -31,7 +31,7 @@ def dev_server_asset(file_path)
file.write(asset)
file.rewind
end
rescue StandardError => e
rescue => e
Rails.logger.error "[inline_svg] Error creating tempfile for #{@filename}: #{e}"
raise
end
Expand All @@ -43,7 +43,7 @@ def fetch_from_dev_server(file_path)
http.verify_mode = OpenSSL::SSL::VERIFY_NONE

http.request(Net::HTTP::Get.new(file_path)).body
rescue StandardError => e
rescue => e
Rails.logger.error "[inline_svg] Error fetching #{@filename} from webpack-dev-server: #{e}"
raise
end
Expand Down
50 changes: 41 additions & 9 deletions spec/inline_svg_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,21 +28,54 @@ def self.named(filename); end
end

context "asset finder" do
it "allows an asset finder to be assigned" do
sprockets = double('SomethingLikeSprockets', find_asset: 'some asset')
InlineSvg.configure do |config|
config.asset_finder = sprockets
context "when Sprockets is detected" do
it "uses the Sprockets asset finder" do
sprockets = double("Sprockets", find_asset: "some asset")
stub_const("Rails", double("Rails"))
allow(Rails).to receive_message_chain(:application, :assets).and_return(sprockets)

expect(InlineSvg.configuration.asset_finder).to be_a InlineSvg::SprocketsAssetFinder
end
end

context "when Propshaft is detected" do
it "uses the Propshaft asset finder" do
stub_const("Propshaft::Assembly", Class.new)
stub_const("Rails", double("Rails"))
allow(Rails).to receive_message_chain(:application, :assets).and_return(Propshaft::Assembly.new)

expect(InlineSvg.configuration.asset_finder).to be_a InlineSvg::PropshaftAssetFinder
end
end

context "when Sprockets and Propshaft are not detected" do
it "uses the static asset finder" do
stub_const("Rails", double("Rails"))
allow(Rails).to receive_message_chain(:application, :assets).and_return(nil)

expect(InlineSvg.configuration.asset_finder).to eq sprockets
expect(InlineSvg.configuration.asset_finder).to be_a InlineSvg::StaticAssetFinder
end
end

it "falls back to StaticAssetFinder when the provided asset finder does not implement #find_asset" do
it "allows an asset finder to be assigned" do
asset_finder = double("An asset finder", find_asset: "some asset")
InlineSvg.configure do |config|
config.asset_finder = 'Not a real asset finder'
config.asset_finder = asset_finder
end

expect(InlineSvg.configuration.asset_finder).to eq InlineSvg::StaticAssetFinder
expect(InlineSvg.configuration.asset_finder).to eq asset_finder
end

it "raises an error if the asset finder does not implement the find_asset method" do
expect {
InlineSvg.configure do |config|
config.asset_finder = "Not a real asset finder"
end
}.to raise_error(InlineSvg::Configuration::Invalid, /asset_finder should implement the #find_asset method/)
end

after do
InlineSvg.reset_configuration!
end
end

Expand Down Expand Up @@ -122,7 +155,6 @@ def self.named(filename); end
end
end.to raise_error(InlineSvg::Configuration::Invalid, /#{:not_a_class} should implement the .create_with_value and #transform methods/)
end

end
end
end
11 changes: 11 additions & 0 deletions spec/sprockets_asset_finder_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
require_relative "../lib/inline_svg"

describe InlineSvg::SprocketsAssetFinder do
it "returns assets from Sprockets" do
sprockets = double("Sprockets", find_asset: "some asset")
stub_const("Rails", double("Rails"))
allow(Rails).to receive_message_chain(:application, :assets).and_return(sprockets)

expect(described_class.find_asset("some-file")).to eq "some asset"
end
end