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

Beautify mode: comments are lost in the end of function body #88

Closed
thorn0 opened this issue Jan 4, 2013 · 11 comments · Fixed by #2633
Closed

Beautify mode: comments are lost in the end of function body #88

thorn0 opened this issue Jan 4, 2013 · 11 comments · Fixed by #2633

Comments

@thorn0
Copy link

thorn0 commented Jan 4, 2013

For example,

function f() {
  /* comment1 */
  alert(1);
  /* comment2 */
}

becomes

function f() {
  /* comment1 */
  alert(1);
}
@mishoo
Copy link
Owner

mishoo commented Jan 14, 2013

There are other cases too. Generally, comments are lost if there is no AST node following them on the current nesting level. More examples:

if (foo /* lost comment */ && bar /* lost comment */) {
    // this one is kept
    bar();
    /* lost comment */
}

// comments right before EOF are lost as well

It's a known problem and I kept thinking of it but there's no easy fix.

@Francisc
Copy link

Francisc commented Apr 4, 2014

Hello,

It seems comments are also lost when code follows as opposed to closing curly brace in your example:

/*! a */
var someVar=1;
/*! someArray */
var someArray=[];
/*! someObject */
var someObject={};
var blabla;

@getify
Copy link

getify commented Sep 17, 2014

Another troublesome example of this problem is with dropped function names:

(function/*!foo*/a(){ console.log(a); })();

=>

!function/*!foo*/o(){console.log(o)}();

But:

(function/*!foo*/a(){ console.log("uh oh"); })();

=>

!function(){console.log("uh oh"))}();

Note: I think my particular example of this comment-dropping is related to #547 in that I think going forward we're going to need an option for not dropping function expression names (even if they're not lexically referenced) -- just filed #552 for such.

BUT, it would certainly be nice if /*!foo*/ could be preserved even in:

(function/*!foo*/(){ console.log(a); })();

=>

!function(){console.log(a)}();

@gaborbernat
Copy link

Any solution for this? :)

@xtianjohns
Copy link

While I'm not certain that the causes are related, I've found another case where comments will not be preserved. Apologies if there is a duplicate issue: closest potential I found was #478.

With comments: /@foo/, the following will not produce any comments:

// example one

if ( true ) { console.log( true ); }
// @foo bar
if ( true ) { console.log( true ); }

// example two

if ( true ) { console.log( true ); }
// @foo bar
if ( true ) { console.log( true ); }
var x;

// example three

if ( true ) { console.log( true ); }
// @foo bar
if ( true ) { console.log( true ); }
anything();

However, this configuration will produce a comment:

if ( true ) { console.log( true ); }
// @foo bar
var x;

Someone should be able to reproduce with version 2.5.0, and with default options. Example:

require( 'uglify-js' ).minify( 'foo.js', {
  output: {
    comments: /@foo/,
  },
});

Important note: I can get this behavior with a regular expression and the 'license' comments option. @avdg may have alluded to why that might be, but even passing 'all' or true does not seem to alter the behavior in my examples.

@kzc
Copy link
Contributor

kzc commented Oct 31, 2015

Certain AST transformations will cause comments to be lost. If you selectively disable some compress options your example will work.

uglifyjs your_code.js --bare-returns --comments all -m -c "sequences=false, dead_code=false"

but comments in the following code will not be preserved with these options:

foo(1 + /* hello */ 2 /* world */ + 3);

===>

foo(6);

Some optimizations are destructive and leave no place for comments. I suppose they could be collected and put at the end of the transformed expression but it would look odd.

@avdg
Copy link
Contributor

avdg commented Jan 21, 2016

I wonder if AST_Scope can contain the comments at the end of a block. This will need to be investigated technically to see if this can be implemented...

@avdg
Copy link
Contributor

avdg commented Jan 21, 2016

(please note that this doesn't mean that I will work on it. I'll be probably too busy to implement this for now)

@tyomo4ka
Copy link

Just to confirm that the issue is still there in version 2.6.4.

@avdg
Copy link
Contributor

avdg commented Sep 26, 2016

alexlamsl added a commit that referenced this issue Dec 21, 2017
- improve handling of comments right after `return`
- retain comments after `OutputStream`
- preserve trailing comments
- fix handling of new line before comments
- handle comments around parentheses

fixes #88
fixes #112
fixes #218
fixes #372
fixes #2629
@avdg
Copy link
Contributor

avdg commented Dec 22, 2017

👍

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

Successfully merging a pull request may close this issue.

9 participants