fix: pass of {Boolean|Function} parameters being ignored (options.extractComments)

README.md

@@ -209,7 +209,7 @@ All comments that match the given expression (resp. are evaluated to `true` by t
 |Name|Type|Default|Description|
 |:--:|:--:|:-----:|:----------|
 |**`condition`**|`{Regex\|Function}`|``|Regular Expression or function (see previous point)|
-|**`filename`**|`{String\|Function}`|`compilation.assets[file]`|The file where the extracted comments will be stored. Can be either a `{String}` or a `{Function<(string) -> {String}>}`, which will be given the original filename. Default is to append the suffix `.LICENSE` to the original filename|
+|**`filename`**|`{String\|Function}`|`${file}.LICENSE`|The file where the extracted comments will be stored. Can be either a `{String}` or a `{Function<(string) -> {String}>}`, which will be given the original filename. Default is to append the suffix `.LICENSE` to the original filename|
 |**`banner`**|`{Boolean\|String\|Function}`|`/*! For license information please see ${filename}.js.LICENSE */`|The banner text that points to the extracted file and will be added on top of the original file. Can be `false` (no banner), a `{String}`, or a `{Function<(string) -> {String}` that will be called with the filename where extracted comments have been stored. Will be wrapped into comment|
 
 ### `warningsFilter`

src/uglify/minify.js

@@ -38,12 +38,16 @@ const buildComments = (options, uglifyOptions, extractedComments) => {
   const commentsOpts = uglifyOptions.output.comments;
 
   if (
+    typeof options.extractComments === 'boolean' ||
     typeof options.extractComments === 'string' ||
     options.extractComments instanceof RegExp
   ) {
     // extractComments specifies the extract condition and commentsOpts specifies the preserve condition
     condition.preserve = commentsOpts;
     condition.extract = options.extractComments;
+  } else if (typeof options.extractComments === 'function') {
+    condition.preserve = false;
+    condition.extract = options.extractComments;
   } else if (
     Object.prototype.hasOwnProperty.call(options.extractComments, 'condition')
   ) {

test/__snapshots__/extract-comments-options.test.js.snap

@@ -0,0 +1,98 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`errors 1`] = `Array []`;
+
+exports[`errors 2`] = `Array []`;
+
+exports[`errors 3`] = `Array []`;
+
+exports[`errors 4`] = `Array []`;
+
+exports[`errors 5`] = `Array []`;
+
+exports[`test.js 1`] = `
+"/*! For license information please see test.js.LICENSE */
+var foo=1;"
+`;
+
+exports[`test.js 2`] = `"var foo=1;"`;
+
+exports[`test.js 3`] = `
+"/*! For license information please see test.js.LICENSE */
+var foo=1;"
+`;
+
+exports[`test.js 4`] = `
+"/*! For license information please see test.js.LICENSE */
+var foo=1;"
+`;
+
+exports[`test.js 5`] = `
+"/*! For license information please see test1.js.LICENSE */
+var foo=1;"
+`;
+
+exports[`test.js 6`] = `
+"/*! License information can be found in test.license.js */
+var foo=1;"
+`;
+
+exports[`test.js.LICENSE 1`] = `
+"// Comment
+"
+`;
+
+exports[`test.js.LICENSE 2`] = `
+"// Comment
+"
+`;
+
+exports[`test.license.js 1`] = `
+"// Comment
+"
+`;
+
+exports[`test1.js 1`] = `
+"/*! For license information please see test1.js.LICENSE */
+var foo=1;"
+`;
+
+exports[`test1.js 2`] = `
+"/*! For license information please see test1.js.LICENSE */
+var foo=1;"
+`;
+
+exports[`test1.js 3`] = `
+"/*! For license information please see test1.js.LICENSE */
+var foo=1;"
+`;
+
+exports[`test1.js.LICENSE 1`] = `
+"/* Comment */
+"
+`;
+
+exports[`test1.js.LICENSE 2`] = `
+"// foo
+"
+`;
+
+exports[`test1.js.LICENSE 3`] = `
+"/* Comment */
+"
+`;
+
+exports[`test1.js.LICENSE 4`] = `
+"/* Comment */
+"
+`;
+
+exports[`warnings 1`] = `Array []`;
+
+exports[`warnings 2`] = `Array []`;
+
+exports[`warnings 3`] = `Array []`;
+
+exports[`warnings 4`] = `Array []`;
+
+exports[`warnings 5`] = `Array []`;

test/extract-comments-options.test.js

@@ -71,6 +71,48 @@ describe('when options.extractComments', () => {
     });
   });
 
