Repository: tkh44/emotion
Pull request #103 (2017-07-07 04:31:38)

Update placeholder regex
<!--
Thanks for your interest in the project. I appreciate bugs filed and PRs submitted!

Please make sure that you are familiar with and follow the Code of Conduct for
this project (found in the CODE_OF_CONDUCT.md file).

Also, please make sure you're familiar with and follow the instructions in the
contributing guidelines (found in the CONTRIBUTING.md file).

If you're new to contributing to open source projects, you might find this free
video course helpful: http://kcd.im/pull-request

Please fill out the information below to expedite the review and (hopefully)
merge of your pull request!
-->

<!-- What changes are being made? (What feature/bug is being fixed here?) -->
**What**:

Change placeholder regex from
```regex
/xxx(\S+)xxx/gm
```
to 
```regex
/xxx(\d+)xxx/gm
```

<!-- Why are these changes necessary? -->
**Why**:

Firstly, in some places the regex didn't have a plus, so having more than 10 composes or more than 10 dynamic values in attrs wouldn't work.

Secondly, the actual change to the regex, previously it matched multiple placeholders without spaces as a single match, now it only matches numbers so that can't happen. This allows you to put interpolations in composes next to each other without spaces.

<!-- How were these changes implemented? -->
**How**:

<!-- Have you done all of these things?  -->
**Checklist**:
<!-- add "N/A" to the end of each line that's irrelevant to your changes -->
<!-- to check an item, place an "x" in the box like so: "- [x] Documentation" -->
- [ ] Documentation N/A
- [x] Tests
- [x] Code complete

<!-- feel free to add additional comments -->


