Repository: styled-components/babel-plugin-styled-components
Pull request #155 (2018-08-29 03:15:16)

 fix single line comments + interpolation issues
this one was a doozy... I did a few things here:

1. Each injected placeholder corresponding to an expression now
   has a unique index that can be tracked.

2. After all the cleanup is completed, I do a diff on which
   indices were removed.

3. That gets passed up the chain to the main minify function, which
   then removes the appropriate expressions from the template literal
   without culling correct ones.

Fixes https://github.com/styled-components/styled-components/issues/1949


Patch:
diff --git a/src/css/placeholderUtils.js b/src/css/placeholderUtils.js
index cfa59c6..10d1f75 100644
--- a/src/css/placeholderUtils.js
+++ b/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,
+]
diff --git a/src/minify/index.js b/src/minify/index.js
index 2f5995a..459fadb 100644
--- a/src/minify/index.js
+++ b/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,7 +89,12 @@ 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]
   }
 }
 
@@ -83,10 +102,10 @@ 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
   )
diff --git a/src/visitors/minify.js b/src/visitors/minify.js
index f484631..d3b5235 100644
--- a/src/visitors/minify.js
+++ b/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]
diff --git a/src/visitors/templateLiterals/transpile.js b/src/visitors/templateLiterals/transpile.js
index 2f0202a..a771667 100644
--- a/src/visitors/templateLiterals/transpile.js
+++ b/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 patch:
diff --git a/test/__snapshots__/index.test.js.snap b/test/__snapshots__/index.test.js.snap
index 6391a6b..66baf90 100644
--- a/test/__snapshots__/index.test.js.snap
+++ b/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\';
 
diff --git a/test/fixtures/minify-single-line-comments-with-interpolations/.babelrc b/test/fixtures/minify-single-line-comments-with-interpolations/.babelrc
new file mode 100644
index 0000000..b64d0b6
--- /dev/null
+++ b/test/fixtures/minify-single-line-comments-with-interpolations/.babelrc
@@ -0,0 +1,10 @@
+{
+    "plugins": [
+        [
+            "../../../src",
+            {
+                "minify": true
+            }
+        ]
+    ]
+}
\ No newline at end of file
diff --git a/test/fixtures/minify-single-line-comments-with-interpolations/index.js b/test/fixtures/minify-single-line-comments-with-interpolations/index.js
new file mode 100644
index 0000000..c431ae0
--- /dev/null
+++ b/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};
+`
diff --git a/test/minify/index.test.js b/test/minify/index.test.js
index 5740944..02647b1 100644
--- a/test/minify/index.test.js
+++ b/test/minify/index.test.js
@@ -2,9 +2,11 @@ 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', () => {
@@ -12,7 +14,9 @@ describe('minify utils', () => {
     })
 
     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,7 +25,9 @@ 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'
+      )
     })
   })
 
@@ -29,46 +35,59 @@ describe('minify utils', () => {
     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]])
     })
   })
 
@@ -76,12 +95,19 @@ describe('minify utils', () => {
     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)
     })
   })