+  it('normalizes when options.extractComments is boolean', () => {
+    const pluginEnvironment = new PluginEnvironment();
+    const compilerEnv = pluginEnvironment.getEnvironmentStub();
+    compilerEnv.context = '';
+
+    const plugin = new UglifyJsPlugin({
+      uglifyOptions: {
+        output: {
+          comments: false,
+        },
+      },
+      extractComments: true,
+    });
+    plugin.apply(compilerEnv);
+    const [eventBinding] = pluginEnvironment.getEventBindings();
+    const chunkPluginEnvironment = new PluginEnvironment();
+    const compilation2 = chunkPluginEnvironment.getEnvironmentStub();
+    compilation2.assets = {
+      'test.js': {
+        source: () => '// Comment\nvar foo = 1;',
+      },
+      'test1.js': {
+        source: () => '/* Comment */\nvar foo = 1;',
+      },
+    };
+    compilation2.warnings = [];
+    compilation2.errors = [];
+
+    eventBinding.handler(compilation2);
+    [compilationEventBinding] = chunkPluginEnvironment.getEventBindings();
+
+    compilationEventBinding.handler([{
+      files: ['test.js', 'test1.js'],
+    }], () => {
+      expect(compilation2.assets['test.js'].source()).toMatchSnapshot('test.js');
+      expect(compilation2.assets['test1.js'].source()).toMatchSnapshot('test1.js');
+      expect(compilation2.assets['test1.js.LICENSE'].source()).toMatchSnapshot('test1.js.LICENSE');
+      expect(compilation2.errors).toMatchSnapshot('errors');
+      expect(compilation2.warnings).toMatchSnapshot('warnings');
+    });
+  });
+
   it('normalizes when options.extractComments is regex', () => {
     const pluginEnvironment = new PluginEnvironment();
     const compilerEnv = pluginEnvironment.getEnvironmentStub();
@@ -90,22 +132,116 @@ describe('when options.extractComments', () => {
     const compilation2 = chunkPluginEnvironment.getEnvironmentStub();
     compilation2.assets = {
       'test.js': {
-        source: () => 'var foo = 1;',
+        source: () => '// Comment\nvar foo = 1;',
+      },
+      'test1.js': {
+        source: () => '// foo\nvar foo = 1;',
       },
     };
+    compilation2.warnings = [];
     compilation2.errors = [];
 
     eventBinding.handler(compilation2);
     [compilationEventBinding] = chunkPluginEnvironment.getEventBindings();
 
     compilationEventBinding.handler([{
-      files: ['test.js'],
+      files: ['test.js', 'test1.js'],
     }], () => {
-      expect(compilation2.errors.length).toBe(0);
+      expect(compilation2.assets['test.js'].source()).toMatchSnapshot('test.js');
+      expect(compilation2.assets['test1.js'].source()).toMatchSnapshot('test1.js');
+      expect(compilation2.assets['test1.js.LICENSE'].source()).toMatchSnapshot('test1.js.LICENSE');
+      expect(compilation2.errors).toMatchSnapshot('errors');
+      expect(compilation2.warnings).toMatchSnapshot('warnings');
     });
   });
 
-  it('converts boolean options.extractComments.condition to function', () => {
+  it('normalizes when options.extractComments is string', () => {
+    const pluginEnvironment = new PluginEnvironment();
+    const compilerEnv = pluginEnvironment.getEnvironmentStub();
+    compilerEnv.context = '';
+
+    const plugin = new UglifyJsPlugin({
+      uglifyOptions: {
+        output: {
+          comments: false,
+        },
+      },
+      extractComments: 'all',
+    });
+    plugin.apply(compilerEnv);
+    const [eventBinding] = pluginEnvironment.getEventBindings();
+    const chunkPluginEnvironment = new PluginEnvironment();
+    const compilation2 = chunkPluginEnvironment.getEnvironmentStub();
+    compilation2.assets = {
+      'test.js': {
+        source: () => '// Comment\nvar foo = 1;',
+      },
+      'test1.js': {
+        source: () => '/* Comment */\nvar foo = 1;',
+      },
+    };
+    compilation2.warnings = [];
+    compilation2.errors = [];
+
+    eventBinding.handler(compilation2);
+    [compilationEventBinding] = chunkPluginEnvironment.getEventBindings();
+
+    compilationEventBinding.handler([{
+      files: ['test.js', 'test1.js'],
+    }], () => {
+      expect(compilation2.assets['test.js'].source()).toMatchSnapshot('test.js');
+      expect(compilation2.assets['test.js.LICENSE'].source()).toMatchSnapshot('test.js.LICENSE');
+      expect(compilation2.assets['test1.js'].source()).toMatchSnapshot('test1.js');
+      expect(compilation2.assets['test1.js.LICENSE'].source()).toMatchSnapshot('test1.js.LICENSE');
+      expect(compilation2.errors).toMatchSnapshot('errors');
+      expect(compilation2.warnings).toMatchSnapshot('warnings');
+    });
+  });
+
+  it('normalizes when options.extractComments is function', () => {
+    const pluginEnvironment = new PluginEnvironment();
+    const compilerEnv = pluginEnvironment.getEnvironmentStub();
+    compilerEnv.context = '';
+
+    const plugin = new UglifyJsPlugin({
+      uglifyOptions: {
+        output: {
+          comments: false,
+        },
+      },
+      extractComments: () => true,
+    });
+    plugin.apply(compilerEnv);
+    const [eventBinding] = pluginEnvironment.getEventBindings();
+    const chunkPluginEnvironment = new PluginEnvironment();
+    const compilation2 = chunkPluginEnvironment.getEnvironmentStub();
+    compilation2.assets = {
+      'test.js': {
+        source: () => '// Comment\nvar foo = 1;',
+      },
+      'test1.js': {
+        source: () => '/* Comment */\nvar foo = 1;',
+      },
+    };
+    compilation2.warnings = [];
+    compilation2.errors = [];
+
+    eventBinding.handler(compilation2);
+    [compilationEventBinding] = chunkPluginEnvironment.getEventBindings();
+
+    compilationEventBinding.handler([{
+      files: ['test.js', 'test1.js'],
+    }], () => {
+      expect(compilation2.assets['test.js'].source()).toMatchSnapshot('test.js');
+      expect(compilation2.assets['test1.js'].source()).toMatchSnapshot('test.js');
+      expect(compilation2.assets['test.js.LICENSE'].source()).toMatchSnapshot('test.js.LICENSE');
+      expect(compilation2.assets['test1.js.LICENSE'].source()).toMatchSnapshot('test1.js.LICENSE');
+      expect(compilation2.errors).toMatchSnapshot('errors');
+      expect(compilation2.warnings).toMatchSnapshot('warnings');
+    });
+  });
+
+  it('normalizes when options.extractComments is object', () => {
     const pluginEnvironment = new PluginEnvironment();
     const compilerEnv = pluginEnvironment.getEnvironmentStub();
     compilerEnv.context = '';
@@ -132,9 +268,10 @@ describe('when options.extractComments', () => {
     const compilation2 = chunkPluginEnvironment.getEnvironmentStub();
     compilation2.assets = {
       'test.js': {
-        source: () => 'var foo = 1;',
+        source: () => '// Comment\nvar foo = 1;',
       },
     };
+    compilation2.warnings = [];
     compilation2.errors = [];
 
     eventBinding.handler(compilation2);
@@ -143,7 +280,10 @@ describe('when options.extractComments', () => {
     compilationEventBinding.handler([{
       files: ['test.js'],
     }], () => {
-      expect(compilation2.errors.length).toBe(0);
+      expect(compilation2.assets['test.js'].source()).toMatchSnapshot('test.js');
+      expect(compilation2.assets['test.license.js'].source()).toMatchSnapshot('test.license.js');
+      expect(compilation2.errors).toMatchSnapshot('errors');
+      expect(compilation2.warnings).toMatchSnapshot('warnings');
     });
   });
 });