From 45f01e09591e78b6a85dd6ae8b734f897a4639f0 Mon Sep 17 00:00:00 2001 From: schneems Date: Fri, 10 Nov 2017 15:16:33 -0600 Subject: [PATCH 01/23] Default source map should come after Directive Processor The directive processor can add a newline to assets. If it comes after the default source map then this line is not accounted for when determining line offsets. --- lib/sprockets.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/sprockets.rb b/lib/sprockets.rb index fe9492203..c912aeffc 100644 --- a/lib/sprockets.rb +++ b/lib/sprockets.rb @@ -107,10 +107,6 @@ module Sprockets [SourceMapCommentProcessor] end - require 'sprockets/preprocessors/default_source_map' - register_preprocessor 'text/css', Preprocessors::DefaultSourceMap.new - register_preprocessor 'application/javascript', Preprocessors::DefaultSourceMap.new - require 'sprockets/directive_processor' register_preprocessor 'text/css', DirectiveProcessor.new(comments: ["//", ["/*", "*/"]]) register_preprocessor 'application/javascript', DirectiveProcessor.new(comments: ["//", ["/*", "*/"]]) @@ -219,4 +215,8 @@ module Sprockets depend_on 'environment-version' depend_on 'environment-paths' + + require 'sprockets/preprocessors/default_source_map' + register_preprocessor 'text/css', Preprocessors::DefaultSourceMap.new + register_preprocessor 'application/javascript', Preprocessors::DefaultSourceMap.new end From 1d28403ae21e79601583441abaf63c49a612fba7 Mon Sep 17 00:00:00 2001 From: schneems Date: Fri, 10 Nov 2017 15:20:59 -0600 Subject: [PATCH 02/23] [close #388] Do not add newline when appending Semi-colons Adding a new line with semi-colons can cause off by 1 errors with source maps. Instead we want to add a semicolon. If the last character was whitespace such as a newline, we want to make sure that there are the same number of `str.lines` before and after, so we replace the whitespace character with a semicolon followed by the same whitespace character. --- lib/sprockets/utils.rb | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/lib/sprockets/utils.rb b/lib/sprockets/utils.rb index ed54882e7..5e281c4f3 100644 --- a/lib/sprockets/utils.rb +++ b/lib/sprockets/utils.rb @@ -68,6 +68,9 @@ def hash_reassoc(hash, key_a, key_b = nil, &block) end end + WHITESPACE_ORDINALS = {0x0A => "\n", 0x20 => " ", 0x09 => "\t"} + private_constant :WHITESPACE_ORDINALS + # Internal: Check if string has a trailing semicolon. # # str - String @@ -79,14 +82,9 @@ def string_end_with_semicolon?(str) c = str[i].ord i -= 1 - # Need to compare against the ordinals because the string can be UTF_8 or UTF_32LE encoded - # 0x0A == "\n" - # 0x20 == " " - # 0x09 == "\t" - # 0x3B == ";" - unless c == 0x0A || c == 0x20 || c == 0x09 - return c === 0x3B - end + next if WHITESPACE_ORDINALS[c] + + return c === 0x3B end true @@ -100,18 +98,18 @@ def string_end_with_semicolon?(str) # # Returns buf String. def concat_javascript_sources(buf, source) - if source.bytesize > 0 - buf << source + return buf if source.bytesize < 0 - # If the source contains non-ASCII characters, indexing on it becomes O(N). - # This will lead to O(N^2) performance in string_end_with_semicolon?, so we should use 32 bit encoding to make sure indexing stays O(1) - source = source.encode(Encoding::UTF_32LE) unless source.ascii_only? + buf << source + # If the source contains non-ASCII characters, indexing on it becomes O(N). + # This will lead to O(N^2) performance in string_end_with_semicolon?, so we should use 32 bit encoding to make sure indexing stays O(1) + source = source.encode(Encoding::UTF_32LE) unless source.ascii_only? + return buf if string_end_with_semicolon?(source) - if !string_end_with_semicolon?(source) - buf << ";\n" - elsif source[source.size - 1].ord != 0x0A - buf << "\n" - end + if whitespace = WHITESPACE_ORDINALS[buf[-1].ord] + buf[-1] = ";#{whitespace}" + else + buf << ";" end buf From 870945b5c3742526c67162f9732408d689f5c28c Mon Sep 17 00:00:00 2001 From: schneems Date: Fri, 10 Nov 2017 15:37:07 -0600 Subject: [PATCH 03/23] Whitespace --- lib/sprockets/source_map_utils.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/sprockets/source_map_utils.rb b/lib/sprockets/source_map_utils.rb index 4e84164fb..4b5efbdc2 100644 --- a/lib/sprockets/source_map_utils.rb +++ b/lib/sprockets/source_map_utils.rb @@ -48,7 +48,7 @@ def format_source_map(map, input) "sources" => map["sources"].map do |source| source = URIUtils.split_file_uri(source)[2] if source.start_with? "file://" source = PathUtils.join(File.dirname(filename), source) unless PathUtils.absolute_path?(source) - _, source = PathUtils.paths_split(load_paths, source) + _, source = PathUtils.paths_split(load_paths, source) source = PathUtils.relative_path_from(file, source) PathUtils.set_pipeline(source, mime_exts, pipeline_exts, :source) end, From 11217f99691f630f2ef74c883c6bd55ea03c6659 Mon Sep 17 00:00:00 2001 From: schneems Date: Fri, 10 Nov 2017 15:43:13 -0600 Subject: [PATCH 04/23] Fix source map concatenation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The calculation of length of asset is done by counting semicolons, however not all mappings end with a semicolon. When they do we must increment the offset. The next part is harder to explain. We concatenate two assets A.js and B.js. A has 10 lines B has 5 lines. After we’ve done this we want an offset that looks like this: ``` A.js: {"line"=>0, "column"=>0} B.js: {"line"=>11, "column"=>0} ``` To get 11 we take the prior length of mappings for A.js which is 10 (from the lines) and increment by one to get to 11. Now we add another asset C.js which has 2 lines. We want an offset that looks like this ``` A.js: {"line"=>0, "column"=>0} B.js: {"line"=>11, "column"=>0} C.js: {"line"=>16, "column"=>0} ``` If we add the last offset which is 11, to the length of the asset of B (which is 5) then we get the correct offset which is 16. We do not have to add an extra +1. I think this was accidentally working previously based on unwanted behavior from adding semi-colons and newlines. --- lib/sprockets/source_map_utils.rb | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/lib/sprockets/source_map_utils.rb b/lib/sprockets/source_map_utils.rb index 4b5efbdc2..6fc3e124c 100644 --- a/lib/sprockets/source_map_utils.rb +++ b/lib/sprockets/source_map_utils.rb @@ -72,13 +72,25 @@ def format_source_map(map, input) # Returns a new source map hash. def concat_source_maps(a, b) return a || b unless a && b - a, b = make_index_map(a), make_index_map(b) - - if a["sections"].count == 0 || a["sections"].last["map"]["mappings"].empty? - offset = 0 - else - offset = a["sections"].last["map"]["mappings"].count(';') + - a["sections"].last["offset"]["line"] + 1 + a = make_index_map(a) + b = make_index_map(b) + + offset = 0 + if a["sections"].count != 0 && !a["sections"].last["map"]["mappings"].empty? + # Account for length of last asset + # Count the different VLQ sections such as "AACA;" by counting commas + # mappings do not always end with a semicolon so we check for that + # and increment + last_mappings = a["sections"].last["map"]["mappings"] + offset += last_mappings.count(';') + offset += 1 if last_mappings[-1] != ";" + + last_offset = a["sections"].last["offset"]["line"] + if last_offset > 0 + offset += last_offset + else + offset += 1 + end end a["sections"] += b["sections"].map do |section| From 66c2bd7d1fe3ec7bb2d1b6ee463f1546b1a78d40 Mon Sep 17 00:00:00 2001 From: schneems Date: Mon, 13 Nov 2017 09:40:38 -0600 Subject: [PATCH 05/23] Preserve original map if available This patch preserves an original source maps if one was present. --- lib/sprockets/preprocessors/default_source_map.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/sprockets/preprocessors/default_source_map.rb b/lib/sprockets/preprocessors/default_source_map.rb index fd4157095..271ded8cf 100644 --- a/lib/sprockets/preprocessors/default_source_map.rb +++ b/lib/sprockets/preprocessors/default_source_map.rb @@ -25,6 +25,8 @@ def call(input) "sources" => [PathUtils.set_pipeline(basename, mime_exts, pipeline_exts, :source)], "names" => [] } + else + result[:map] = map end return result end From e146451de22642cb3f09a0f7e6e2c1b9b3decf18 Mon Sep 17 00:00:00 2001 From: schneems Date: Mon, 13 Nov 2017 09:41:01 -0600 Subject: [PATCH 06/23] Length is faster than count MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ``` Warming up -------------------------------------- count 317.318k i/100ms length 329.512k i/100ms size 329.953k i/100ms Calculating ------------------------------------- count 9.806M (± 7.7%) i/s - 48.867M in 5.014627s length 11.696M (± 6.3%) i/s - 58.324M in 5.008040s size 11.074M (± 7.5%) i/s - 55.102M in 5.005626s ``` --- lib/sprockets/preprocessors/default_source_map.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/sprockets/preprocessors/default_source_map.rb b/lib/sprockets/preprocessors/default_source_map.rb index 271ded8cf..a051bdbf8 100644 --- a/lib/sprockets/preprocessors/default_source_map.rb +++ b/lib/sprockets/preprocessors/default_source_map.rb @@ -13,7 +13,7 @@ def call(input) map = input[:metadata][:map] filename = input[:filename] load_path = input[:load_path] - lines = input[:data].lines.count + lines = input[:data].lines.length basename = File.basename(filename) mime_exts = input[:environment].config[:mime_exts] pipeline_exts = input[:environment].config[:pipeline_exts] From ff5b6dd89b655da065f36b4c8d98f9c4daa6a8f5 Mon Sep 17 00:00:00 2001 From: schneems Date: Mon, 13 Nov 2017 09:41:35 -0600 Subject: [PATCH 07/23] =?UTF-8?q?Only=20=E2=80=9Creduce=E2=80=9D=20source?= =?UTF-8?q?=20maps=20for=20css/js?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/sprockets.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/sprockets.rb b/lib/sprockets.rb index c912aeffc..4530a509d 100644 --- a/lib/sprockets.rb +++ b/lib/sprockets.rb @@ -119,7 +119,6 @@ module Sprockets register_bundle_metadata_reducer 'application/javascript', :data, proc { String.new("") }, Utils.method(:concat_javascript_sources) register_bundle_metadata_reducer '*/*', :links, :+ register_bundle_metadata_reducer '*/*', :sources, proc { [] }, :+ - register_bundle_metadata_reducer '*/*', :map, proc { |input| { "version" => 3, "file" => PathUtils.split_subpath(input[:load_path], input[:filename]), "sections" => [] } }, SourceMapUtils.method(:concat_source_maps) require 'sprockets/closure_compressor' require 'sprockets/sass_compressor' @@ -217,6 +216,9 @@ module Sprockets depend_on 'environment-paths' require 'sprockets/preprocessors/default_source_map' - register_preprocessor 'text/css', Preprocessors::DefaultSourceMap.new + register_preprocessor 'text/css', Preprocessors::DefaultSourceMap.new register_preprocessor 'application/javascript', Preprocessors::DefaultSourceMap.new + + register_bundle_metadata_reducer 'text/css', :map, proc { |input| { "version" => 3, "file" => PathUtils.split_subpath(input[:load_path], input[:filename]), "sections" => [] } }, SourceMapUtils.method(:concat_source_maps) + register_bundle_metadata_reducer 'application/javascript', :map, proc { |input| { "version" => 3, "file" => PathUtils.split_subpath(input[:load_path], input[:filename]), "sections" => [] } }, SourceMapUtils.method(:concat_source_maps) end From 033c6b693b2cd6e0a61735f781409d35c57b1f7e Mon Sep 17 00:00:00 2001 From: schneems Date: Mon, 13 Nov 2017 09:47:15 -0600 Subject: [PATCH 08/23] Cannot rely on semicolons in mappings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Based on the spec it is not guaranteed that there will be a mapping for each and every line https://groups.google.com/forum/#!topic/mozilla.dev.js-sourcemap/gp_ULp-h1fQ We were previously using this behavior to know the length of a previous asset when concatenating source maps. We can get around this by storing the length of the previous asset in an “x_sprockets_linecount” key. This is guaranteed to be on the asset since it is added by the `DefaultSourceMap`. We no longer need to offset the first asset by one line. This was accidentally working before. --- lib/sprockets/preprocessors/default_source_map.rb | 4 +++- lib/sprockets/source_map_utils.rb | 11 +++-------- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/lib/sprockets/preprocessors/default_source_map.rb b/lib/sprockets/preprocessors/default_source_map.rb index a051bdbf8..7f8c9340e 100644 --- a/lib/sprockets/preprocessors/default_source_map.rb +++ b/lib/sprockets/preprocessors/default_source_map.rb @@ -28,9 +28,11 @@ def call(input) else result[:map] = map end + + result[:map]["x_sprockets_linecount"] = lines return result end - + private def default_mappings(lines) diff --git a/lib/sprockets/source_map_utils.rb b/lib/sprockets/source_map_utils.rb index 6fc3e124c..5c9465502 100644 --- a/lib/sprockets/source_map_utils.rb +++ b/lib/sprockets/source_map_utils.rb @@ -81,16 +81,11 @@ def concat_source_maps(a, b) # Count the different VLQ sections such as "AACA;" by counting commas # mappings do not always end with a semicolon so we check for that # and increment - last_mappings = a["sections"].last["map"]["mappings"] - offset += last_mappings.count(';') - offset += 1 if last_mappings[-1] != ";" + last_line_count = a["sections"].last["map"].delete("x_sprockets_linecount") + offset += last_line_count last_offset = a["sections"].last["offset"]["line"] - if last_offset > 0 - offset += last_offset - else - offset += 1 - end + offset += last_offset end a["sections"] += b["sections"].map do |section| From 30b5838fd857f6330b68bd63fc7c227bd3d41e4e Mon Sep 17 00:00:00 2001 From: schneems Date: Mon, 13 Nov 2017 10:00:27 -0600 Subject: [PATCH 09/23] Fix tests for concat_javascript_sources We no longer add a newline if one did not previously exist. --- test/test_utils.rb | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/test/test_utils.rb b/test/test_utils.rb index 49537c39c..61d87f6fb 100644 --- a/test/test_utils.rb +++ b/test/test_utils.rb @@ -93,12 +93,13 @@ def test_string_ends_with_semicolon def test_concat_javascript_sources assert_equal "var foo;\n", apply_concat_javascript_sources("".freeze, "var foo;\n".freeze) assert_equal "\nvar foo;\n", apply_concat_javascript_sources("\n".freeze, "var foo;\n".freeze) - assert_equal " \nvar foo;\n", apply_concat_javascript_sources(" ".freeze, "var foo;\n".freeze) - assert_equal "var foo;\nvar bar;\n", apply_concat_javascript_sources("var foo;\n".freeze, "var bar;\n".freeze) - assert_equal "var foo;\nvar bar;\n", apply_concat_javascript_sources("var foo".freeze, "var bar".freeze) - assert_equal "var foo;\nvar bar;\n", apply_concat_javascript_sources("var foo;".freeze, "var bar;".freeze) - assert_equal "var foo;\nvar bar;\n", apply_concat_javascript_sources("var foo".freeze, "var bar;".freeze) + + assert_equal " var foo;\n", apply_concat_javascript_sources(" ".freeze, "var foo;\n".freeze) + assert_equal "var foo;var bar;", apply_concat_javascript_sources("var foo".freeze, "var bar".freeze) + assert_equal "var foo;var bar;", apply_concat_javascript_sources("var foo;".freeze, "var bar;".freeze) + assert_equal "var foo;var bar;", apply_concat_javascript_sources("var foo".freeze, "var bar;".freeze) + assert_equal "var foo;\nvar bar;", apply_concat_javascript_sources("var foo\n".freeze, "var bar;".freeze) end def apply_concat_javascript_sources(*args) From 3a0c297ae3d49bf50afc5b10322783e507ed0268 Mon Sep 17 00:00:00 2001 From: schneems Date: Mon, 13 Nov 2017 10:16:30 -0600 Subject: [PATCH 10/23] No newline after semicolon --- test/test_asset.rb | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/test/test_asset.rb b/test/test_asset.rb index e1063ba23..07abe9a7c 100644 --- a/test/test_asset.rb +++ b/test/test_asset.rb @@ -1080,8 +1080,14 @@ def setup end test "appends missing semicolons" do - assert_equal "var Bar\n;\n\n(function() {\n var Foo\n})\n;\n", - asset("semicolons.js").to_s +expected = <<-EOS +var Bar; + +(function() { + var Foo +}); +EOS + assert_equal expected, asset("semicolons.js").to_s end test 'keeps code in same line after multi-line comments' do From 21fb47b9182f4251377dc2237c07ad9d14904003 Mon Sep 17 00:00:00 2001 From: schneems Date: Mon, 13 Nov 2017 10:25:36 -0600 Subject: [PATCH 11/23] Ensure asset ends with a newline --- lib/sprockets/source_map_comment_processor.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/sprockets/source_map_comment_processor.rb b/lib/sprockets/source_map_comment_processor.rb index 599295825..e7f751a10 100644 --- a/lib/sprockets/source_map_comment_processor.rb +++ b/lib/sprockets/source_map_comment_processor.rb @@ -29,7 +29,7 @@ def self.call(input) path = PathUtils.relative_path_from(PathUtils.split_subpath(input[:load_path], uri), map.digest_path) asset.metadata.merge( - data: asset.source + (comment % path), + data: asset.source + (comment % path) + "\n", links: asset.links + [asset.uri, map.uri] ) end From 9d808a451358ef62a1ce9e48c0acfbb161571587 Mon Sep 17 00:00:00 2001 From: schneems Date: Mon, 13 Nov 2017 10:46:21 -0600 Subject: [PATCH 12/23] No newline after semicolon tests --- test/test_asset.rb | 38 +++++++++++++++++++++++++++++--------- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/test/test_asset.rb b/test/test_asset.rb index 07abe9a7c..d968e07da 100644 --- a/test/test_asset.rb +++ b/test/test_asset.rb @@ -885,8 +885,7 @@ def setup define("application.js", "application-955b2dddd0d1449b1c617124b83b46300edadec06d561104f7f6165241b31a94.js") define("application.css", "application-46d50149c56fc370805f53c29f79b89a52d4cc479eeebcdc8db84ab116d7ab1a.css") -define("POW.png", "POW-1da2e59df75d33d8b74c3d71feede698f203f136512cbaab20c68a5bdebd5800.png") -; +define("POW.png", "POW-1da2e59df75d33d8b74c3d71feede698f203f136512cbaab20c68a5bdebd5800.png"); EOS assert_equal [ "file://#{fixture_path_for_uri("asset/POW.png")}?type=image/png&id=xxx", @@ -904,8 +903,7 @@ def setup define("application.js", "application-955b2dddd0d1449b1c617124b83b46300edadec06d561104f7f6165241b31a94.js") define("application.css", "application-46d50149c56fc370805f53c29f79b89a52d4cc479eeebcdc8db84ab116d7ab1a.css") -define("POW.png", "POW-1da2e59df75d33d8b74c3d71feede698f203f136512cbaab20c68a5bdebd5800.png") -; +define("POW.png", "POW-1da2e59df75d33d8b74c3d71feede698f203f136512cbaab20c68a5bdebd5800.png"); EOS assert_equal [ @@ -1091,8 +1089,11 @@ def setup end test 'keeps code in same line after multi-line comments' do - assert_equal "/******/ function foo() {\n}\n;\n", - asset('multi_line_comment.js').to_s +expected = <<-EOS +/******/ function foo() { +}; +EOS + assert_equal expected, asset('multi_line_comment.js').to_s end test "should not fail if home is not set in environment" do @@ -1140,7 +1141,7 @@ def setup end test "digest path" do - assert_equal "application.debug-60cfa762e3139c2580dbfbefd46a5359803225363eaef31efb725e6731ab6849.js", + assert_equal "application.debug-dee339ce0ee2dc7cdfa0d36dff3ef946cebe1bd3e414515d40c3cafc49c0a51a.js", @asset.digest_path end @@ -1149,7 +1150,7 @@ def setup end test "length" do - assert_equal 264, @asset.length + assert_equal 265, @asset.length end test "charset is UTF-8" do @@ -1157,7 +1158,26 @@ def setup end test "to_s" do - assert_equal "var Project = {\n find: function(id) {\n }\n};\nvar Users = {\n find: function(id) {\n }\n};\n\n\n\ndocument.on('dom:loaded', function() {\n $('search').focus();\n});\n\n//# sourceMappingURL=application.js-161f7edec524c6e94ab3b89924c4fb96d4552bb83b32d975c074b7fb9a84c454.map", @asset.to_s +expected = <<-EOS +var Project = { + find: function(id) { + } +}; +var Users = { + find: function(id) { + } +}; + + + +document.on('dom:loaded', function() { + $('search').focus(); +}); + +//# sourceMappingURL=application.js-95e519d4e0f0a5c4c7d24787ded990b0d027f7ad30b39f402c4c5e3196a41e8b.map +EOS + + assert_equal expected, @asset.to_s end def asset(logical_path, options = {}) From b4f2dfac363d3c87c637097a1184d2b2804efd4d Mon Sep 17 00:00:00 2001 From: schneems Date: Mon, 13 Nov 2017 10:46:32 -0600 Subject: [PATCH 13/23] Fix comment --- lib/sprockets/source_map_utils.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/sprockets/source_map_utils.rb b/lib/sprockets/source_map_utils.rb index 5c9465502..ee216b342 100644 --- a/lib/sprockets/source_map_utils.rb +++ b/lib/sprockets/source_map_utils.rb @@ -77,10 +77,6 @@ def concat_source_maps(a, b) offset = 0 if a["sections"].count != 0 && !a["sections"].last["map"]["mappings"].empty? - # Account for length of last asset - # Count the different VLQ sections such as "AACA;" by counting commas - # mappings do not always end with a semicolon so we check for that - # and increment last_line_count = a["sections"].last["map"].delete("x_sprockets_linecount") offset += last_line_count From 8ad78ba378549b8f8964e1e9aa29be0299c833a9 Mon Sep 17 00:00:00 2001 From: schneems Date: Mon, 13 Nov 2017 11:10:43 -0600 Subject: [PATCH 14/23] =?UTF-8?q?Update=20tests=20for=20=E2=80=9Cx=5Fsproc?= =?UTF-8?q?kets=5Flinecount=E2=80=9D?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- test/test_environment.rb | 8 ++-- test/test_source_map_utils.rb | 82 ++++++++++++++++++++++++++++++----- test/test_source_maps.rb | 23 ++++++---- 3 files changed, 91 insertions(+), 22 deletions(-) diff --git a/test/test_environment.rb b/test/test_environment.rb index c2f639527..d5dc4b803 100644 --- a/test/test_environment.rb +++ b/test/test_environment.rb @@ -798,10 +798,12 @@ def foo; end end test "disabling default directive preprocessor" do - assert processor = @env.preprocessors['application/javascript'][0] - assert_kind_of Sprockets::DirectiveProcessor, processor + assert processors = @env.preprocessors['application/javascript'] + processor = processors.detect {|p| p.is_a?(Sprockets::DirectiveProcessor) } + + assert processor @env.unregister_preprocessor('application/javascript', processor) - assert_equal "// =require \"notfound\"\n;\n", @env["missing_require.js"].to_s + assert_equal "// =require \"notfound\";\n", @env["missing_require.js"].to_s end test "disabling processors by class name also disables processors which are instances of that class" do diff --git a/test/test_source_map_utils.rb b/test/test_source_map_utils.rb index ee1eff92b..6414a015c 100644 --- a/test/test_source_map_utils.rb +++ b/test/test_source_map_utils.rb @@ -3,6 +3,60 @@ require 'sprockets/source_map_utils' class TestSourceMapUtils < MiniTest::Test + module SprocketsDeepDup + refine Object do + # Returns a deep copy of object if it's duplicable. If it's + # not duplicable, returns +self+. + # + # object = Object.new + # dup = object.deep_dup + # dup.instance_variable_set(:@a, 1) + # + # object.instance_variable_defined?(:@a) # => false + # dup.instance_variable_defined?(:@a) # => true + def deep_dup + dup + end + end + + refine Array do + # Returns a deep copy of array. + # + # array = [1, [2, 3]] + # dup = array.deep_dup + # dup[1][2] = 4 + # + # array[1][2] # => nil + # dup[1][2] # => 4 + def deep_dup + map(&:deep_dup) + end + end + + refine Hash do + # Returns a deep copy of hash. + # + # hash = { a: { b: 'b' } } + # dup = hash.deep_dup + # dup[:a][:c] = 'c' + # + # hash[:a][:c] # => nil + # dup[:a][:c] # => "c" + def deep_dup + hash = dup + each_pair do |key, value| + if key.frozen? && ::String === key + hash[key] = value.deep_dup + else + hash.delete(key) + hash[key.deep_dup] = value.deep_dup + end + end + hash + end + end + end + using SprocketsDeepDup def test_map source_map = { @@ -94,12 +148,15 @@ def test_concat_empty_source_map_returns_original { source: 'c.js', generated: [rand(1..100), rand(0..100)], original: [rand(1..100), rand(0..100)] } ].freeze + linecount = abc_mappings.count(";") + linecount += 1 if abc_mappings[-1] == ";" map = { "version" => 3, "file" => "a.js", "mappings" => Sprockets::SourceMapUtils.encode_vlq_mappings(abc_mappings), "sources" => ["a.js", "b.js", "c.js"], - "names" => [] + "names" => [], + "x_sprockets_linecount" => linecount } empty_map = { "version" => 3, "file" => 'a.js', "sections" => [] } index_map = { @@ -113,11 +170,14 @@ def test_concat_empty_source_map_returns_original ] } - assert_equal map, Sprockets::SourceMapUtils.concat_source_maps(nil, map.dup) - assert_equal map, Sprockets::SourceMapUtils.concat_source_maps(map.dup, nil) + assert_equal map, Sprockets::SourceMapUtils.concat_source_maps(nil, map.deep_dup) + assert_equal map, Sprockets::SourceMapUtils.concat_source_maps(map.deep_dup, nil) + + assert_equal index_map, Sprockets::SourceMapUtils.concat_source_maps(empty_map.deep_dup, map.dup) - assert_equal index_map, Sprockets::SourceMapUtils.concat_source_maps(empty_map.dup, map.dup) - assert_equal index_map, Sprockets::SourceMapUtils.concat_source_maps(map.dup, empty_map.dup) + expected_index_map = index_map.deep_dup + expected_index_map["sections"].first["map"].delete("x_sprockets_linecount") + assert_equal expected_index_map, Sprockets::SourceMapUtils.concat_source_maps(map.deep_dup, empty_map.deep_dup) end def test_concat_source_maps @@ -136,7 +196,9 @@ def test_concat_source_maps "file" => "a.js", "mappings" => Sprockets::SourceMapUtils.encode_vlq_mappings(abc_mappings), "sources" => ["a.js", "b.js", "c.js"], - "names" => [] + "names" => [], + "x_sprockets_linecount" => 3 + } d_map = { @@ -144,11 +206,11 @@ def test_concat_source_maps "file" => "d.js", "mappings" => Sprockets::SourceMapUtils.encode_vlq_mappings(d_mapping), "sources" => ["d.js"], - "names" => [] + "names" => [], + "x_sprockets_linecount" => 1 } - - combined = Sprockets::SourceMapUtils.concat_source_maps(abc_map.dup, d_map.dup) + combined = Sprockets::SourceMapUtils.concat_source_maps(abc_map.deep_dup, d_map.deep_dup) assert_equal [ { source: 'a.js', generated: [1, 0], original: [1, 0] }, { source: 'b.js', generated: [2, 0], original: [20, 0] }, @@ -156,7 +218,7 @@ def test_concat_source_maps { source: 'd.js', generated: [4, 0], original: [1, 0] } ], Sprockets::SourceMapUtils.decode_source_map(combined)[:mappings] - combined = Sprockets::SourceMapUtils.concat_source_maps(d_map.dup, abc_map.dup) + combined = Sprockets::SourceMapUtils.concat_source_maps(d_map.deep_dup, abc_map.deep_dup) assert_equal [ { source: 'd.js', generated: [1, 0], original: [1, 0] }, { source: 'a.js', generated: [2, 0], original: [1, 0] }, diff --git a/test/test_source_maps.rb b/test/test_source_maps.rb index 7a7f8a97a..35240a83b 100644 --- a/test/test_source_maps.rb +++ b/test/test_source_maps.rb @@ -37,7 +37,6 @@ def get_sources(map) test "builds a minified source map" do @env.js_compressor = Sprockets::UglifierCompressor.new - asset = @env['application.js'] map = Sprockets::SourceMapUtils.decode_source_map(asset.metadata[:map]) @@ -103,7 +102,8 @@ def get_sources(map) "file" => "coffee/main.coffee", "mappings" => "AACA;AAAA,MAAA,sDAAA;IAAA;;EAAA,MAAA,GAAW;;EACX,QAAA,GAAW;;EAGX,IAAgB,QAAhB;IAAA,MAAA,GAAS,CAAC,GAAV;;;EAGA,MAAA,GAAS,SAAC,CAAD;WAAO,CAAA,GAAI;EAAX;;EAGT,IAAA,GAAO,CAAC,CAAD,EAAI,CAAJ,EAAO,CAAP,EAAU,CAAV,EAAa,CAAb;;EAGP,IAAA,GACE;IAAA,IAAA,EAAQ,IAAI,CAAC,IAAb;IACA,MAAA,EAAQ,MADR;IAEA,IAAA,EAAQ,SAAC,CAAD;aAAO,CAAA,GAAI,MAAA,CAAO,CAAP;IAAX,CAFR;;;EAKF,IAAA,GAAO,SAAA;AACL,QAAA;IADM,uBAAQ;WACd,KAAA,CAAM,MAAN,EAAc,OAAd;EADK;;EAIP,IAAsB,8CAAtB;IAAA,KAAA,CAAM,YAAN,EAAA;;;EAGA,KAAA;;AAAS;SAAA,sCAAA;;mBAAA,IAAI,CAAC,IAAL,CAAU,GAAV;AAAA;;;AA1BT", "sources" => ["main.source.coffee"], - "names" => [] + "names" => [], + "x_sprockets_linecount"=>47 } } ] @@ -157,7 +157,8 @@ def get_sources(map) "file" => "babel/main.es6", "mappings" => ";;;;;;;;;;AACA,IAAI,IAAI,GAAG,KAAK,CAAC,GAAG,CAAC,UAAA,CAAC;SAAI,CAAC,GAAG,CAAC;CAAA,CAAC,CAAC;AACjC,IAAI,IAAI,GAAG,KAAK,CAAC,GAAG,CAAC,UAAC,CAAC,EAAE,CAAC;SAAK,CAAC,GAAG,CAAC;CAAA,CAAC,CAAC;;IAEhC,WAAW;YAAX,WAAW;;AACJ,WADP,WAAW,CACH,QAAQ,EAAE,SAAS,EAAE;0BAD7B,WAAW;;AAEb,+BAFE,WAAW,6CAEP,QAAQ,EAAE,SAAS,EAAE;GAE5B;;eAJG,WAAW;;WAKT,gBAAC,MAAM,EAAE;AACb,iCANE,WAAW,wCAME;KAChB;;;WACmB,yBAAG;AACrB,aAAO,IAAI,KAAK,CAAC,OAAO,EAAE,CAAC;KAC5B;;;SAVG,WAAW;GAAS,KAAK,CAAC,IAAI;;AAapC,IAAI,SAAS,uBACV,MAAM,CAAC,QAAQ,0BAAG;MACb,GAAG,EAAM,GAAG,EAEV,IAAI;;;;AAFN,WAAG,GAAG,CAAC,EAAE,GAAG,GAAG,CAAC;;;AAEd,YAAI,GAAG,GAAG;;AACd,WAAG,GAAG,GAAG,CAAC;AACV,WAAG,IAAI,IAAI,CAAC;;eACN,GAAG;;;;;;;;;;;CAEZ,EACF,CAAA", "sources" => ["main.source.es6"], - "names" => [] + "names" => [], + "x_sprockets_linecount"=>66 } } ] @@ -216,7 +217,8 @@ def get_sources(map) "file" => "sass/main.scss", "mappings" => "AACE,MAAG;EACD,MAAM,EAAE,CAAC;EACT,OAAO,EAAE,CAAC;EACV,UAAU,EAAE,IAAI;AAGlB,MAAG;EAAE,OAAO,EAAE,YAAY;AAE1B,KAAE;EACA,OAAO,EAAE,KAAK;EACd,OAAO,EAAE,QAAQ;EACjB,eAAe,EAAE,IAAI", "sources" => ['main.source.scss'], - "names" => [] + "names" => [], + "x_sprockets_linecount"=>10 } } ] @@ -260,7 +262,8 @@ def get_sources(map) "_imported.source.scss", "with-import.source.scss" ], - "names" => [] + "names" => [], + "x_sprockets_linecount"=>5 } } ] @@ -294,7 +297,7 @@ def get_sources(map) test "source maps work with index alias" do asset = @env.find_asset("foo.js", pipeline: :debug) mapUrl = asset.source.match(/^\/\/# sourceMappingURL=(.*)$/)[1] - assert_equal "foo/index.js-501c1acd99a6f760dd3ec4195ab25a3518f689fcf1ffc9be33f28e2f28712826.map", mapUrl + assert_equal "foo/index.js-008b5ccb5459dc75d7fd51bf5b1ac79fe54d05157d50586c16e558f33d28e9c4.map", mapUrl map = JSON.parse(@env.find_asset('foo/index.js.map').source) assert_equal [ @@ -323,7 +326,7 @@ def get_sources(map) s["offset"]["line"] += 1 end end - + File.open(filename, 'a') do |file| file.puts "console.log('newline');" end @@ -378,7 +381,8 @@ def setup "file" => "sass/main.scss", "mappings" => "AAAA,AACE,GADC,CACD,EAAE,CAAC;EACD,MAAM,EAAE,CAAC;EACT,OAAO,EAAE,CAAC;EACV,UAAU,EAAE,IAAI,GACjB;;AALH,AAOE,GAPC,CAOD,EAAE,CAAC;EAAE,OAAO,EAAE,YAAY,GAAK;;AAPjC,AASE,GATC,CASD,CAAC,CAAC;EACA,OAAO,EAAE,KAAK;EACd,OAAO,EAAE,QAAQ;EACjB,eAAe,EAAE,IAAI,GACtB", "sources" => ["main.source.scss"], - "names" => [] + "names" => [], + "x_sprockets_linecount"=>12 } } ] @@ -422,7 +426,8 @@ def setup "with-import.source.scss", "_imported.source.scss" ], - "names" => [] + "names" => [], + "x_sprockets_linecount"=>5 } } ] From 25d1fa83f5a5c9f34373f1f9044120ea4fb4ab56 Mon Sep 17 00:00:00 2001 From: schneems Date: Mon, 13 Nov 2017 16:56:44 -0600 Subject: [PATCH 15/23] Unset _JAVA_OPTIONS https://github.com/travis-ci/travis-ci/issues/8408#issuecomment-344087255 --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 2652f43c8..daca89987 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,7 +3,7 @@ language: ruby cache: bundler sudo: false - +before_script: "unset _JAVA_OPTIONS" rvm: - 2.2 - 2.3.4 From 809ecc93d33e1b1622b300f82c559913277a2abd Mon Sep 17 00:00:00 2001 From: schneems Date: Mon, 13 Nov 2017 17:08:27 -0600 Subject: [PATCH 16/23] Rescue when trying to dup an immutable object --- test/test_source_map_utils.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/test_source_map_utils.rb b/test/test_source_map_utils.rb index 6414a015c..1429b9d7f 100644 --- a/test/test_source_map_utils.rb +++ b/test/test_source_map_utils.rb @@ -16,6 +16,8 @@ module SprocketsDeepDup # dup.instance_variable_defined?(:@a) # => true def deep_dup dup + rescue TypeError # TypeError: can't dup Fixnum + self end end From b2383420a2de3109e0ab3f281e187332f7bfa14b Mon Sep 17 00:00:00 2001 From: schneems Date: Mon, 13 Nov 2017 17:24:41 -0600 Subject: [PATCH 17/23] Deep dup without refinements --- test/test_source_map_utils.rb | 70 +++++------------------------------ 1 file changed, 9 insertions(+), 61 deletions(-) diff --git a/test/test_source_map_utils.rb b/test/test_source_map_utils.rb index 1429b9d7f..febca7b45 100644 --- a/test/test_source_map_utils.rb +++ b/test/test_source_map_utils.rb @@ -3,62 +3,10 @@ require 'sprockets/source_map_utils' class TestSourceMapUtils < MiniTest::Test - module SprocketsDeepDup - refine Object do - # Returns a deep copy of object if it's duplicable. If it's - # not duplicable, returns +self+. - # - # object = Object.new - # dup = object.deep_dup - # dup.instance_variable_set(:@a, 1) - # - # object.instance_variable_defined?(:@a) # => false - # dup.instance_variable_defined?(:@a) # => true - def deep_dup - dup - rescue TypeError # TypeError: can't dup Fixnum - self - end - end - - refine Array do - # Returns a deep copy of array. - # - # array = [1, [2, 3]] - # dup = array.deep_dup - # dup[1][2] = 4 - # - # array[1][2] # => nil - # dup[1][2] # => 4 - def deep_dup - map(&:deep_dup) - end - end - refine Hash do - # Returns a deep copy of hash. - # - # hash = { a: { b: 'b' } } - # dup = hash.deep_dup - # dup[:a][:c] = 'c' - # - # hash[:a][:c] # => nil - # dup[:a][:c] # => "c" - def deep_dup - hash = dup - each_pair do |key, value| - if key.frozen? && ::String === key - hash[key] = value.deep_dup - else - hash.delete(key) - hash[key.deep_dup] = value.deep_dup - end - end - hash - end - end + def deep_dup(object) + Marshal.load( Marshal.dump(object) ) end - using SprocketsDeepDup def test_map source_map = { @@ -172,14 +120,14 @@ def test_concat_empty_source_map_returns_original ] } - assert_equal map, Sprockets::SourceMapUtils.concat_source_maps(nil, map.deep_dup) - assert_equal map, Sprockets::SourceMapUtils.concat_source_maps(map.deep_dup, nil) + assert_equal map, Sprockets::SourceMapUtils.concat_source_maps(nil, deep_dup(map)) + assert_equal map, Sprockets::SourceMapUtils.concat_source_maps(deep_dup(map), nil) - assert_equal index_map, Sprockets::SourceMapUtils.concat_source_maps(empty_map.deep_dup, map.dup) + assert_equal index_map, Sprockets::SourceMapUtils.concat_source_maps(deep_dup(empty_map), map.dup) - expected_index_map = index_map.deep_dup + expected_index_map = deep_dup(index_map) expected_index_map["sections"].first["map"].delete("x_sprockets_linecount") - assert_equal expected_index_map, Sprockets::SourceMapUtils.concat_source_maps(map.deep_dup, empty_map.deep_dup) + assert_equal expected_index_map, Sprockets::SourceMapUtils.concat_source_maps(deep_dup(map), deep_dup(empty_map)) end def test_concat_source_maps @@ -212,7 +160,7 @@ def test_concat_source_maps "x_sprockets_linecount" => 1 } - combined = Sprockets::SourceMapUtils.concat_source_maps(abc_map.deep_dup, d_map.deep_dup) + combined = Sprockets::SourceMapUtils.concat_source_maps(deep_dup(abc_map), deep_dup(d_map)) assert_equal [ { source: 'a.js', generated: [1, 0], original: [1, 0] }, { source: 'b.js', generated: [2, 0], original: [20, 0] }, @@ -220,7 +168,7 @@ def test_concat_source_maps { source: 'd.js', generated: [4, 0], original: [1, 0] } ], Sprockets::SourceMapUtils.decode_source_map(combined)[:mappings] - combined = Sprockets::SourceMapUtils.concat_source_maps(d_map.deep_dup, abc_map.deep_dup) + combined = Sprockets::SourceMapUtils.concat_source_maps(deep_dup(d_map), deep_dup(abc_map)) assert_equal [ { source: 'd.js', generated: [1, 0], original: [1, 0] }, { source: 'a.js', generated: [2, 0], original: [1, 0] }, From f2bbb417312179f46401751b7e62162a0f04ebea Mon Sep 17 00:00:00 2001 From: schneems Date: Tue, 14 Nov 2017 10:51:34 -0600 Subject: [PATCH 18/23] More robust concurrent test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previous test was failing intermittently with results like `["0", "0", "1", "1", "0", "0", "1", “1”]` --- test/test_manifest.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_manifest.rb b/test/test_manifest.rb index 732612861..40f35b5a0 100644 --- a/test/test_manifest.rb +++ b/test/test_manifest.rb @@ -610,7 +610,7 @@ class SlowExporter2 < SlowExporter @env.register_exporter 'image/png',SlowExporter @env.register_exporter 'image/png',SlowExporter2 Sprockets::Manifest.new(@env, @dir).compile('logo.png', 'troll.png') - assert_equal %w(0 0 0 0 1 1 1 1), SlowExporter.seq + refute_equal %w(0 1 0 1 0 1 0 1), SlowExporter.seq end test 'sequential exporting' do @@ -642,7 +642,7 @@ def call(_) processor = SlowProcessor.new @env.register_postprocessor 'image/png', processor Sprockets::Manifest.new(@env, @dir).compile('logo.png', 'troll.png') - assert_equal %w(0 0 1 1), processor.seq + refute_equal %w(0 1 0 1), processor.seq end test 'sequential processing' do From 1c8756abf50c5b7b378bbb0fdd9589a0cbef5893 Mon Sep 17 00:00:00 2001 From: schneems Date: Tue, 14 Nov 2017 13:45:41 -0600 Subject: [PATCH 19/23] Test offsets --- test/fixtures/source-maps/multi-require.js | 4 ++++ test/fixtures/source-maps/plain.js | 3 +++ test/test_source_maps.rb | 26 ++++++++++++++++++++++ 3 files changed, 33 insertions(+) create mode 100644 test/fixtures/source-maps/multi-require.js create mode 100644 test/fixtures/source-maps/plain.js diff --git a/test/fixtures/source-maps/multi-require.js b/test/fixtures/source-maps/multi-require.js new file mode 100644 index 000000000..d54985a72 --- /dev/null +++ b/test/fixtures/source-maps/multi-require.js @@ -0,0 +1,4 @@ +//= require child.js +//= require coffee/main.js +//= require sub/a.js +//= require plain.js diff --git a/test/fixtures/source-maps/plain.js b/test/fixtures/source-maps/plain.js new file mode 100644 index 000000000..fba911629 --- /dev/null +++ b/test/fixtures/source-maps/plain.js @@ -0,0 +1,3 @@ +function plain(input) { + console.log("Plain" + input) +} diff --git a/test/test_source_maps.rb b/test/test_source_maps.rb index 35240a83b..deed24205 100644 --- a/test/test_source_maps.rb +++ b/test/test_source_maps.rb @@ -19,6 +19,32 @@ def get_sources(map) map["sections"].reduce([]) { |r, s| r | s["map"]["sources"] } end + # Offset should be the line that the asset starts on minus one + test "correct offsets" do + asset = @env["multi-require.js"] + map = asset.metadata[:map] + + child = @env["child.js"] + child_lines = child.to_s.lines.length + child_section = map["sections"][0] + assert_equal 0, child_section["offset"]["line"] + + coffee_main = @env["coffee/main.js"] + coffee_main_lines = coffee_main.to_s.lines.length + coffee_main_section = map["sections"][1] + assert_equal child_lines, coffee_main_section["offset"]["line"] + + sub_a_js = @env["sub/a.js"] + sub_a_js_lines = sub_a_js.to_s.lines.length + sub_a_js_section = map["sections"][2] + + assert_equal coffee_main_lines + child_lines, sub_a_js_section["offset"]["line"] + + plain_js = @env["plain.js"] + plain_js_section = map["sections"][3] + assert_equal sub_a_js_lines + coffee_main_lines + child_lines, plain_js_section["offset"]["line"] + end + test "builds a source map for js files" do asset = @env['child.js'] map = asset.metadata[:map] From 3b13ef84157a3fdee9961489e46af7381d38b4d5 Mon Sep 17 00:00:00 2001 From: schneems Date: Tue, 14 Nov 2017 13:49:29 -0600 Subject: [PATCH 20/23] Bump cache version MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We need this because `concat_source_maps` expects a `x_sprockets_linecount` key in the map metadata. It wasn’t put there until this PR, so if a map gets pulled out of the old cache and attempts to get loaded into memory then it will fail. By rev-ing the cache we can ensure the key will be there, since it’s added by `DefaultSourceMap`. --- lib/sprockets/cache.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/sprockets/cache.rb b/lib/sprockets/cache.rb index 7191e7e35..2b31b334d 100644 --- a/lib/sprockets/cache.rb +++ b/lib/sprockets/cache.rb @@ -51,7 +51,7 @@ class Cache # Internal: Cache key version for this class. Rarely should have to change # unless the cache format radically changes. Will be bump on major version # releases though. - VERSION = '4.0' + VERSION = '4.0.0' def self.default_logger logger = Logger.new($stderr) From e59e64d89925169f28bbba6950885a9256bbe8f5 Mon Sep 17 00:00:00 2001 From: schneems Date: Tue, 14 Nov 2017 16:15:17 -0600 Subject: [PATCH 21/23] Return if source bytesize is zero --- lib/sprockets/utils.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/sprockets/utils.rb b/lib/sprockets/utils.rb index 5e281c4f3..1233b78f7 100644 --- a/lib/sprockets/utils.rb +++ b/lib/sprockets/utils.rb @@ -98,7 +98,7 @@ def string_end_with_semicolon?(str) # # Returns buf String. def concat_javascript_sources(buf, source) - return buf if source.bytesize < 0 + return buf if source.bytesize <= 0 buf << source # If the source contains non-ASCII characters, indexing on it becomes O(N). From 3e75ea282123c62eb8cebef0b9741b482da20906 Mon Sep 17 00:00:00 2001 From: schneems Date: Tue, 14 Nov 2017 16:19:18 -0600 Subject: [PATCH 22/23] Index into `source` instead of `buf` So we can take advantage of the UTF32 encoding. --- lib/sprockets/utils.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/sprockets/utils.rb b/lib/sprockets/utils.rb index 1233b78f7..bb0c68d71 100644 --- a/lib/sprockets/utils.rb +++ b/lib/sprockets/utils.rb @@ -106,7 +106,7 @@ def concat_javascript_sources(buf, source) source = source.encode(Encoding::UTF_32LE) unless source.ascii_only? return buf if string_end_with_semicolon?(source) - if whitespace = WHITESPACE_ORDINALS[buf[-1].ord] + if whitespace = WHITESPACE_ORDINALS[source[-1].ord] buf[-1] = ";#{whitespace}" else buf << ";" From b7f530be60dd38b58061c542083b3b89f70a16a2 Mon Sep 17 00:00:00 2001 From: schneems Date: Tue, 14 Nov 2017 16:43:05 -0600 Subject: [PATCH 23/23] Add comment explaining logic --- lib/sprockets/utils.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/sprockets/utils.rb b/lib/sprockets/utils.rb index bb0c68d71..72e74b267 100644 --- a/lib/sprockets/utils.rb +++ b/lib/sprockets/utils.rb @@ -106,6 +106,9 @@ def concat_javascript_sources(buf, source) source = source.encode(Encoding::UTF_32LE) unless source.ascii_only? return buf if string_end_with_semicolon?(source) + # If the last character in the string was whitespace, + # such as a newline, then we want to put the semicolon + # before the whitespace. Otherwise append a semicolon. if whitespace = WHITESPACE_ORDINALS[source[-1].ord] buf[-1] = ";#{whitespace}" else