Repository: styled-components/babel-plugin-styled-components
Pull request #55 (2017-06-11 17:05:49)

Strip comments properly from styled template strings
Fixes: https://github.com/styled-components/styled-components/issues/888

The new logic works like this:

- Joins quasis with placeholders
- Removes multi-line comments
- Splits by newlines
- Removes line-comments
- Filters lines with only line-comments
- Joins

This thus removes all comments and gives us greater
granularity for minification, since it's only a single
string we now operate on. This should prevent mangling
in the future.


Patch:
diff --git a/README.md b/README.md
index d89880f..59b3ecf 100644
--- a/README.md
+++ b/README.md
@@ -88,6 +88,8 @@ You can enable preprocessing with the `preprocess` option:
 
 ### Minification
 
+**This option is turned on by default! If you experience mangled CSS results, turn it off and open an issue please.**
+
 This plugin minifies your styles in the tagged template literals, giving you big bundle size savings. (note that you will not see the effect of minification in generated `<style>` tags, it solely affects the style strings inside your JS bundle)
 
 > This operation may potentially break your styles in some rare cases, so we recommend to keep this option enabled in development if it's enabled in the production build.
diff --git a/src/css/placeholderUtils.js b/src/css/placeholderUtils.js
index ab24698..f7bde4c 100644
--- a/src/css/placeholderUtils.js
+++ b/src/css/placeholderUtils.js
@@ -1,6 +1,9 @@
 // The capture group makes sure that the split contains the interpolation index
 const placeholderRegex = /__PLACEHOLDER_(\d+?)__/
 
+// Alternative regex that splits without a capture group
+const placeholderNonCapturingRegex = /__PLACEHOLDER_(?:\d+?)__/
+
 // This matches the global group w/o a selector
 const globalRulesetRegex = /^{([^}]*)}/
 
@@ -60,7 +63,8 @@ export const temporaryClassname = '__TEMPORARY_CLASSNAME__'
 export const containsPlaceholders = css => !!css.match(placeholderRegex)
 
 // Splits CSS by placeholders
-export const splitByPlaceholders = css => css.split(placeholderRegex)
+export const splitByPlaceholders = (css, capture = true) => css
+  .split(capture ? placeholderRegex : placeholderNonCapturingRegex)
 
 // Remove curly braces around global placeholders
 // We need to replace mixin-semicolons with newlines to not break browser CSS parsing
diff --git a/src/minify/index.js b/src/minify/index.js
new file mode 100644
index 0000000..f9d1360
--- /dev/null
+++ b/src/minify/index.js
@@ -0,0 +1,75 @@
+import { makePlaceholder, splitByPlaceholders } from '../css/placeholderUtils'
+
+const makeMultilineCommentRegex = newlinePattern => new RegExp('\\/\\*(.|' + newlinePattern + ')*?\\*\\/', 'g')
+const lineCommentStart = /\/\//g
+
+// Counts occurences of substr inside str
+const countOccurences = (str, substr) => str.split(substr).length - 1
+
+// Joins substrings until predicate returns true
+const reduceSubstr = (substrs, join, predicate) => {
+  const length = substrs.length
+  let res = substrs[0]
+
+  if (length === 1) {
+    return res
+  }
+
+  for (let i = 1; i < length; i++) {
+    if (predicate(res)) {
+      break
+    }
+
+    res += join + substrs[i]
+  }
+
+  return res
+}
+
+// Joins at comment starts when it's inside a string or parantheses
+// effectively removing line comments
+export const stripLineComment = line => (
+  reduceSubstr(line.split(lineCommentStart), '//', str => (
+    !str.endsWith(':') && // NOTE: This is another guard against urls, if they're not inside strings or parantheses.
+    countOccurences(str, '\'') % 2 === 0 &&
+    countOccurences(str, '\"') % 2 === 0 &&
+    countOccurences(str, '(') === countOccurences(str, ')')
+  ))
+)
+
+// Detects lines that are exclusively line comments
+const isLineComment = line => line.trim().startsWith('//')
+
+// Creates a minifier with a certain linebreak pattern
+const minify = linebreakPattern => {
+  const linebreakRegex = new RegExp(linebreakPattern + '\\s*', 'g')
+  const multilineCommentRegex = makeMultilineCommentRegex(linebreakPattern)
+
+  return code => {
+    const lines = code
+      .replace(multilineCommentRegex, '\n') // Remove multiline comments
+      .split(linebreakRegex) // Split at newlines
+      .map(stripLineComment) // Remove line comments inside text
+
+    return lines
+      .filter(line => !isLineComment(line)) // Removes lines containing only line comments
+      .join('')
+  }
+}
+
+export const minifyRaw = minify('(?:\\\\r|\\\\n|\\r|\\n)')
+export const minifyCooked = minify('[\\r\\n]')
+
+export const minifyRawValues = rawValues => splitByPlaceholders(
+  minifyRaw(
+    rawValues.join(makePlaceholder(123))
+  ),
+  false
+)
+
+export const minifyCookedValues = cookedValues => splitByPlaceholders(
+  minifyCooked(
+    cookedValues.join(makePlaceholder(123))
+  ),
+  false
+)
diff --git a/src/visitors/minify.js b/src/visitors/minify.js
index f7128a7..1a5cb7d 100644
--- a/src/visitors/minify.js
+++ b/src/visitors/minify.js
@@ -1,19 +1,8 @@
 import * as t from 'babel-types'
