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.

Sign up for GitHub

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

Provide :all and :app helpers for stylesheet references via cached methods #190

Merged
merged 6 commits into from
May 20, 2024
Merged

Provide :all and :app helpers for stylesheet references via cached methods #190

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
39d8ec3
Cache asset paths by methods
dhh May 19, 2024
f6b636e
Fix references
dhh May 19, 2024
ea85d4c
Add test for app globing
dhh May 19, 2024
73f75c0
Ensure we only glob from the app root
dhh May 20, 2024
6998e18
Test that :app really only pulls from app/assets
dhh May 20, 2024
3992e55
Document :app for helper
dhh May 20, 2024
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
21 changes: 13 additions & 8 deletions lib/propshaft/helper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,27 @@ def compute_asset_path(path, options = {})
Rails.application.assets.resolver.resolve(path) || raise(MissingAssetError.new(path))
end

# Add an option to call `stylesheet_link_tag` with `:all` to include every css file found on the load path.
# Add an option to call `stylesheet_link_tag` with `:all` to include every css file found on the load path
# or `:app` to include css files found in `Rails.root("app/assets/**/*.css")`, which will exclude lib/ and plugins.
def stylesheet_link_tag(*sources, **options)
if sources.first == :all
super(*all_stylesheets_paths, **options)
case sources.first
when :all
super(*all_stylesheets_paths , **options)
when :app
super(*app_stylesheets_paths , **options)
else
super
end
end

# Returns a sorted and unique array of logical paths for all stylesheets in the load path.
def all_stylesheets_paths
Rails.application.assets.load_path
.assets(content_types: [ Mime::EXTENSION_LOOKUP["css"] ])
.collect { |css| css.logical_path.to_s }
.sort
.uniq
Rails.application.assets.load_path.asset_paths_by_type("css")
end

# Returns a sorted and unique array of logical paths for all stylesheets in app/assets/**/*.css.
def app_stylesheets_paths
Rails.application.assets.load_path.asset_paths_by_glob("#{Rails.root.join("app/assets")}/**/*.css")
dhh marked this conversation as resolved.
Show resolved Hide resolved
end
end
end
24 changes: 18 additions & 6 deletions lib/propshaft/load_path.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,18 @@ def find_referenced_by(asset)
compilers.referenced_by(asset).delete(self)
end

def assets(content_types: nil)
if content_types
assets_by_path.values.select { |asset| asset.content_type.in?(content_types) }
else
assets_by_path.values
end
def assets
assets_by_path.values
end

def asset_paths_by_type(content_type)
(@cached_asset_paths_by_type ||= Hash.new)[content_type] ||=
extract_logical_paths_from(assets.select { |a| a.content_type == Mime::EXTENSION_LOOKUP[content_type] })
end

def asset_paths_by_glob(glob)
(@cached_asset_paths_by_glob ||= Hash.new)[glob] ||=
extract_logical_paths_from(assets.select { |a| a.path.fnmatch?(glob) })
end

def manifest
Expand Down Expand Up @@ -61,12 +67,18 @@ def all_files_from_tree(path)
path.children.flat_map { |child| child.directory? ? all_files_from_tree(child) : child }
end

def extract_logical_paths_from(assets)
assets.collect { |asset| asset.logical_path.to_s }.sort
end

def without_dotfiles(files)
files.reject { |file| file.basename.to_s.starts_with?(".") }
end

def clear_cache
@cached_assets_by_path = nil
@cached_asset_paths_by_type = nil
@cached_asset_paths_by_glob = nil
end

def dedup(paths)
Expand Down
1 change: 1 addition & 0 deletions test/dummy/app/views/layouts/application.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
<%= csp_meta_tag %>

<%= stylesheet_link_tag :all, data: { custom_attribute: true } %>
<%= stylesheet_link_tag :app, data: { glob_attribute: true } %>
</head>

<body>
Expand Down
1 change: 1 addition & 0 deletions test/dummy/lib/assets/stylesheets/library.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# Library css that should not be included by :app but should be included by :all
19 changes: 12 additions & 7 deletions test/propshaft/load_path_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,6 @@ class Propshaft::LoadPathTest < ActiveSupport::TestCase
assert_not_includes @load_path.assets, find_asset(".stuff")
end

test "assets by given content types" do
assert_not_includes @load_path.assets(content_types: [ Mime[:js] ]), find_asset("one.txt")
assert_includes @load_path.assets(content_types: [ Mime[:js] ]), find_asset("again.js")
assert_includes @load_path.assets(content_types: [ Mime[:js], Mime[:css] ]), find_asset("again.js")
assert_includes @load_path.assets(content_types: [ Mime[:js], Mime[:css] ]), find_asset("another.css")
end

test "manifest" do
@load_path.manifest.tap do |manifest|
assert_equal "one-f2e1ec14.txt", manifest["one.txt"]
Expand Down Expand Up @@ -70,6 +63,18 @@ class Propshaft::LoadPathTest < ActiveSupport::TestCase
assert_equal Pathname.new("app/assets"), paths.last
end

test "asset paths by type" do
assert_equal \
["another.css", "dependent/a.css", "dependent/b.css", "dependent/c.css", "file-already-abcdefVWXYZ0123456789_-.digested.css", "file-already-abcdefVWXYZ0123456789_-.digested.debug.css", "file-not.digested.css"],
@load_path.asset_paths_by_type("css")
end

test "asset paths by glob" do
assert_equal \
["dependent/a.css", "dependent/b.css", "dependent/c.css"],
@load_path.asset_paths_by_glob("**/dependent/*.css")
end

private
def find_asset(logical_path)
root_path = Pathname.new("#{__dir__}/../fixtures/assets/first_path")
Expand Down
9 changes: 9 additions & 0 deletions test/propshaft_integration_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,19 @@ class PropshaftIntegrationTest < ActionDispatch::IntegrationTest

assert_select 'link[href="/assets/hello_world-4137140a.css"][data-custom-attribute="true"]'
assert_select 'link[href="/assets/goodbye-b1dc9940.css"][data-custom-attribute="true"]'
assert_select 'link[href="/assets/library-86a3b7a9.css"][data-custom-attribute="true"]'

assert_select 'script[src="/assets/hello_world-888761f8.js"]'
end

test "should find app styles via glob" do
get sample_load_real_assets_url

assert_select 'link[href="/assets/hello_world-4137140a.css"][data-glob-attribute="true"]'
assert_select 'link[href="/assets/goodbye-b1dc9940.css"][data-glob-attribute="true"]'
assert_select('link[href="/assets/library-86a3b7a9.css"][data-glob-attribute="true"]', count: 0)
end

test "should raise an exception when resolving nonexistent assets" do
exception = assert_raises ActionView::Template::Error do
get sample_load_nonexistent_assets_url
Expand Down