fix: improve comments handling in presets + api + gulp (#855)

* fix: improve comments handling presets + api * fix comment regex in gulp minify * update readme and test
babel · May 17, 2018 · 3b1a8f0 · 3b1a8f0
1 parent 367cc04
commit 3b1a8f0
Show file tree

Hide file tree

Showing 11 changed files with 78 additions and 44 deletions.
diff --git a/packages/babel-helper-to-multiple-sequence-expressions/src/index.js b/packages/babel-helper-to-multiple-sequence-expressions/src/index.js
@@ -26,6 +26,7 @@ module.exports = function(t) {
 
     function convert(nodes) {
       const exprs = [];
+      const comments = [];
 
       for (let i = 0; i < nodes.length; i++) {
         const bail = () => {
@@ -47,6 +48,9 @@ module.exports = function(t) {
         if (t.isExpression(node)) {
           exprs.push(node);
         } else if (t.isExpressionStatement(node)) {
+          if (node.leadingComments) {
+            comments.push(...node.leadingComments);
+          }
           if (node.expression) exprs.push(node.expression);
         } else if (t.isIfStatement(node)) {
           let consequent;
@@ -97,6 +101,14 @@ module.exports = function(t) {
         seq = t.sequenceExpression(exprs);
       }
 
+      /**
+       * collect all the comment ast nodes that are before expression
+       * statments and add it to the new generated node
+       */
+      if (seq) {
+        seq.leadingComments = comments;
+      }
+
       /* eslint-disable no-self-assign */
       seq = seq;
       return { seq };

diff --git a/packages/babel-minify/README.md b/packages/babel-minify/README.md
@@ -50,6 +50,7 @@ Refer [babel-preset-minify options](https://github.com/babel/minify/tree/master/
 + `minifyPreset`: Custom minify preset
 + `inputSourceMap`: Input Sourcemap
 + `sourceMaps`: [Boolean]
++ `comments`: [Function | RegExp | Boolean]
 
 ## CLI Options
 

diff --git a/packages/babel-minify/__tests__/__snapshots__/node-api-tests.js.snap b/packages/babel-minify/__tests__/__snapshots__/node-api-tests.js.snap
@@ -4,4 +4,8 @@ exports[`babel-minify Node API override default minify options 1`] = `"function
 
 exports[`babel-minify Node API override nested minify options 1`] = `"function foo(){const a=x(1),b=y(2);return z(a,b)}"`;
 
+exports[`babel-minify Node API preserve default comments 1`] = `"/* @license MIT */(function(){/*! mylib.js */(function(){})()})();"`;
+
+exports[`babel-minify Node API remove comments  1`] = `"var a=10;!function(){}();"`;
+
 exports[`babel-minify Node API simple usage 1`] = `"function foo(){const a=x(1),b=y(2);return z(a,b)}"`;
diff --git a/packages/babel-minify/__tests__/node-api-tests.js b/packages/babel-minify/__tests__/node-api-tests.js
@@ -28,4 +28,28 @@ describe("babel-minify Node API", () => {
     const minifyOpts = { mangle: { keepFnName: false } };
     expect(minify(sampleInput, minifyOpts).code).toMatchSnapshot();
   });
+
+  it("preserve default comments", () => {
+    const code = `
+      /* @license MIT */
+      (function() {
+        /*! mylib.js */
+        function a() {}
+        a();
+      })();
+    `;
+
+    expect(minify(code, {}).code).toMatchSnapshot();
+  });
+
+  it("remove comments ", () => {
+    const code = `
+      /* foo */
+      var a = 10;
+
+      !function(){}() // blah
+    `;
+
+    expect(minify(code, {}).code).toMatchSnapshot();
+  });
 });
diff --git a/packages/babel-minify/src/index.js b/packages/babel-minify/src/index.js
@@ -5,29 +5,49 @@ module.exports = function babelMinify(
   input,
   // Minify options passed to minifyPreset
   // defaults are handled in preset
-  options = {},
+  minifyOptions = {},
   // overrides and other options
   {
     minified = true,
     inputSourceMap,
     sourceMaps = false,
     sourceType = "script",
+    comments = /^\**!|@preserve|@licen[sc]e|@cc_on/,
 
     // to override the default babelCore used
     babel = babelCore,
 
     // to override the default minify preset used
-    minifyPreset = babelPresetMinify
+    minifyPreset = babelPresetMinify,
+
+    // passthrough to babel
+    filename,
+    filenameRelative
   } = {}
 ) {
   return babel.transformSync(input, {
     babelrc: false,
     configFile: false,
-    presets: [[minifyPreset, options]],
-    comments: false,
+    presets: [[minifyPreset, minifyOptions]],
+    shouldPrintComment(contents) {
+      return shouldPrintComment(contents, comments);
+    },
     inputSourceMap,
     sourceMaps,
     minified,
-    sourceType
+    sourceType,
+    filename,
+    filenameRelative
   });
 };
+
+function shouldPrintComment(contents, predicate) {
+  switch (typeof predicate) {
+    case "function":
+      return predicate(contents);
+    case "object":
+      return predicate.test(contents);
+    default:
+      return !!predicate;
+  }
+}
diff --git a/packages/babel-plugin-minify-simplify/__tests__/fixtures/merge-if-2/actual.js b/packages/babel-plugin-minify-simplify/__tests__/fixtures/merge-if-2/actual.js
@@ -1,8 +1,5 @@
-// FIXME: for some reason, the inner `if` statement gets indented 4 spaces.
-`
-    function foo() {
-      if (a) {
-          if (b()) return false;
-      } else if (c()) return true;
-    }
-  `
+function foo() {
+  if (a) {
+    if (b()) return false;
+  } else if (c()) return true;
+}
diff --git a/packages/babel-plugin-minify-simplify/__tests__/fixtures/merge-if-2/expected.js b/packages/babel-plugin-minify-simplify/__tests__/fixtures/merge-if-2/expected.js
@@ -1,7 +1,5 @@
-`
-    function foo() {
-      if (a) {
-          if (b()) return false;
-      } else if (c()) return true;
-    }
-  `;
+function foo() {
+  if (a) {
+    if (b()) return false;
+  } else if (c()) return true;
+}
diff --git a/packages/babel-preset-minify/__tests__/preset-tests.js b/packages/babel-preset-minify/__tests__/preset-tests.js
@@ -33,28 +33,6 @@ describe("preset", () => {
   `
   );
 
-  thePlugin(
-    "should fix remove comments",
-    `
-    var asdf = 1; // test
-  `,
-    `
-    var asdf = 1;
-  `
-  );
-
-  thePlugin(
-    "should keep license/preserve annotated comments",
-    `
-    /* @license */
-    var asdf = 1;
-  `,
-    `
-    /* @license */
-    var asdf = 1;
-  `
-  );
-
   thePlugin(
     "should fix issue#385 - impure if statements with Sequence and DCE",
     `

diff --git a/packages/babel-preset-minify/src/index.js b/packages/babel-preset-minify/src/index.js
@@ -99,7 +99,6 @@ function preset(context, _opts = {}) {
 
   return {
     minified: true,
-    comments: false,
     presets: [{ plugins }],
     passPerPreset: true
   };

diff --git a/packages/gulp-babel-minify/README.md b/packages/gulp-babel-minify/README.md
@@ -39,3 +39,4 @@ Default: `{}`
 
 + `babel`: Use a custom `@babel/core`
 + `minifyPreset`: Use a custom `babel-preset-minify`
++ `comments`: [Function | RegExp | Boolean]
diff --git a/packages/gulp-babel-minify/src/index.js b/packages/gulp-babel-minify/src/index.js
@@ -13,7 +13,7 @@ function gulpBabelMinify(
   {
     babel = babelCore,
     minifyPreset = babelPresetMinify,
-    comments = /preserve|licen(s|c)e/,
+    comments = /^\**!|@preserve|@licen[sc]e|@cc_on/,
     sourceType = "script"
   } = {}
 ) {