Merge remote-tracking branch 'upstream/main' into fix-missing-sourcem…

…ap-with-esbuild-public-path

UPGRADING.md

@@ -207,7 +207,7 @@ Start by following these steps:
 1. Remove `sprockets`, `sprockets-rails`, and `sass-rails` from the Gemfile and add `propshaft`;
 2. Run `./bin/bundle install`;
 3. Open `config/application.rb` and remove `config.assets.paths << Rails.root.join('app','assets')`;
-4. Remove `asset/config/manifest.js`.
+4. Remove `app/assets/config/manifest.js`.
 5. Replace all asset_helpers (`image_url`, `font_url`) in css files with standard `urls`.
 6. If you are importing only the frameworks you need (instead of `rails/all`), remove `require "sprockets/railtie"`;
 

lib/propshaft/compiler/source_mapping_urls.rb

@@ -3,10 +3,10 @@
 require "propshaft/compiler"
 
 class Propshaft::Compiler::SourceMappingUrls < Propshaft::Compiler
-  SOURCE_MAPPING_PATTERN = %r{^(//|/\*)# sourceMappingURL=(?:.*\/)?(.*\.map)}
+  SOURCE_MAPPING_PATTERN = %r{(//|/\*)# sourceMappingURL=(?:.*\/)?(.*\.map)(\s*?\*\/)?\s*?\Z}
 
   def compile(logical_path, input)
-    input.gsub(SOURCE_MAPPING_PATTERN) { source_mapping_url(asset_path($2, logical_path), $1) }
+    input.gsub(SOURCE_MAPPING_PATTERN) { source_mapping_url(asset_path($2, logical_path), $1, $3) }
   end
 
   private
@@ -18,12 +18,12 @@ def asset_path(source_mapping_url, logical_path)
       end
     end
 
-    def source_mapping_url(resolved_path, comment)
+    def source_mapping_url(resolved_path, comment_start, comment_end)
       if asset = assembly.load_path.find(resolved_path)
-        "#{comment}# sourceMappingURL=#{url_prefix}/#{asset.digested_path}"
+        "#{comment_start}# sourceMappingURL=#{url_prefix}/#{asset.digested_path}#{comment_end}"
       else
         Propshaft.logger.warn "Removed sourceMappingURL comment for missing asset '#{resolved_path}' from #{resolved_path}"
-        comment
+        "#{comment_start}#{comment_end}"
       end
     end
 end

lib/propshaft/helper.rb

@@ -3,5 +3,23 @@ module Helper
     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.
+    def stylesheet_link_tag(*sources)
+      if sources.first == :all
+        super *all_stylesheets_paths
+      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
+    end
   end
 end

lib/propshaft/server.rb

@@ -11,19 +11,19 @@ def call(env)
     if (asset = @assembly.load_path.find(path)) && asset.fresh?(digest)
       compiled_content = @assembly.compilers.compile(asset)
 
-      [ 
-        200, 
+      [
+        200,
         {
-          "content-length"  => compiled_content.length.to_s,
-          "content-type"    => asset.content_type.to_s,
-          "vary"            => "Accept-Encoding",
-          "etag"            => asset.digest,
-          "cache-control"   => "public, max-age=31536000, immutable"
+          Rack::CONTENT_LENGTH  => compiled_content.length.to_s,
+          Rack::CONTENT_TYPE    => asset.content_type.to_s,
+          VARY                  => "Accept-Encoding",
+          Rack::ETAG            => asset.digest,
+          Rack::CACHE_CONTROL   => "public, max-age=31536000, immutable"
         },
         [ compiled_content ]
       ]
     else
-      [ 404, { "content-type" => "text/plain", "content-length" => "9" }, [ "Not found" ] ]
+      [ 404, { Rack::CONTENT_TYPE => "text/plain", Rack::CONTENT_LENGTH => "9" }, [ "Not found" ] ]
     end
   end
 
@@ -39,4 +39,10 @@ def extract_path_and_digest(env)
 
       [ path, digest ]
     end
+
+    if Gem::Version.new(Rack::RELEASE) < Gem::Version.new("3")
+      VARY = "Vary"
+    else
+      VARY = "vary"
+    end
 end

test/dummy/app/assets/stylesheets/goodbye.css

@@ -0,0 +1,3 @@
+h2:after {
+  content: "Goodbye!";
+}

test/dummy/app/views/layouts/application.html.erb

@@ -5,7 +5,7 @@
     <%= csrf_meta_tags %>
     <%= csp_meta_tag %>
 
-    <%= stylesheet_link_tag "application" %>
+    <%= stylesheet_link_tag :all %>
   </head>
 
   <body>

test/fixtures/assets/mapped/sourceMappingURL-not-at-end.css

@@ -0,0 +1,4 @@
+.class {
+/*# sourceMappingURL=sourceMappingURL-not-at-end.css.map */
+  color: green;
+}

test/fixtures/assets/mapped/sourceMappingURL-not-at-start.css

@@ -1,3 +1,4 @@
-.class {
-  color: green; /*# sourceMappingURL=sourceMappingURL-not-at-start.css.map */
-}
+.class{color:green;}/*# sourceMappingURL=sourceMappingURL-not-at-start.css.map */
+
+
+

test/fixtures/assets/mapped/sourceMappingURL-not-at-start.js

@@ -0,0 +1,2 @@
+var fun; //# sourceMappingURL=sourceMappingURL-not-at-start.js.map
+

test/propshaft/compiler/source_mapping_urls_test.rb

@@ -42,16 +42,22 @@ class Propshaft::Compiler::SourceMappingUrlsTest < ActiveSupport::TestCase
                  compile_asset(find_asset("sourceless.css", fixture_path: "mapped"))
   end
 
+  test "sourceMappingURL not at the beginning of the line, but at end of file, is processed" do
+    assert_match %r{//# sourceMappingURL=/assets/sourceMappingURL-not-at-start.js-[a-z0-9]{40}\.map},
+                 compile_asset(find_asset("sourceMappingURL-not-at-start.js", fixture_path: "mapped"))
+    assert_match %r{/\*# sourceMappingURL=/assets/sourceMappingURL-not-at-start.css-[a-z0-9]{40}\.map \*/},
+                 compile_asset(find_asset("sourceMappingURL-not-at-start.css", fixture_path: "mapped"))
+  end
+
+  test "sourceMappingURL not at end of file should be left alone" do
+    assert_match %r{sourceMappingURL=sourceMappingURL-not-at-end.css.map},
+                 compile_asset(find_asset("sourceMappingURL-not-at-end.css", fixture_path: "mapped"))
+  end
   test "sourceMappingURL outside of a comment should be left alone" do
     assert_match %r{sourceMappingURL=sourceMappingURL-outside-comment.css.map},
                  compile_asset(find_asset("sourceMappingURL-outside-comment.css", fixture_path: "mapped"))
   end
 
-  test "sourceMappingURL not at the beginning of the line should be left alone" do
-    assert_match %r{sourceMappingURL=sourceMappingURL-not-at-start.css.map},
-                 compile_asset(find_asset("sourceMappingURL-not-at-start.css", fixture_path: "mapped"))
-  end
-
   test "relative url root" do
     @options.relative_url_root = "/url-root"
 

test/propshaft_integration_test.rb

@@ -3,8 +3,12 @@
 class PropshaftIntegrationTest < ActionDispatch::IntegrationTest
   test "should be able to resolve real assets" do
     get sample_load_real_assets_url
+
     assert_response :success
+
     assert_select 'link[href="/assets/hello_world-4137140a1298c3924d5f7135617c23e23fb167a8.css"]'
+    assert_select 'link[href="/assets/goodbye-b1dc9940e9800d8bc96f7434617c043e58277419.css"]'
+
     assert_select 'script[src="/assets/hello_world-888761f849ba63a95a56f6ef898a9eb70ca4c46e.js"]'
   end