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

MIssing newline in .min.js breaks downstream JS-combining builds #780

Closed
sgbeal opened this issue May 20, 2022 · 8 comments
Closed

MIssing newline in .min.js breaks downstream JS-combining builds #780

sgbeal opened this issue May 20, 2022 · 8 comments

Comments

@sgbeal
Copy link

sgbeal commented May 20, 2022

Issue summary

The distributed jquery.terminal.min.js has no newline on its final line. This leads to downstream pain for builds which combine multiple JS files into one, as the file appended after that one ends up being appended to a single-line comment:

//# sourceMappingURL=jquery.terminal.min.js.map/* This is the --pre-js file for emcc. It gets prepended to the

Yes, it can (must) be worked around downstream, but...

Expected behavior

All text files "really need" to end with a newline.

Actual behavior

See above.

Steps to reproduce

N/A

Browser and OS

N/A

Additional notes

N/A

@jcubic
Copy link
Owner

jcubic commented May 20, 2022

Thanks for the report. The file is generated by Uglify-js you can report this issue there. In the meantime, I can add a new line in Makefile.

jcubic added a commit that referenced this issue May 20, 2022
@jcubic
Copy link
Owner

jcubic commented May 20, 2022

I will report the issue upstream.

@jcubic
Copy link
Owner

jcubic commented May 20, 2022

Can you share an example of a build script that fails to build because of the issue?

@sgbeal
Copy link
Author

sgbeal commented May 20, 2022

Makefile snippet (noting that the hard tab is replaced by spaces here):

bundle.js:
    cat jquery.terminal.min.js foo.js bar.js > $@

In my case, foo.js starts with the first line of a /* ...comment. The final line of the terminal script started with // ... and didn't have a newline, so that /*... was part of that //... comment, but the rest of the /* ... comment block was not.

It's trivial to work around, but annoying to have to work around it. It's always good practice to have a final newline on text files, and some tools will complain if that's not the case.

@jcubic
Copy link
Owner

jcubic commented May 20, 2022

So you don't use any build system. So this is not a bug in UglifyJS. You can easily modify your script. If you would use Babel or any other bundler instead of primitive Unix cat, everything would work as expected.

@sgbeal
Copy link
Author

sgbeal commented May 20, 2022

i use a makefile (a build system which predates node and friends by 30 or 40 years ;)) to concatenate multiple JS files, one of which was copied verbatim from this project's git checkout:

[stephan@nuc:~/src/jquery.terminal/js]$ tail -1 jquery.terminal.min.js
//# sourceMappingURL=jquery.terminal.min.js.map[stephan@nuc:~/src/jquery.terminal/js]$ 

Note that the final line has no trailing newline, causing my shell prompt to continue on that same line. Whether that missing newline is caused by one of your own build tools or not, i have no clue, i just know that the file in question is part of your project, thus the bug report filed here.

Modifying my build was exactly what i ended up having to do, but not having a trailing newline is "poor practice" exactly because it causes grief for downstream tools. That's what i'm reporting.

@jcubic
Copy link
Owner

jcubic commented May 20, 2022

I'm not sure if you're aware. But the fix was released in version 2.33.3 it's already on NPM.

@jcubic jcubic closed this as completed May 20, 2022
@jcubic
Copy link
Owner

jcubic commented May 20, 2022

BTW: I'm using make myself, you would look at the code you will see.

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

2 participants