Fix sourcemapping url replacement (#484)

* Fix sourcemapping url replacement

* Improve naming

* Add test for dealing with the deep nest issue

lib/sprockets/rails/sourcemapping_url_processor.rb

@@ -4,18 +4,50 @@ module Rails
     class SourcemappingUrlProcessor
       REGEX = /\/\/# sourceMappingURL=(.*\.map)/
 
-      def self.call(input)
-        env     = input[:environment]
-        context = env.context_class.new(input)
-        data    = input[:data].gsub(REGEX) do |_match|
-          context.resolve($1) # Ensure file is present
-          "//# sourceMappingURL=#{context.asset_path($1)}\n//!\n"
-        rescue Sprockets::FileNotFound
-          env.logger.warn "Removed sourceMappingURL comment for missing asset '#{$1}' from #{input[:filename]}"
-          nil
+      class << self
+        def call(input)
+          env     = input[:environment]
+          context = env.context_class.new(input)
+          data    = input[:data].gsub(REGEX) do |_match|
+            sourcemap_logical_path = combine_sourcemap_logical_path(sourcefile: input[:name], sourcemap: $1)
+
+            begin
+              resolved_sourcemap_comment(sourcemap_logical_path, context: context)
+            rescue Sprockets::FileNotFound
+              removed_sourcemap_comment(sourcemap_logical_path, filename: input[:filename], env: env)
+            end
+          end
+
+          { data: data }
         end
 
-        { data: data }
+        private
+          def combine_sourcemap_logical_path(sourcefile:, sourcemap:)
+            if (parts = sourcefile.split("/")).many?
+              parts[0..-2].append(sourcemap).join("/")
+            else
+              sourcemap
+            end
+          end
+
+          def resolved_sourcemap_comment(sourcemap_logical_path, context:)
+            "//# sourceMappingURL=#{sourcemap_asset_path(sourcemap_logical_path, context: context)}\n//!\n"
+          end
+
+          def sourcemap_asset_path(sourcemap_logical_path, context:)
+            # FIXME: Work-around for bug where if the sourcemap is nested two levels deep, it'll resolve as the source file
+            # that's being mapped, rather than the map itself. So context.resolve("a/b/c.js.map") will return "c.js?"
+            if context.resolve(sourcemap_logical_path) =~ /\.map/
+              context.asset_path(sourcemap_logical_path)
+            else
+              raise Sprockets::FileNotFound, "Failed to resolve source map asset due to nesting depth"
+            end
+          end
+
+          def removed_sourcemap_comment(sourcemap_logical_path, filename:, env:)
+            env.logger.warn "Removed sourceMappingURL comment for missing asset '#{sourcemap_logical_path}' from #{filename}"
+            nil
+          end
       end
     end
   end

test/test_sourcemapping_url_processor.rb

@@ -1,7 +1,6 @@
 require 'minitest/autorun'
 require 'sprockets/railtie'
 
-
 Minitest::Test = MiniTest::Unit::TestCase unless defined?(Minitest::Test)
 class TestSourceMappingUrlProcessor < Minitest::Test
   def setup
@@ -11,17 +10,29 @@ def setup
   def test_successful
     @env.context_class.class_eval do
       def resolve(path, **kargs)
-        "/yes"
+        "/assets/mapped.js.map"
       end
 
       def asset_path(path, options = {})
-        'mapped-HEXGOESHERE.js.map'
+        "/assets/mapped-HEXGOESHERE.js.map"
+      end
+    end
+
+    input = { environment: @env, data: "var mapped;\n//# sourceMappingURL=mapped.js.map", name: 'mapped', filename: 'mapped.js', metadata: {} }
+    output = Sprockets::Rails::SourcemappingUrlProcessor.call(input)
+    assert_equal({ data: "var mapped;\n//# sourceMappingURL=/assets/mapped-HEXGOESHERE.js.map\n//!\n" }, output)
+  end
+
+  def test_resolving_erroneously_without_map_extension
+    @env.context_class.class_eval do
+      def resolve(path, **kargs)
+        "/assets/mapped.js"
       end
     end
 
-    input = { environment: @env, data: "var mapped;\n//# sourceMappingURL=mapped.js.map", filename: 'mapped.js', metadata: {} }
+    input = { environment: @env, data: "var mapped;\n//# sourceMappingURL=mapped.js.map", name: 'mapped', filename: 'mapped.js', metadata: {} }
     output = Sprockets::Rails::SourcemappingUrlProcessor.call(input)
-    assert_equal({ data: "var mapped;\n//# sourceMappingURL=mapped-HEXGOESHERE.js.map\n//!\n" }, output)
+    assert_equal({ data: "var mapped;\n" }, output)
   end
 
   def test_missing
@@ -31,7 +42,7 @@ def resolve(path, **kargs)
       end
     end
 
-    input = { environment: @env, data: "var mapped;\n//# sourceMappingURL=mappedNOT.js.map", filename: 'mapped.js', metadata: {} }
+    input = { environment: @env, data: "var mapped;\n//# sourceMappingURL=mappedNOT.js.map", name: 'mapped', filename: 'mapped.js', metadata: {} }
     output = Sprockets::Rails::SourcemappingUrlProcessor.call(input)
     assert_equal({ data: "var mapped;\n" }, output)
   end