Skip to content
This repository has been archived by the owner on Dec 5, 2019. It is now read-only.

fix(index): don't remove legal comments by default (options.extractComments) #250

Merged
merged 1 commit into from
Mar 16, 2018
Merged

fix(index): don't remove legal comments by default (options.extractComments) #250

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class UglifyJsPlugin {
exclude,
uglifyOptions: {
output: {
comments: false,
comments: extractComments ? false : /^\**!|@preserve|@license|@cc_on/,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be just the {RegExp} to always preserve this comments ? Why is the extractComments check needed here ?

Copy link
Member Author

@alexander-akait alexander-akait Mar 13, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michael-ciniawsky When we use extractComments we should remove their from source code. If you pass extractComments: true and comments: /^\**!|@preserve|@license|@cc_on/ output will be contain legal comments in source file and generate name.js.LICENSE file

Copy link
Member

@michael-ciniawsky michael-ciniawsky Mar 13, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't extractComments not only responsible for only extracting what's preseverd via uglifyOptions.output.comments ?

// Preserve comments based on options, 
// defaults to /^\**!|@preserve|@license|@cc_on/
if (uglifyOptions.output.comments) {
   // Extract all preserved comments
   if (options.extractComments) {
       // [name].js.LICENSE
       extractComments()
   }
   // Leave comments in the source 
}

Why is are these options 'decoupled' ? 🙃

Anyways if that's more complicated atm and this PR now leaves the default preserved comments in the source aswell as options.extractComments also preserves the same default comments (with it's own logic) when extracting them into a separate file, then let's do it :)

Copy link
Member Author

@alexander-akait alexander-akait Mar 13, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michael-ciniawsky extractComments does not responsible uglify comments, they are separatly. If think about your logic. But sometimes people leave comment in source code and generate license file to allow difference type of distribution.

},
...uglifyOptions,
},
Expand Down
117 changes: 70 additions & 47 deletions test/__snapshots__/extract-comments-options.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,107 +1,130 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`errors 1`] = `Array []`;
exports[`"options.extractComments" is "boolean" - "true": errors 1`] = `Array []`;

exports[`errors 2`] = `Array []`;
exports[`"options.extractComments" is "boolean" - "true": test.js 1`] = `"var foo=1;"`;

exports[`errors 3`] = `Array []`;

exports[`errors 4`] = `Array []`;

exports[`errors 5`] = `Array []`;

exports[`nested/nested/test1.js 1`] = `
exports[`"options.extractComments" is "boolean" - "true": test1.js 1`] = `
"/*! For license information please see test1.js.LICENSE */
var foo=1;"
`;

exports[`nested/nested/test1.js.LICENSE 1`] = `
"/* Comment */
exports[`"options.extractComments" is "boolean" - "true": test1.js.LICENSE 1`] = `
"/*! Legal Comment */
"
`;

exports[`nested/test.js 1`] = `
exports[`"options.extractComments" is "boolean" - "true": warnings 1`] = `Array []`;

exports[`"options.extractComments" is "function": errors 1`] = `Array []`;

exports[`"options.extractComments" is "function": test.js 1`] = `
"/*! For license information please see test.js.LICENSE */
var foo=1;"
`;

exports[`nested/test1.js.LICENSE 1`] = `
exports[`"options.extractComments" is "function": test.js.LICENSE 1`] = `
"// Comment
"
`;

exports[`test.js 1`] = `"var foo=1;"`;

exports[`test.js 2`] = `"var foo=1;"`;

exports[`test.js 3`] = `
"/*! For license information please see test.js.LICENSE */
exports[`"options.extractComments" is "function": test1.js 1`] = `
"/*! For license information please see test1.js.LICENSE */
var foo=1;"
`;

exports[`test.js 4`] = `
"/*! For license information please see test.js.LICENSE */
var foo=1;"
exports[`"options.extractComments" is "function": test1.js.LICENSE 1`] = `
"/* Comment */
"
`;

exports[`test.js 5`] = `
"/*! For license information please see test1.js.LICENSE */
var foo=1;"
`;
exports[`"options.extractComments" is "function": warnings 1`] = `Array []`;

