From 78b0f58e7b802b774a7b154d4e08a15d51f21d6d Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Mon, 22 Nov 2021 19:38:50 +0100 Subject: [PATCH 1/3] Fix sourcemapping url replacement --- .../rails/sourcemapping_url_processor.rb | 52 +++++++++++++++---- test/test_sourcemapping_url_processor.rb | 11 ++-- 2 files changed, 47 insertions(+), 16 deletions(-) diff --git a/lib/sprockets/rails/sourcemapping_url_processor.rb b/lib/sprockets/rails/sourcemapping_url_processor.rb index 2792f7be..70301882 100644 --- a/lib/sprockets/rails/sourcemapping_url_processor.rb +++ b/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 + sourcemap_comment(sourcemap_logical_path, context) + rescue Sprockets::FileNotFound + sourcemap_comment_removed(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 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 sourcemap_comment_removed(sourcemap_logical_path, filename:, env:) + env.logger.warn "Removed sourceMappingURL comment for missing asset '#{sourcemap_logical_path}' from #{filename}" + nil + end end end end diff --git a/test/test_sourcemapping_url_processor.rb b/test/test_sourcemapping_url_processor.rb index a35d04b2..6b4c0c5e 100644 --- a/test/test_sourcemapping_url_processor.rb +++ b/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,17 @@ 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", 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//# sourceMappingURL=/assets/mapped-HEXGOESHERE.js.map\n//!\n" }, output) end def test_missing @@ -31,7 +30,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 From 82f5ba1c7966fce11b47344fc7e8a52551807865 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Mon, 22 Nov 2021 19:40:40 +0100 Subject: [PATCH 2/3] Improve naming --- lib/sprockets/rails/sourcemapping_url_processor.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/sprockets/rails/sourcemapping_url_processor.rb b/lib/sprockets/rails/sourcemapping_url_processor.rb index 70301882..fe8a29cc 100644 --- a/lib/sprockets/rails/sourcemapping_url_processor.rb +++ b/lib/sprockets/rails/sourcemapping_url_processor.rb @@ -12,9 +12,9 @@ def call(input) sourcemap_logical_path = combine_sourcemap_logical_path(sourcefile: input[:name], sourcemap: $1) begin - sourcemap_comment(sourcemap_logical_path, context) + resolved_sourcemap_comment(sourcemap_logical_path, context: context) rescue Sprockets::FileNotFound - sourcemap_comment_removed(sourcemap_logical_path, filename: input[:filename], env: env) + removed_sourcemap_comment(sourcemap_logical_path, filename: input[:filename], env: env) end end @@ -30,7 +30,7 @@ def combine_sourcemap_logical_path(sourcefile:, sourcemap:) end end - def sourcemap_comment(sourcemap_logical_path, context) + def resolved_sourcemap_comment(sourcemap_logical_path, context:) "//# sourceMappingURL=#{sourcemap_asset_path(sourcemap_logical_path, context: context)}\n//!\n" end @@ -44,7 +44,7 @@ def sourcemap_asset_path(sourcemap_logical_path, context:) end end - def sourcemap_comment_removed(sourcemap_logical_path, filename:, env:) + 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 From 59179bd2f4563b79ba1461ced50a190dde224a1d Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Mon, 22 Nov 2021 20:15:45 +0100 Subject: [PATCH 3/3] Add test for dealing with the deep nest issue --- test/test_sourcemapping_url_processor.rb | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/test_sourcemapping_url_processor.rb b/test/test_sourcemapping_url_processor.rb index 6b4c0c5e..ead7c33a 100644 --- a/test/test_sourcemapping_url_processor.rb +++ b/test/test_sourcemapping_url_processor.rb @@ -23,6 +23,18 @@ def asset_path(path, options = {}) 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", name: 'mapped', filename: 'mapped.js', metadata: {} } + output = Sprockets::Rails::SourcemappingUrlProcessor.call(input) + assert_equal({ data: "var mapped;\n" }, output) + end + def test_missing @env.context_class.class_eval do def resolve(path, **kargs)