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

dhh · web-flow · commit 463a5024afdc · 2024-05-20T16:02:20.000-07:00
* Cache asset paths by methods

* Fix references

* Add test for app globing

* Ensure we only glob from the app root

* Test that :app really only pulls from app/assets

* Document :app for helper
diff --git a/lib/propshaft/helper.rb b/lib/propshaft/helper.rb
@@ -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")
     end
   end
 end
diff --git a/lib/propshaft/load_path.rb b/lib/propshaft/load_path.rb
@@ -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
@@ -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)
diff --git a/test/dummy/app/views/layouts/application.html.erb b/test/dummy/app/views/layouts/application.html.erb
@@ -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>
diff --git a/test/dummy/lib/assets/stylesheets/library.css b/test/dummy/lib/assets/stylesheets/library.css
@@ -0,0 +1 @@
+# Library css that should not be included by :app but should be included by :all
diff --git a/test/propshaft/load_path_test.rb b/test/propshaft/load_path_test.rb
@@ -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"]
@@ -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")
diff --git a/test/propshaft_integration_test.rb b/test/propshaft_integration_test.rb
@@ -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

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+# Library css that should not be included by :app but should be included by :all`