-import {
-  useMinify,
-  useCSSPreprocessor
-} from '../utils/options'
-import { isStyled, isHelper } from '../utils/detectors'
-
-const minify = (linebreak) => {
-  const regex = new RegExp(linebreak + '\\s*', 'g')
-  return (code) => code.split(regex).filter(line => line.length > 0).map((line) => {
-    return line.indexOf('//') === -1 ? line : line + '\n';
-  }).join('')
-}
 
-const minifyRaw = minify('(?:\\\\r|\\\\n|\\r|\\n)')
-const minifyCooked = minify('[\\r\\n]')
+import { useMinify, useCSSPreprocessor } from '../utils/options'
+import { isStyled, isHelper } from '../utils/detectors'
+import { minifyRawValues, minifyCookedValues } from '../minify'
 
 export default (path, state) => {
   if (
@@ -25,9 +14,16 @@ export default (path, state) => {
     )
   ) {
     const templateLiteral = path.node.quasi
-    for (let element of templateLiteral.quasis) {
-      element.value.raw = minifyRaw(element.value.raw)
-      element.value.cooked = minifyCooked(element.value.cooked)
+    const quasisLength = templateLiteral.quasis.length
+
+    const rawValuesMinified = minifyRawValues(templateLiteral.quasis.map(x => x.value.raw))
+    const cookedValuesMinfified = minifyCookedValues(templateLiteral.quasis.map(x => x.value.cooked))
+
+    for (let i = 0; i < quasisLength; i++) {
+      const element = templateLiteral.quasis[i]
+
+      element.value.raw = rawValuesMinified[i]
+      element.value.cooked = cookedValuesMinfified[i]
     }
   }
 }


Test patch:
diff --git a/test/fixtures/08-minify-css-to-use-with-transpilation/after.js b/test/fixtures/08-minify-css-to-use-with-transpilation/after.js
index c645c62..d3bc61b 100644
--- a/test/fixtures/08-minify-css-to-use-with-transpilation/after.js
+++ b/test/fixtures/08-minify-css-to-use-with-transpilation/after.js
@@ -3,7 +3,7 @@
 var _templateObject = _taggedTemplateLiteral(['width: 100%;'], ['width: 100%;']),
     _templateObject2 = _taggedTemplateLiteral(['content: "  ', '  ";'], ['content: "  ', '  ";']),
     _templateObject3 = _taggedTemplateLiteral(['content: "  ', '  ";color: red;'], ['content: "  ', '  ";color: red;']),
-    _templateObject4 = _taggedTemplateLiteral(['// comment\ncolor: red;'], ['// comment\ncolor: red;']),
+    _templateObject4 = _taggedTemplateLiteral(['color: red;'], ['color: red;']),
     _templateObject5 = _taggedTemplateLiteral(['&:hover {color: blue;}'], ['&:hover {color: blue;}']);
 
 var _styledComponents = require('styled-components');
diff --git a/test/fixtures/09-minify-css-to-use-without-transpilation/after.js b/test/fixtures/09-minify-css-to-use-without-transpilation/after.js
index 7357c88..b0e2585 100644
--- a/test/fixtures/09-minify-css-to-use-without-transpilation/after.js
+++ b/test/fixtures/09-minify-css-to-use-without-transpilation/after.js
@@ -2,11 +2,12 @@ import styled from 'styled-components';
 
 const Simple = styled.div`width: 100%;`;
 
-const Interpolation = styled.div`content: "  ${props => props.text}  ";`;
+const Interpolation = styled.div`content: "https://test.com/${props => props.endpoint}";`;
 
 const SpecialCharacters = styled.div`content: "  ${props => props.text}  ";color: red;`;
 
-const Comment = styled.div`width: 100%;// comment
-color: red;`;
+const Comment = styled.div`width: 100%;color: red;`;
 
 const Parens = styled.div`&:hover {color: blue;}color: red;`;
+
+const UrlComments = styled.div`color: red;background: red;border: 1px solid green;`;
diff --git a/test/fixtures/09-minify-css-to-use-without-transpilation/before.js b/test/fixtures/09-minify-css-to-use-without-transpilation/before.js
index 4a93290..02aa35d 100644
--- a/test/fixtures/09-minify-css-to-use-without-transpilation/before.js
+++ b/test/fixtures/09-minify-css-to-use-without-transpilation/before.js
@@ -5,7 +5,7 @@ const Simple = styled.div`
 `;
 
 const Interpolation = styled.div`
-  content: "  ${props => props.text}  ";
+  content: "https://test.com/${props => props.endpoint}";
 `;
 
 const SpecialCharacters = styled.div`
@@ -24,3 +24,13 @@ const Parens = styled.div`
   }
   color: red;
 `;
+
+const UrlComments = styled.div`
+  color: red;
+  /* // */
+  background: red;
+  /* comment 1 */
+  /* comment 2 */
+  // comment 3
+  border: 1px solid green;
+`;
diff --git a/test/minify/index.test.js b/test/minify/index.test.js
new file mode 100644
index 0000000..3aa223b
--- /dev/null
+++ b/test/minify/index.test.js
@@ -0,0 +1,77 @@
+import {
+  stripLineComment,
+  minifyRaw,
+  minifyCooked
+} from '../../src/minify'
+
+describe('minify utils', () => {
+  describe('stripLineComment', () => {
+    it('splits a line by potential comment starts and joins until one is an actual comment', () => {
+      expect(stripLineComment('abc def//ghi//jkl')).toBe('abc def')
+    })
+
+    it('ignores comment markers that are inside strings', () => {
+      expect(stripLineComment('abc def"//"ghi\'//\'jkl//the end')).toBe('abc def"//"ghi\'//\'jkl')
+      expect(stripLineComment('abc def"//"')).toBe('abc def"//"')
+    })
+
+    it('ignores comment markers that are inside parantheses', () => {
+      expect(stripLineComment('bla (//) bla//the end')).toBe('bla (//) bla')
+    })
+
+    it('ignores even unescaped URLs', () => {
+      expect(stripLineComment('https://test.com// comment//')).toBe('https://test.com')
+    })
+  })
+
+  describe('minify(Raw|Cooked)', () => {
+    it('Removes multi-line comments', () => {
+      const input = 'this is a/* ignore me please */test'
+      const expected = 'this is atest' // NOTE: They're replaced with newlines, and newlines are joined
+      const actual = minifyRaw(input)
+
+      expect(actual).toBe(expected)
+      expect(actual).toBe(minifyCooked(input))
+    })
+
+    it('Joins all lines of code', () => {
+      const input = 'this\nis\na/* ignore me \n please */\ntest'
+      const expected = 'thisisatest'
+      const actual = minifyRaw(input)
+
+      expect(actual).toBe(expected)
+      expect(actual).toBe(minifyCooked(input))
+    })
+
+    it('Removes line comments filling an entire line', () => {
+      const input = 'line one\n// remove this comment\nline two'
+      const expected = 'line oneline two'
+      const actual = minifyRaw(input)
+
+      expect(actual).toBe(expected)
+      expect(actual).toBe(minifyCooked(input))
+    })
+
+    it('Removes line comments at the end of lines of code', () => {
+      const input = 'valid line with // a comment\nout comments'
+      const expected = 'valid line with out comments'
+      const actual = minifyRaw(input)
+
+      expect(actual).toBe(expected)
+      expect(actual).toBe(minifyCooked(input))
+    })
+  })
+
+  describe('minifyRaw', () => {
+    it('works with raw escape codes', () => {
+      const input = 'this\\nis\\na/* ignore me \\n please */\\ntest'
+      const expected = 'thisisatest'
+      const actual = minifyRaw(input)
+
+      expect(minifyRaw(input)).toBe(expected)
+
+      // NOTE: This is just a sanity check
+      expect(minifyCooked(input)).toBe('this\\nis\\na\\ntest')
+    })
+  })
+})