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

Conversation

xymbol
Copy link
Contributor

@xymbol xymbol commented Oct 26, 2023

This commit changes the way the asset finder is configured. Instead of setting the asset finder from an initializer to a specific class, the asset finder is now detected unless configured otherwise.

This allows InlineSvg to work with Sprockets, Propshaft, or a fallback static asset finder without any configuration.

This commit changes the way the asset finder is configured. Instead of setting
the asset finder from an initializer to a specific class, the asset finder is
now detected unless configured otherwise.

This allows InlineSvg to work with Sprockets, Propshaft, or a fallback static
asset finder without any configuration.
It allows caching at the finder without class instance variables.
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.

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.

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.

That's for another refactor, but we can drop it now.
@jamesmartin jamesmartin deleted the branch jamesmartin:main September 30, 2024 07:32
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

Successfully merging this pull request may close these issues.

2 participants