Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added semicolon has no effect if source file ends with a comment line #702

Open
TylerRick opened this issue Mar 11, 2021 · 0 comments
Open

Comments

@TylerRick
Copy link

TylerRick commented Mar 11, 2021

Expected behavior

There should not be any JavaScript errors after concatenating 2 source files, which themselves are valid JavaScript scripts.

Actual behavior

b.source.js:1 Uncaught TypeError: (intermediate value)(...) is not a function
    at b.source.js:1

How to reproduce

Given that we are concatenating 2 files like this:

//= require a
//= require b

and given that a.js ends with a comment line, then the resulting output ends up being this:

(function() {
  console.log('a.js')
})()
//;
(function() {
  console.log('b.js')
})();

... causing the semicolon to have no effect and causing the error mentioned above — the exact problem the added semicolon was supposed to protect us from.

Reproduced in this example app: https://github.com/TylerRick/sprockets_concat_issue_when_last_line_is_comment

Commentary

I don't think we want to make it a (documented) requirement that JS scripts must not end with a comment line, right?

Seems like it would be more reasonable to make it so the concatenation works in all cases (provided the source files themselves are valid JS scripts).

Possible solutions

Any reason we shouldn't just unconditionally append "\n;" (newline + semicolon) to every JS file that is concatenated? That would force the semicolon to be on a line by itself (well, possibly the start of the next script might end joining it on the same line, but that shouldn't be a problem).

As @casperisfine asked so eloquently pointed out (as did @SamSaffron in #300):

  • This semi-colon is added to prevent changing the code behaviour (the famous line ending with parentheses, etc)
  • Most people will use a JS minifier
  • If they don't, a single extra character is unlikely to change much

If I'm right about all the above: Why don't we simply always add a semi-colon regardless of what the file ends with?

@rmacklin had the same idea — unconditionally adding ";\n" — and even turned the proposal into a PR, #310, but #311 ended up getting merged instead.

It was mentioned that "shifting lines" and "breaking" source maps was a potential concern with that approach, but I don't understand why that concern couldn't simply be directly addressed/solved, instead of fearing it... I wonder if @casperisfine's
simple idea might work?

Maybe it would be simple to always add that line, and always shift the source maps by 1.

I'm also wondering if there was any reason why ";\n" would be preferable over "\n;"? (An unconditional ";\n" wouldn't help with the present issue.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant