From 2c06f461aa9f0bee98f6337747c07b4a8434c25f Mon Sep 17 00:00:00 2001 From: Evilebot Tnawi Date: Fri, 17 Nov 2017 19:08:50 +0300 Subject: [PATCH] fix: pass of `{Boolean|Function}` parameters being ignored (`options.extractComments`) (#168) --- README.md | 2 +- src/uglify/minify.js | 4 + .../extract-comments-options.test.js.snap | 98 +++++++++++ test/extract-comments-options.test.js | 152 +++++++++++++++++- 4 files changed, 249 insertions(+), 7 deletions(-) create mode 100644 test/__snapshots__/extract-comments-options.test.js.snap diff --git a/README.md b/README.md index f8068759..953974d2 100644 --- a/README.md +++ b/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` diff --git a/src/uglify/minify.js b/src/uglify/minify.js index 334fb599..aa079967 100644 --- a/src/uglify/minify.js +++ b/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') ) { diff --git a/test/__snapshots__/extract-comments-options.test.js.snap b/test/__snapshots__/extract-comments-options.test.js.snap new file mode 100644 index 00000000..111ccb7b --- /dev/null +++ b/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 []`; diff --git a/test/extract-comments-options.test.js b/test/extract-comments-options.test.js index 18b72310..b13427f1 100644 --- a/test/extract-comments-options.test.js +++ b/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'); }); }); });