Repository: webpack-contrib/uglifyjs-webpack-plugin
Pull request #387 (2018-12-27 12:15:08)

fix: don't crash when no extracted comments
<!--
  HOLY CRAP a Pull Request. We ❤️ those!

  If you remove or skip this template, you'll make the 🐼 sad and the mighty god
  of Github will appear and pile-drive the close button from a great height
  while making animal noises.

  Please place an x (no spaces!) in all [ ] that apply
-->

This PR contains a:

- [x] **bugfix**
- [ ] new **feature**
- [ ] **code refactor**
- [x] **test update** <!-- if bug or feature is checked, this should be too -->
- [ ] **typo fix**
- [ ] **metadata update**

### Motivation / Use-Case

Backport from terser plugin

### Breaking Changes

No

### Additional Info

No

codecov[bot]: # [Codecov](https://codecov.io/gh/webpack-contrib/uglifyjs-webpack-plugin/pull/387?src=pr&el=h1) Report
> Merging [#387](https://codecov.io/gh/webpack-contrib/uglifyjs-webpack-plugin/pull/387?src=pr&el=desc) into [master](https://codecov.io/gh/webpack-contrib/uglifyjs-webpack-plugin/commit/e3eff7675cb1f71936cd150f359bae1ac088582d?src=pr&el=desc) will **increase** coverage by `0.01%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/webpack-contrib/uglifyjs-webpack-plugin/pull/387/graphs/tree.svg?width=650&token=eJ1YcVlo0C&height=150&src=pr)](https://codecov.io/gh/webpack-contrib/uglifyjs-webpack-plugin/pull/387?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #387      +/-   ##
==========================================
+ Coverage   98.56%   98.57%   +0.01%     
==========================================
  Files           5        5              
  Lines         278      281       +3     
  Branches      107      110       +3     
==========================================
+ Hits          274      277       +3     
  Misses          4        4
```


| [Impacted Files](https://codecov.io/gh/webpack-contrib/uglifyjs-webpack-plugin/pull/387?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/index.js](https://codecov.io/gh/webpack-contrib/uglifyjs-webpack-plugin/pull/387/diff?src=pr&el=tree#diff-c3JjL2luZGV4Lmpz) | `98.06% <100%> (+0.03%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/webpack-contrib/uglifyjs-webpack-plugin/pull/387?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/webpack-contrib/uglifyjs-webpack-plugin/pull/387?src=pr&el=footer). Last update [e3eff76...2fd5af6](https://codecov.io/gh/webpack-contrib/uglifyjs-webpack-plugin/pull/387?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

codecov[bot]: # [Codecov](https://codecov.io/gh/webpack-contrib/uglifyjs-webpack-plugin/pull/387?src=pr&el=h1) Report
> Merging [#387](https://codecov.io/gh/webpack-contrib/uglifyjs-webpack-plugin/pull/387?src=pr&el=desc) into [master](https://codecov.io/gh/webpack-contrib/uglifyjs-webpack-plugin/commit/e3eff7675cb1f71936cd150f359bae1ac088582d?src=pr&el=desc) will **increase** coverage by `0.01%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/webpack-contrib/uglifyjs-webpack-plugin/pull/387/graphs/tree.svg?width=650&token=eJ1YcVlo0C&height=150&src=pr)](https://codecov.io/gh/webpack-contrib/uglifyjs-webpack-plugin/pull/387?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #387      +/-   ##
==========================================
+ Coverage   98.56%   98.57%   +0.01%     
==========================================
  Files           5        5              
  Lines         278      281       +3     
  Branches      107      110       +3     
==========================================
+ Hits          274      277       +3     
  Misses          4        4
```


| [Impacted Files](https://codecov.io/gh/webpack-contrib/uglifyjs-webpack-plugin/pull/387?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/index.js](https://codecov.io/gh/webpack-contrib/uglifyjs-webpack-plugin/pull/387/diff?src=pr&el=tree#diff-c3JjL2luZGV4Lmpz) | `98.06% <100%> (+0.03%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/webpack-contrib/uglifyjs-webpack-plugin/pull/387?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/webpack-contrib/uglifyjs-webpack-plugin/pull/387?src=pr&el=footer). Last update [e3eff76...2fd5af6](https://codecov.io/gh/webpack-contrib/uglifyjs-webpack-plugin/pull/387?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/src/index.js b/src/index.js
index 4437535..19bdbfc 100644
--- a/src/index.js
+++ b/src/index.js
@@ -306,7 +306,11 @@ class UglifyJsPlugin {
           }
 
           // Write extracted comments to commentsFile
-          if (commentsFile && extractedComments.length > 0) {
+          if (
+            commentsFile &&
+            extractedComments &&
+            extractedComments.length > 0
+          ) {
             if (commentsFile in compilation.assets) {
               const commentsFileSource = compilation.assets[
                 commentsFile


Test patch:
diff --git a/test/__snapshots__/minify-option.test.js.snap b/test/__snapshots__/minify-option.test.js.snap
index ea54705..4bd200b 100644
--- a/test/__snapshots__/minify-option.test.js.snap
+++ b/test/__snapshots__/minify-option.test.js.snap
@@ -183,6 +183,12 @@ exports[`when applied with \`minify\` option matches snapshot for \`terser\` min
 
 exports[`when applied with \`minify\` option matches snapshot for \`terser\` minifier: warnings 1`] = `Array []`;
 
+exports[`when applied with \`minify\` option matches snapshot for \`uglify-js\` minifier while extracting comments: errors 1`] = `Array []`;
+
+exports[`when applied with \`minify\` option matches snapshot for \`uglify-js\` minifier while extracting comments: main.js 1`] = `"!function(e){var t={};function n(r){if(t[r])return t[r].exports;var o=t[r]={i:r,l:!1,exports:{}};return e[r].call(o.exports,o,o.exports,n),o.l=!0,o.exports}n.m=e,n.c=t,n.d=function(e,t,r){n.o(e,t)||Object.defineProperty(e,t,{enumerable:!0,get:r})},n.r=function(e){\\"undefined\\"!=typeof Symbol&&Symbol.toStringTag&&Object.defineProperty(e,Symbol.toStringTag,{value:\\"Module\\"}),Object.defineProperty(e,\\"__esModule\\",{value:!0})},n.t=function(e,t){if(1&t&&(e=n(e)),8&t)return e;if(4&t&&\\"object\\"==typeof e&&e&&e.__esModule)return e;var r=Object.create(null);if(n.r(r),Object.defineProperty(r,\\"default\\",{enumerable:!0,value:e}),2&t&&\\"string\\"!=typeof e)for(var o in e)n.d(r,o,function(t){return e[t]}.bind(null,o));return r},n.n=function(e){var t=e&&e.__esModule?function(){return e.default}:function(){return e};return n.d(t,\\"a\\",t),t},n.o=function(e,t){return Object.prototype.hasOwnProperty.call(e,t)},n.p=\\"\\",n(n.s=0)}([function(e,t){e.exports=function(){var baz=document.getElementById(\\"root\\").innerHTML;document.getElementById(\\"demo\\").innerHTML=\\"Paragraph changed.\\"+baz}}]);"`;
+
+exports[`when applied with \`minify\` option matches snapshot for \`uglify-js\` minifier while extracting comments: warnings 1`] = `Array []`;
+
 exports[`when applied with \`minify\` option matches snapshot for errors into \`minify\` option and \`parallel\` is \`true\`: errors 1`] = `
 Array [
   "Error: main.js from UglifyJs
diff --git a/test/minify-option.test.js b/test/minify-option.test.js
index 83b7300..3b24e2b 100644
--- a/test/minify-option.test.js
+++ b/test/minify-option.test.js
@@ -212,4 +212,43 @@ describe('when applied with `minify` option', () => {
       }
     });
   });
+
+  it('matches snapshot for `uglify-js` minifier while extracting comments', () => {
+    const compiler = createCompiler({
+      entry: `${__dirname}/fixtures/minify/es5.js`,
+      output: {
+        path: `${__dirname}/dist-uglify-js`,
+        filename: '[name].js',
+        chunkFilename: '[id].[name].js',
+      },
+    });
+
+    new UglifyJsPlugin({
+      extractComments: true,
+      minify(file) {
+        // eslint-disable-next-line global-require
+        return require('terser').minify(file, {
+          mangle: {
+            reserved: ['baz'],
+          },
+        });
+      },
+    }).apply(compiler);
+
+    return compile(compiler).then((stats) => {
+      const errors = stats.compilation.errors.map(cleanErrorStack);
+      const warnings = stats.compilation.warnings.map(cleanErrorStack);
+
+      expect(errors).toMatchSnapshot('errors');
+      expect(warnings).toMatchSnapshot('warnings');
+
+      for (const file in stats.compilation.assets) {
+        if (
+          Object.prototype.hasOwnProperty.call(stats.compilation.assets, file)
+        ) {
+          expect(stats.compilation.assets[file].source()).toMatchSnapshot(file);
+        }
+      }
+    });
+  });
 });