Merge pull request #155 from styled-components/minify-single-line-comment

 fix single line comments + interpolation issues

Author: kitten

src/css/placeholderUtils.js

@@ -1,12 +1,14 @@
 // The capture group makes sure that the split contains the interpolation index
-const placeholderRegex = /__PLACEHOLDER_(\d+?)__/
+export const placeholderRegex = /(?:__PLACEHOLDER_(\d+)__)/g
 
 // Alternative regex that splits without a capture group
-const placeholderNonCapturingRegex = /__PLACEHOLDER_(?:\d+?)__/
+const placeholderNonCapturingRegex = /__PLACEHOLDER_(?:\d+)__/g
 
 // Generates a placeholder from an index
 export const makePlaceholder = index => `__PLACEHOLDER_${index}__`
 
 // Splits CSS by placeholders
-export const splitByPlaceholders = (css, capture = true) =>
-  css.split(capture ? placeholderRegex : placeholderNonCapturingRegex)
+export const splitByPlaceholders = ([css, ...rest], capture = true) => [
+  css.split(capture ? placeholderRegex : placeholderNonCapturingRegex),
+  ...rest,
+]

src/minify/index.js

@@ -1,4 +1,18 @@
-import { makePlaceholder, splitByPlaceholders } from '../css/placeholderUtils'
+import difference from 'lodash/difference'
+
+import {
+  makePlaceholder,
+  placeholderRegex,
+  splitByPlaceholders,
+} from '../css/placeholderUtils'
+
+const injectUniquePlaceholders = strArr => {
+  let i = 0
+
+  return strArr.reduce((str, val, index, arr) => {
+    return str + val + (index < arr.length - 1 ? makePlaceholder(i++) : '')
+  }, '')
+}
 
 const makeMultilineCommentRegex = newlinePattern =>
   new RegExp('\\/\\*[^!](.|' + newlinePattern + ')*?\\*\\/', 'g')
@@ -75,18 +89,23 @@ const minify = linebreakPattern => {
       .map(stripLineComment) // Remove line comments inside text
       .join(' ') // Rejoin all lines
 
-    return compressSymbols(newCode)
+    const eliminatedExpressionIndices = difference(
+      code.match(placeholderRegex),
+      newCode.match(placeholderRegex)
+    ).map(x => parseInt(x.match(/\d+/)[0], 10))
+
+    return [compressSymbols(newCode), eliminatedExpressionIndices]
   }
 }
 
 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)
+  splitByPlaceholders(minifyRaw(injectUniquePlaceholders(rawValues)), false)
 
 export const minifyCookedValues = cookedValues =>
   splitByPlaceholders(
-    minifyCooked(cookedValues.join(makePlaceholder(123))),
+    minifyCooked(injectUniquePlaceholders(cookedValues)),
     false
   )

src/visitors/minify.js

