From 463a5024afdc39e4074f18c3ee008b50d43a6544 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Mon, 20 May 2024 16:02:20 -0700 Subject: [PATCH] Provide :all and :app helpers for stylesheet references via cached methods (#190) * 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 --- lib/propshaft/helper.rb | 21 +++++++++------- lib/propshaft/load_path.rb | 24 ++++++++++++++----- .../app/views/layouts/application.html.erb | 1 + test/dummy/lib/assets/stylesheets/library.css | 1 + test/propshaft/load_path_test.rb | 19 +++++++++------ test/propshaft_integration_test.rb | 9 +++++++ 6 files changed, 54 insertions(+), 21 deletions(-) create mode 100644 test/dummy/lib/assets/stylesheets/library.css diff --git a/lib/propshaft/helper.rb b/lib/propshaft/helper.rb index 66e8d25..f84427f 100644 --- a/lib/propshaft/helper.rb +++ b/lib/propshaft/helper.rb @@ -4,10 +4,14 @@ 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 @@ -15,11 +19,12 @@ def stylesheet_link_tag(*sources, **options) # 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 index 6abdbe2..801be23 100644 --- 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 index c412d08..db2255c 100644 --- 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 } %> diff --git a/test/dummy/lib/assets/stylesheets/library.css b/test/dummy/lib/assets/stylesheets/library.css new file mode 100644 index 0000000..543e3ec --- /dev/null +++ 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 index 4d52376..3284f76 100644 --- 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 index 554303d..b2eeecc 100644 --- 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