Comments: preserveComments option not working as expected

[ntwb]

Updating to v0.10.0 causes some issues with the preserveComments option:

The Grunt task grunt uglify:jqueryui produces minified files that include comments that start with //, previously in v0.9.2 these comments were stripped.

e.g Using preserveComments: 'some': (Truncated for readability, full diff is here)

diff --git wp-includes/js/jquery/ui/accordion.min.js wp-includes/js/jquery/ui/accordion.min.js
index 259a325..84ed01b 100644
--- wp-includes/js/jquery/ui/accordion.min.js
+++ wp-includes/js/jquery/ui/accordion.min.js
@@ -8,4 +8,49 @@
  *
  * http://api.jqueryui.com/accordion/
  */
-!function(a){"function"==typeof define&&define.amd?define(["jquery","./core","./widget"],a):
\ No newline at end of file
+!function(a){"function"==typeof define&&define.amd?
+// AMD. Register as an anonymous module.
+define(["jquery","./core","./widget"],a):
+// Browser globals

Changing the task config to preserveComments: 'all' or preserveComments: false the results are correct and expected with all comments kept or removed respectively.

The above is based on following configuration (again truncated for readability) the full WordPress grunt task configuration is here Gruntfile.js#L411 (unchanged for v0.10.0)

        uglify: {
            options: {
                ASCIIOnly: true
            },
            ...
            ...
            jqueryui: {
                options: {
                    preserveComments: 'some'
                },
                expand: true,
                cwd: SOURCE_DIR,
                dest: BUILD_DIR,
                ext: '.min.js',
                src: ['wp-includes/js/jquery/ui/*.js']
            },

[Mottie]

I'm having this same issue... it appears that Uglify2 now only accepts three comments settings (ref):

• Preserve copyright comments in the output. By default this works like Google Closure, keeping JSDoc-style comments that contain "@license" or "@preserve".
• You can optionally pass one of the following arguments to this flag:
  1. "all" or "true" (ref) to keep all comments.
  2. A valid JS regexp (needs to start with a slash) to keep only comments that match.
• Note that currently not all comments can be kept when compression is on, because of dead code removal or cascading statements into sequences.

The only way I was able to get it to work like the previous 'some' setting was to add a function to my Gruntfilte.js:

uglify: {
    options: {
        preserveComments: function(node, comment) {
            // preserve comments that start with a bang
            return /^!/.test( comment.value );
        },
    }
}

Trying to use preserveComments: '/^!/' did not work for me.

[avdg]

hmm might be a side-effect of // being recognized as a reg-exp
uh nevermind, that's the comment option I guess. I'm not sure how the preserveComments from grunt is hooked up with uglifyjs yet and how uglifyjs is using it.

[avdg]

Note: Old post, code not working, use preserveComments instead of comments

not sure if this works:

uglify: {
    options: {
        output: {
            comments: /^!/
        }
    }
}

[Mottie]

I don't think so. If you look at the code that starts at line 236:

if (ARGS.comments != null) {
    if (/^\/.*\/[a-zA-Z]*$/.test(ARGS.comments)) {
        try {
            OUTPUT_OPTIONS.comments = extractRegex(ARGS.comments);
        } catch (e) {
            print_error("ERROR: Invalid --comments: " + e.message);
        }
    } else if (ARGS.comments == "all") {

It uses test on ARGS.comments, so it is expecting a string.

The extractRegex function on line 203:

function extractRegex(str) {
  if (/^\/.*\/[a-zA-Z]*$/.test(str)) {
    var regex_pos = str.lastIndexOf("/");
    return new RegExp(str.substr(1, regex_pos - 1), str.substr(regex_pos + 1));
  } else {
    throw new Error("Invalid regular expression: " + str);
  }
}

is testing for a string again, but it must start with a /, which it strips out, then it uses new RegExp. So, that option will only accept a string, and it should also be noted that special characters will need to be double escaped, e.g. \\s.

[avdg]

That's the case if grunt uses the cli to communicate to uglifyjs, with the api, it can directly call the uglifyjs api and bypass the cli options (avoiding to parse such things).

[avdg]

Uh... I see that this project is using its own minifier instead of UglifyJS's minify function. It's using this: https://github.com/gruntjs/grunt-contrib-uglify/blob/master/tasks/lib/uglify.js.

Need some time to investigate stuff here before I can say more...

[ntwb]

For WordPress the issue is now resolved via https://core.trac.wordpress.org/changeset/35564

Which is to use: preserveComments: /^!/, instead of the old some value

Related: #190 preserveComments:'some' removes subsequent /*! */ comments

[Mottie]

The README.md needs to be updated as well as it still includes 'some' as an option: https://github.com/gruntjs/grunt-contrib-uglify#preservecomments

@avdg shouldn't this be preserveComments instead of comments?

uglify: {
    options: {
        output: {
            comments: /^!/
        }
    }
}

[avdg]

uh, that's because I mixed up the UglifyJS.minify options with grunt-contrib-uglify options. Cleaning up...

[kara-ryli]

To those dealing with this issue, the full "some" behavior as documented should be something like this:

grunt.initConfig({
  uglify: {
    options: {
      preserveComments: /(?:^!|@(?:license|preserve|cc_on))/
    },
    files: []
  }
});