codecov-io: # [Codecov](https://codecov.io/gh/tkh44/emotion/pull/103?src=pr&el=h1) Report
> Merging [#103](https://codecov.io/gh/tkh44/emotion/pull/103?src=pr&el=desc) into [master](https://codecov.io/gh/tkh44/emotion/commit/40162d4cba00eeacb72f88024ac235d6705476fb?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/tkh44/emotion/pull/103/graphs/tree.svg?height=150&token=B1bWuYNE2t&src=pr&width=650)](https://codecov.io/gh/tkh44/emotion/pull/103?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##           master     #103   +/-   ##
=======================================
  Coverage   88.44%   88.44%           
=======================================
  Files          15       15           
  Lines         571      571           
  Branches      122      122           
=======================================
  Hits          505      505           
  Misses         53       53           
  Partials       13       13
```


| [Impacted Files](https://codecov.io/gh/tkh44/emotion/pull/103?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/babel.js](https://codecov.io/gh/tkh44/emotion/pull/103?src=pr&el=tree#diff-c3JjL2JhYmVsLmpz) | `100% <100%> (ø)` | :arrow_up: |
| [src/parser.js](https://codecov.io/gh/tkh44/emotion/pull/103?src=pr&el=tree#diff-c3JjL3BhcnNlci5qcw==) | `100% <100%> (ø)` | :arrow_up: |
| [src/attrs.js](https://codecov.io/gh/tkh44/emotion/pull/103?src=pr&el=tree#diff-c3JjL2F0dHJzLmpz) | `100% <100%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/tkh44/emotion/pull/103?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/tkh44/emotion/pull/103?src=pr&el=footer). Last update [40162d4...bb98d9f](https://codecov.io/gh/tkh44/emotion/pull/103?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/src/attrs.js b/src/attrs.js
index 3cfc5343..4a1f0d32 100644
--- a/src/attrs.js
+++ b/src/attrs.js
@@ -1,5 +1,5 @@
 export default function createAttrExpression (match, vars, composes, t) {
-  const placeholderRegex = /xxx(\S)xxx/gm
+  const placeholderRegex = /xxx(\d+)xxx/gm
   const propNameMatch = placeholderRegex.exec(match.propName)
   let propName = t.identifier(match.propName)
   if (propNameMatch) {
diff --git a/src/babel.js b/src/babel.js
index 57a42548..5f736b27 100644
--- a/src/babel.js
+++ b/src/babel.js
@@ -19,7 +19,7 @@ function joinExpressionsWithSpaces (expressions, t) {
 function parseDynamicValues (rules, t, options) {
   if (options.composes === undefined) options.composes = 0
   return rules.map(rule => {
-    const re = /xxx(\S+)xxx|attr\(([^,\s)]+)(?:\s*([^,)]*)?)(?:,\s*(\S+))?\)/gm
+    const re = /xxx(\d+)xxx|attr\(([^,\s)]+)(?:\s*([^,)]*)?)(?:,\s*(\S+))?\)/gm
     const VAR = 'VAR'
     const ATTR = 'ATTR'
     let match
diff --git a/src/parser.js b/src/parser.js
index 10337594..126e6b3c 100644
--- a/src/parser.js
+++ b/src/parser.js
@@ -39,20 +39,20 @@ export function parseCSS (
         if (decl.parent.selector !== `.${options.name}-${options.hash}`) {
           throw new Error('composes cannot be on nested selectors')
         }
-        if (!/xxx(\S+)xxx/gm.exec(decl.value)) {
+        if (!/xxx(\d+)xxx/gm.exec(decl.value)) {
           throw new Error('composes must be a interpolation')
         }
         if (decl.parent.nodes[0] !== decl) {
           throw new Error('composes must be the first rule')
         }
-        const numOfComposes = decl.value.match(/xxx(\S)xxx/gm).length
+        const numOfComposes = decl.value.match(/xxx(\d+)xxx/gm).length
         composes += numOfComposes
         vars += numOfComposes
         return decl.remove()
       }
     }
     if (!options.inlineMode) {
-      const match = /xxx(\S+)xxx/gm.exec(decl.value)
+      const match = /xxx(\d+)xxx/gm.exec(decl.value)
       if (match) {
         vars++
       }
@@ -60,7 +60,7 @@ export function parseCSS (
   })
   if (!options.inlineMode && vars === options.matches && !hasCssFunction) {
     root.walkDecls((decl) => {
-      decl.value = decl.value.replace(/xxx(\S+)xxx/gm, (match, p1) => {
+      decl.value = decl.value.replace(/xxx(\d+)xxx/gm, (match, p1) => {
         return `var(--${options.name}-${options.hash}-${p1})`
       })
     })


Test patch:
diff --git a/test/babel/__snapshots__/css.test.js.snap b/test/babel/__snapshots__/css.test.js.snap
index bc29bfb0..fc459457 100644
--- a/test/babel/__snapshots__/css.test.js.snap
+++ b/test/babel/__snapshots__/css.test.js.snap
@@ -131,6 +131,21 @@ const cls2 = css([\\"css-cls2-16o34vq\\"], [className], function createEmotionRu
 });"
 `;
 
+exports[`babel css inline lots of composes 1`] = `
+"
+const cls2 = css(['css-cls2-hal2lb', 'one-class', 'another-class', 'another-class', 'another-class', 'another-class', 'another-class', 'another-class', 'another-class', 'another-class', 'another-class', 'another-class', 'another-class', 'another-class', 'another-class', 'another-class', 'another-class', 'another-class', 'another-class', 'another-class', 'another-class', 'another-class', 'another-class', 'another-class', 'another-class'], ['center'], function createEmotionRules(x0) {
+  return [\`.css-cls2-hal2lb {
+          -webkit-justify-content: center;
+          -ms-flex-pack: center;
+          -webkit-box-pack: center;
+          justify-content: center;
+          -webkit-align-items: \${x0};
+          -ms-flex-align: \${x0};
+          -webkit-box-align: \${x0};
+          align-items: \${x0} }\`];
+});"
+`;
+
 exports[`babel css inline throws correct error when composes is not the first rule 1`] = `"unknown: Error: composes must be the first rule"`;
 
 exports[`babel css inline throws correct error when composes is on a nested selector 1`] = `"unknown: Error: composes cannot be on nested selectors"`;
diff --git a/test/babel/__snapshots__/styled.test.js.snap b/test/babel/__snapshots__/styled.test.js.snap
index 0b0d63f4..08c56846 100644
--- a/test/babel/__snapshots__/styled.test.js.snap
+++ b/test/babel/__snapshots__/styled.test.js.snap
@@ -128,6 +128,65 @@ exports[`babel styled component inline function call 1`] = `
 });"
 `;
 
+exports[`babel styled component inline lots of attrs with interpolated values 1`] = `
+"styled('input', ['css-1kdt7pt'], [marginProp, marginProp, marginProp, marginProp, marginProp, marginProp, marginProp, marginProp, marginProp, marginProp, marginProp, marginProp, marginProp, marginProp, marginProp, marginProp, marginProp, displayProp, function getProp(props) {
+       return props[marginProp];
+}, function getProp(props) {
+       return props[marginProp];
+}, function getProp(props) {
+       return props[marginProp];
+}, function getProp(props) {
+       return props[marginProp];
+}, function getProp(props) {
+       return props[marginProp];
+}, function getProp(props) {
+       return props[marginProp];
+}, function getProp(props) {
+       return props[marginProp];
+}, function getProp(props) {
+       return props[marginProp];
+}, function getProp(props) {
+       return props[marginProp];
+}, function getProp(props) {
+       return props[marginProp];
+}, function getProp(props) {
+       return props[marginProp];
+}, function getProp(props) {
+       return props[marginProp];
+}, function getProp(props) {
+       return props[marginProp];
+}, function getProp(props) {
+       return props[marginProp];
+}, function getProp(props) {
+       return props[marginProp];
+}, function getProp(props) {
+       return props[marginProp];
+}, function getProp(props) {
+       return props[marginProp];
+}, function getProp(props) {
+       return props[displayProp];
+}], function createEmotionStyledRules(x0, x1, x2, x3, x4, x5, x6, x7, x8, x9, x10, x11, x12, x13, x14, x15, x16, x17, x18, x19, x20, x21, x22, x23, x24, x25, x26, x27, x28, x29, x30, x31, x32, x33, x34, x35) {
+       return [\`.css-1kdt7pt { margin: \${x18};
+       margin: \${x19};
+       margin: \${x20};
+       margin: \${x21};
+       margin: \${x22};
+       margin: \${x23};
+       margin: \${x24};
+       margin: \${x25};
+       margin: \${x26};
+       margin: \${x27};
+       margin: \${x28};
+       margin: \${x29};
+       margin: \${x30};
+       margin: \${x31};
+       margin: \${x32};
+       margin: \${x33};
+       margin: \${x34};
+       display: \${x35}; }\`];
+});"
+`;
+
 exports[`babel styled component inline match works on multiple 1`] = `
 "styled('input', ['css-1vvfgof'], [function getMargin(props) {
         return (props.margin ? props.margin : '16') + 'px';
diff --git a/test/babel/css.test.js b/test/babel/css.test.js
index d805e384..29ed731b 100644
--- a/test/babel/css.test.js
+++ b/test/babel/css.test.js
@@ -57,6 +57,20 @@ describe('babel css', () => {
       expect(code).toMatchSnapshot()
     })
 
+    test('lots of composes', () => {
+      const basic = `
+        const cls2 = css\`
+          composes: \${'one-class'} \${'another-class'} \${'another-class'} \${'another-class'} \${'another-class'} \${'another-class'} \${'another-class'} \${'another-class'} \${'another-class'} \${'another-class'} \${'another-class'} \${'another-class'} \${'another-class'} \${'another-class'} \${'another-class'} \${'another-class'} \${'another-class'} \${'another-class'} \${'another-class'} \${'another-class'} \${'another-class'} \${'another-class'} \${'another-class'} \${'another-class'};
+          justify-content: center;
+          align-items: \${'center'}
+        \`
+      `
+      const { code } = babel.transform(basic, {
+        plugins: [[plugin]]
+      })
+      expect(code).toMatchSnapshot()
+    })
+
     test('throws correct error when composes is not the first rule', () => {
       const basic = `
         const cls1 = css\`
diff --git a/test/babel/styled.test.js b/test/babel/styled.test.js
index a1523172..01c91734 100644
--- a/test/babel/styled.test.js
+++ b/test/babel/styled.test.js
@@ -84,6 +84,33 @@ describe('babel styled component', () => {
       expect(code).toMatchSnapshot()
     })
 
+    test('lots of attrs with interpolated values', () => {
+      const basic = `styled('input')\`
+       margin: attr(\${marginProp});
+       margin: attr(\${marginProp});
+       margin: attr(\${marginProp});
+       margin: attr(\${marginProp});
+       margin: attr(\${marginProp});
+       margin: attr(\${marginProp});
+       margin: attr(\${marginProp});
+       margin: attr(\${marginProp});
+       margin: attr(\${marginProp});
+       margin: attr(\${marginProp});
+       margin: attr(\${marginProp});
+       margin: attr(\${marginProp});
+       margin: attr(\${marginProp});
+       margin: attr(\${marginProp});
+       margin: attr(\${marginProp});
+       margin: attr(\${marginProp});
+       margin: attr(\${marginProp});
+       display: attr(\${displayProp});
+      \``
+      const { code } = babel.transform(basic, {
+        plugins: [plugin]
+      })
+      expect(code).toMatchSnapshot()
+    })
+
     test('attr with value type', () => {
       const basic = `styled('input')\`
         margin: attr(margin px);