@@ -10,12 +10,18 @@ export default t => (path, state) => {
     const templateLiteral = path.node.quasi
     const quasisLength = templateLiteral.quasis.length
 
-    const rawValuesMinified = minifyRawValues(
+    const [rawValuesMinified] = minifyRawValues(
       templateLiteral.quasis.map(x => x.value.raw)
     )
-    const cookedValuesMinfified = minifyCookedValues(
-      templateLiteral.quasis.map(x => x.value.cooked)
-    )
+
+    const [
+      cookedValuesMinfified,
+      eliminatedExpressionIndices,
+    ] = minifyCookedValues(templateLiteral.quasis.map(x => x.value.cooked))
+
+    eliminatedExpressionIndices.forEach((expressionIndex, iteration) => {
+      templateLiteral.expressions.splice(expressionIndex - iteration, 1)
+    })
 
     for (let i = 0; i < quasisLength; i++) {
       const element = templateLiteral.quasis[i]

src/visitors/templateLiterals/transpile.js

@@ -8,7 +8,9 @@ export default t => (path, state) => {
     } = path.node
 
     const values = t.arrayExpression(
-      quasis.map(quasi => t.stringLiteral(quasi.value.cooked))
+      quasis
+        .filter(quasi => quasi.value.cooked !== undefined)
+        .map(quasi => t.stringLiteral(quasi.value.cooked))
     )
 
     path.replaceWith(t.callExpression(callee, [values, ...expressions]))

test/__snapshots__/index.test.js.snap

@@ -158,6 +158,45 @@ const Parens = styled(\'div\')\`&:hover{color:blue;}color:red;\`;
 const UrlComments = styled(\'div\')\`color:red;background:red;border:1px solid green;\`;"
 `;
 
+exports[`fixtures should minify single line comments with interpolations 1`] = `
+"import styled from \'styled-components/no-tags\';
+
+const Test1 = styled(\'div\').withConfig({
+  displayName: \'minify-single-line-comments-with-interpolations__Test1\',
+  componentId: \'sc-1p41dy5-0\'
+})([\'width:100%;\']);
+
+const Test2 = styled(\'div\').withConfig({
+  displayName: \'minify-single-line-comments-with-interpolations__Test2\',
+  componentId: \'sc-1p41dy5-1\'
+})([\'width:100%;\']);
+
+const Test3 = styled(\'div\').withConfig({
+  displayName: \'minify-single-line-comments-with-interpolations__Test3\',
+  componentId: \'sc-1p41dy5-2\'
+})([\'width:100%;\', \';\'], \'red\');
+
+const Test4 = styled(\'div\').withConfig({
+  displayName: \'minify-single-line-comments-with-interpolations__Test4\',
+  componentId: \'sc-1p41dy5-3\'
+})([\'width:100%;\']);
+
+const Test5 = styled(\'div\').withConfig({
+  displayName: \'minify-single-line-comments-with-interpolations__Test5\',
+  componentId: \'sc-1p41dy5-4\'
+})([\'width:100%;\']);
+
+const Test6 = styled(\'div\').withConfig({
+  displayName: \'minify-single-line-comments-with-interpolations__Test6\',
+  componentId: \'sc-1p41dy5-5\'
+})([\'background:url(\"https://google.com\");width:100%;\', \' \'], \'green\');
+
+const Test7 = styled(\'div\').withConfig({
+  displayName: \'minify-single-line-comments-with-interpolations__Test7\',
+  componentId: \'sc-1p41dy5-6\'
+})([\'background:url(\"https://google.com\");width:\', \';\', \'  height:\', \';\'], p => p.props.width, \'green\', p => p.props.height);"
+`;
+
 exports[`fixtures should not use private api if not required 1`] = `
 "import styled from \'styled-components/no-tags\';
 

test/fixtures/minify-single-line-comments-with-interpolations/.babelrc

@@ -0,0 +1,10 @@
+{
+    "plugins": [
+        [
+            "../../../src",
+            {
+                "minify": true
+            }
+        ]
+    ]
+}

test/fixtures/minify-single-line-comments-with-interpolations/index.js

@@ -0,0 +1,40 @@
+import styled from 'styled-components'
+
+const Test1 = styled.div`
+  width: 100%;
+  // color: ${'red'};
+`
+
+const Test2 = styled.div`
+  width: 100%;
+  // color: pale${'red'};
+`
+
+const Test3 = styled.div`
+  width: 100%;
+  // color
+  ${'red'};
+`
+
+const Test4 = styled.div`
+  width: 100%;
+  // color: ${'red'}-blue;
+`
+
+const Test5 = styled.div`
+  width: 100%;
+  // color: ${'red'}${'blue'};
+`
+
+const Test6 = styled.div`
+  background: url("https://google.com");
+  width: 100%;
+  ${'green'} // color: ${'red'}${'blue'};
+`
+
+const Test7 = styled.div`
+  background: url("https://google.com");
+  width: ${p => p.props.width};
+  ${'green'} // color: ${'red'}${'blue'};
+  height: ${p => p.props.height};
+`

test/minify/index.test.js

@@ -2,17 +2,21 @@ import {
   stripLineComment,
   minifyRaw,
   minifyCooked,
-  compressSymbols
+  compressSymbols,
 } from '../../src/minify'
 
+import { makePlaceholder } from '../../src/css/placeholderUtils'
+
 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"//"ghi\'//\'jkl//the end')).toBe(
+        'abc def"//"ghi\'//\'jkl'
+      )
       expect(stripLineComment('abc def"//"')).toBe('abc def"//"')
     })
 
@@ -21,67 +25,89 @@ describe('minify utils', () => {
     })
 
     it('ignores even unescaped URLs', () => {
-      expect(stripLineComment('https://test.com// comment//')).toBe('https://test.com')
+      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 a test' // NOTE: They're replaced with newlines, and newlines are joined
-      const actual = minifyRaw(input)
+      const [actual] = minifyRaw(input)
 
       expect(actual).toBe(expected)
-      expect(actual).toBe(minifyCooked(input))
+      expect(actual).toBe(minifyCooked(input)[0])
     })
 
     it('Joins all lines of code', () => {
       const input = 'this\nis\na/* ignore me \n please */\ntest'
       const expected = 'this is a test'
-      const actual = minifyRaw(input)
+      const [actual] = minifyRaw(input)
 
       expect(actual).toBe(expected)
-      expect(actual).toBe(minifyCooked(input))
+      expect(actual).toBe(minifyCooked(input)[0])
     })
 
     it('Removes line comments filling an entire line', () => {
       const input = 'line one\n// remove this comment\nline two'
       const expected = 'line one line two'
-      const actual = minifyRaw(input)
+      const [actual] = minifyRaw(input)
 
       expect(actual).toBe(expected)
-      expect(actual).toBe(minifyCooked(input))
+      expect(actual).toBe(minifyCooked(input)[0])
     })
 
     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)
+      const [actual] = minifyRaw(input)
 
       expect(actual).toBe(expected)
-      expect(actual).toBe(minifyCooked(input))
+      expect(actual).toBe(minifyCooked(input)[0])
     })
 
     it('Preserves multi-line comments starting with /*!', () => {
-      const input = 'this is a /*! dont ignore me please */ test/* but you can ignore me */'
+      const input =
+        'this is a /*! dont ignore me please */ test/* but you can ignore me */'
       const expected = 'this is a /*! dont ignore me please */ test'
-      const actual = minifyRaw(input)
+      const [actual] = minifyRaw(input)
+
+      expect(actual).toBe(expected)
+      expect(actual).toBe(minifyCooked(input)[0])
+    })
+
+    it('Returns the indices of removed placeholders (expressions)', () => {
+      const placeholder1 = makePlaceholder(0)
+      const placeholder2 = makePlaceholder(1)
+      const input = `this is some\ninput with ${placeholder1} and // ignored ${placeholder2}`
+      const expected = `this is some input with ${placeholder1} and `
+      const [actual, indices] = minifyRaw(input)
 
       expect(actual).toBe(expected)
-      expect(actual).toBe(minifyCooked(input))
+      expect(indices).toEqual([1])
+      expect(minifyCooked(input)).toEqual([actual, [1]])
     })
   })
 
   describe('minifyRaw', () => {
     it('works with raw escape codes', () => {
       const input = 'this\\nis\\na/* ignore me \\n please */\\ntest'
       const expected = 'this is a test'
-      const actual = minifyRaw(input)
+      const [actual] = minifyRaw(input)
+
+      expect(actual).toBe(expected)
+    })
+  })
 
-      expect(minifyRaw(input)).toBe(expected)
+  describe('minifyCooked', () => {
+    it('works with raw escape codes', () => {
+      const input = 'this\\nis\\na/* ignore me \\n please */\\ntest'
+      const expected = 'this\\nis\\na \\ntest'
+      const [actual] = minifyCooked(input)
 
-      // NOTE: This is just a sanity check
-      expect(minifyCooked(input)).toBe('this\\nis\\na \\ntest')
+      expect(actual).toBe(expected)
     })
   })