Special comments in the middle of input get dropped

[laniakea64]

I'm looking into using minify command-line tool for minifying Javascript, and so far it seems really nice! 👍

One minor issue:

Per #68 it looks like minify is supposed to keep special/important/bang comments as-is in the output. But this is not fully working for me. Testcase:

$ ./minify --version
minify v2.12.4-14-ga07d14f
$ cat ./test.js
/*! important comment 1 */
const foo = '123';
/*! important comment 2 */
let bar = '456';

$ ./minify ./test.js
/*! important comment 1 */const foo="123";let bar="456"

but the expected result is

/*! important comment 1 */const foo="123";/*! important comment 2 */let bar="456"

Seems minify is dropping all bang comments in the middle or end of the output - even --bundle doesn't work around this:

$ cat ./test2.js
/*! important comment 3 */
function zzz() {}
/*! important comment 4 */
let arrowfunc = () => {};

$ ./minify --bundle -o ./test-all.min.js ./test.js ./test2.js 
$ cat ./test-all.min.js 
/*! important comment 1 */const foo="123";let bar="456";function zzz(){}let arrowfunc=()=>{}

Is there a way to have minify preserve all bang comments in the output?

Thanks 🙂

[tdewolff]

Thanks for raising this, in March I'll be back from vacation and will take a look at this!

…

On Sun, 8 Jan 2023, 22:56 laniakea64, ***@***.***> wrote: I'm looking into using minify command-line tool for minifying Javascript, and so far it seems really nice! 👍 One minor issue: Per #68 <#68> it looks like minify is supposed to keep special/important/bang comments as-is in the output. But this is not fully working for me. Testcase: $ ./minify --version minify v2.12.4-14-ga07d14f $ cat ./test.js /*! important comment 1 */ const foo = '123'; /*! important comment 2 */ let bar = '456'; $ ./minify ./test.js /*! important comment 1 */const foo="123";let bar="456" but the expected result is /*! important comment 1 */const foo="123";/*! important comment 2 */let bar="456" Seems minify is dropping all bang comments in the middle or end of the output - even --bundle doesn't work around this: $ cat ./test2.js /*! important comment 3 */ function zzz() {} /*! important comment 4 */ let arrowfunc = () => {}; $ ./minify --bundle -o ./test-all.min.js ./test.js ./test2.js $ cat ./test-all.min.js /*! important comment 1 */const foo="123";let bar="456";function zzz(){}let arrowfunc=()=>{} Is there a way to have minify preserve all bang comments in the output? Thanks 🙂 — Reply to this email directly, view it on GitHub <#552>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABKOGHRDVZAGDM2AEHNOKI3WRN46HANCNFSM6AAAAAATU6JBXU> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[aniaq]

Following up @tdewolff :)

[tdewolff]

Thanks for the follow up, must have slipped through! Can you check if it works alright now?

[aniaq]

@tdewolff Thank you! I tested with 2.12.8 and it still doesn't work for me

[laniakea64]

Thanks for working on this. 2.12.8 is an improvement, fixes the cases in the original report, but this is not fully fixed. The important comment is still dropped from e.g. this input:

let o = {
  /*! important comment */
  abc: 123,
  def: '456',
};

console.log(Object.getOwnPropertyNames(o));

produces

let o={abc:123,def:"456"};console.log(Object.getOwnPropertyNames(o))

but the expected result is

let o={/*! important comment */abc:123,def:"456"};console.log(Object.getOwnPropertyNames(o))

[tdewolff]

Hm, the problem is that this is quite difficult to do as I'd have to add the possibility to have comments in all types of expressions: additions, subtractions, comma-listed, part of a list, part of a dictionary (key and value), ...

This isn't really feasible. The only reason to keep bang-comments is because they contain license/legal information, and you should put that between statements and not in expressions. I'm not sure what kind of comment you have, but if it's anything other than legal stuff it's not really the idea to keep them anyways. Why do you need to keep a specific comment?

I'm actually regretting the commit above, since legal comments should be at the top of the file which saves us from checking for comments throughout the entire file (which is sliiiiightly slower).

[laniakea64]

The case that prompted this issue was a userscript that is partially dynamically generated from downloaded data converted into a Javascript-friendly format at build time. It contains two bang comments - one at the very beginning containing the userscript header and license info, and a second containing the date and time of the download that was used for the dynamically-generated part. The second bang comment that was being dropped is placed more like in the latest example.

The only reason to keep bang-comments is because they contain license/legal information,

Oh, I was not aware it was specific to this use only. In that case, I guess this is now fully covered in 2.12.8, so sorry for the follow-up noise - sounds like for my userscript, only my first bang comment should be a bang comment, and for the second one I should find some other way to include that info if I want it available in the output.

[tdewolff]

Yeah, I mean if it were easy to implement I'd do it, but this would really impact the AST that is build and the code that uses it, sorry!