exports[`"options.extractComments" is "object": errors 1`] = `Array []`;

exports[`test.js 6`] = `
exports[`"options.extractComments" is "object": test.js 1`] = `
"/*! License information can be found in test.license.js */
var foo=1;"
`;

exports[`test.js.LICENSE 1`] = `
exports[`"options.extractComments" is "object": test.license.js 1`] = `
"// Comment
"
`;

exports[`test.js.LICENSE 2`] = `
"// Comment
"
exports[`"options.extractComments" is "object": warnings 1`] = `Array []`;

exports[`"options.extractComments" is "regex": errors 1`] = `Array []`;

exports[`"options.extractComments" is "regex": test.js 1`] = `"var foo=1;"`;

exports[`"options.extractComments" is "regex": test1.js 1`] = `
"/*! For license information please see test1.js.LICENSE */
var foo=1;"
`;

exports[`test.license.js 1`] = `
"// Comment
exports[`"options.extractComments" is "regex": test1.js.LICENSE 1`] = `
"// foo
"
`;

exports[`test1.js 1`] = `"var foo=1;"`;
exports[`"options.extractComments" is "regex": warnings 1`] = `Array []`;

exports[`test1.js 2`] = `
exports[`"options.extractComments" is "string" - "all" and license file should be relative source file: errors 1`] = `Array []`;

exports[`"options.extractComments" is "string" - "all" and license file should be relative source file: nested/nested/test1.js 1`] = `
"/*! For license information please see test1.js.LICENSE */
var foo=1;"
`;

exports[`test1.js 3`] = `
"/*! For license information please see test1.js.LICENSE */
exports[`"options.extractComments" is "string" - "all" and license file should be relative source file: nested/nested/test1.js.LICENSE 1`] = `
"/* Comment */
"
`;

exports[`"options.extractComments" is "string" - "all" and license file should be relative source file: nested/test.js 1`] = `
"/*! For license information please see test.js.LICENSE */
var foo=1;"
`;

exports[`test1.js.LICENSE 1`] = `
"// foo
exports[`"options.extractComments" is "string" - "all" and license file should be relative source file: nested/test.js.LICENSE 1`] = `
"// Comment
"
`;

exports[`test1.js.LICENSE 2`] = `
"/* Comment */
exports[`"options.extractComments" is "string" - "all" and license file should be relative source file: warnings 1`] = `Array []`;

exports[`"options.extractComments" is "string": errors 1`] = `Array []`;

exports[`"options.extractComments" is "string": test.js 1`] = `
"/*! For license information please see test.js.LICENSE */
var foo=1;"
`;

exports[`"options.extractComments" is "string": test.js.LICENSE 1`] = `
"// Comment
"
`;

exports[`test1.js.LICENSE 3`] = `
exports[`"options.extractComments" is "string": test1.js 1`] = `
"/*! For license information please see test1.js.LICENSE */
var foo=1;"
`;

exports[`"options.extractComments" is "string": test1.js.LICENSE 1`] = `
"/* Comment */
"
`;

exports[`warnings 1`] = `Array []`;
exports[`"options.extractComments" is "string": warnings 1`] = `Array []`;

exports[`warnings 2`] = `Array []`;
exports[`"options.extractComments" is not specify: errors 1`] = `Array []`;

exports[`warnings 3`] = `Array []`;
exports[`"options.extractComments" is not specify: test.js 1`] = `"var foo=1;"`;

exports[`warnings 4`] = `Array []`;
exports[`"options.extractComments" is not specify: test1.js 1`] = `
"/*! Legal Comment */
var foo=1;"
`;

exports[`warnings 5`] = `Array []`;
exports[`"options.extractComments" is not specify: warnings 1`] = `Array []`;
2 changes: 1 addition & 1 deletion test/__snapshots__/invalid-options.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ exports[`warnings 1`] = `Array []`;
exports[`when applied with invalid options throws validation errors 1`] = `
"UglifyJs Plugin Invalid Options

options['doesntExist'] is an invalid additional property
options['doesntExist'] should NOT have additional properties
"
`;

Expand Down
Loading