From 0a56eec89d86d7ef36af439bad7df51adbc575c5 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Mon, 22 Nov 2021 20:26:39 +0100 Subject: [PATCH] Fix sourcemapping url replacement (#484) * Fix sourcemapping url replacement * Improve naming * Add test for dealing with the deep nest issue --- .../rails/sourcemapping_url_processor.rb | 52 +++++++++++++++---- test/test_sourcemapping_url_processor.rb | 23 +++++--- 2 files changed, 59 insertions(+), 16 deletions(-) diff --git a/lib/sprockets/rails/sourcemapping_url_processor.rb b/lib/sprockets/rails/sourcemapping_url_processor.rb index 2792f7be..fe8a29cc 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 + 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 diff --git a/test/test_sourcemapping_url_processor.rb b/test/test_sourcemapping_url_processor.rb index a35d04b2..ead7c33a 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,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