Sprockets Semicolon insertion causes sourcemap to be off by (n) number of lines

[danshultz]

Expected behavior

When using sprockets with source maps, it's expected the sourcemap lines up with the original source.

Actual behavior

Each source file will be off by (n) number of lines based upon the order it was appended and the (n) number of semicolons inserted.

The semicolons are inserted here which is after default source maps are created it appears.

System configuration

• Sprockets 4.0.0.beta3
• Ruby 2.3.1
• Rails 4.2.6
• Sprockets Rails 3.2.0

Example App

This can be reproduced in the test suite by introspecting the source map generation that happens during the appends missing semicolons test run.

The input JS is

test/fixtures/asset/semicolons/bar.js

var Bar

test/fixtures/asset/semicolons/index.js

//= require ./bar

(function() {
  var Foo
})

The output of the JS is

var Bar
;

(function() {
  var Foo
})
;

The source map representation is

 :map=>
  [
    # correct line
    {:source=>"semicolons/bar.source-d10fa4c82aa701b7cd015bb9588451ad753c08fc48952d5b8a3a1270aee1ad68.js", :generated=>[1, 0], :original=>[1, 0]}, 
    # should be ":generated=>[3, 0]"
   {:source=>"semicolons/index.source-47a14a5ea5ac770cc99d947179aa913a0869508bc555bac2a621e5aba564485e.js", :generated=>[2, 0], :original=>[2, 0]},
    # should be ":generated=>[4, 0]"
   {:source=>"semicolons/index.source-47a14a5ea5ac770cc99d947179aa913a0869508bc555bac2a621e5aba564485e.js", :generated=>[3, 0], :original=>[2, 0]},
    # should be ":generated=>[5, 0]"
   {:source=>"semicolons/index.source-47a14a5ea5ac770cc99d947179aa913a0869508bc555bac2a621e5aba564485e.js", :generated=>[4, 0], :original=>[3, 0]},
    # should be ":generated=>[6, 0]"
   {:source=>"semicolons/index.source-47a14a5ea5ac770cc99d947179aa913a0869508bc555bac2a621e5aba564485e.js", :generated=>[5, 0], :original=>[4, 0]},
     # should be ":generated=>[7, 0]"
   {:source=>"semicolons/index.source-47a14a5ea5ac770cc99d947179aa913a0869508bc555bac2a621e5aba564485e.js", :generated=>[6, 0], :original=>[5, 0]}]

[danshultz]

I started to look into this a little more but was unsure what a correct solution should be.

I wondered if we could change the way semicolon addition works to simply append a semicolon on the last line of the JS file and omit the extra newline.

If someone has a thought on how to address this, I'll work on a PR

[schneems]

Inserting on the same line seems fine

[SamSaffron]

I wonder if the semicolon appending is even needed these days...

[danshultz]

Thanks for posing that question @SamSaffron - I had wondered about that as well...especially since we are ensuring there is a newline before concatenating now which was not the case previously.

https://github.com/rails/sprockets/blob/44b48d505820e578c01167a0a081f05814402ccc/lib/sprockets/safety_colons.rb

Anyone have thoughts/opinions if this is still necessary?

[bouk]

@danshultz yeah the newline stuff isn't necessary, just appending a semicolon should be OK. Appending them is still needed I think, as not adding a semicolon at the end of the file is fairly common and valid JS.

[TylerHorth]

We need to be careful to ensure there is a newline between the two files when they are concatenated. Otherwise the last line of the first file and the first line of the second file will get appended onto the same line and break source maps.

It will need to check if the file ends in a newline, and append the semicolon before the newline, otherwise append a semicolon + newline.

[marcusmalmberg]

Any news on this one?