Repository: tkh44/emotion (js)

Issue:
attr css function is not being turned into a dynamic value
<!--
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).

Please fill out this template with all the relevant information so we can
understand what's going on and fix the issue.

I'll probably ask you to submit the fix (after giving some direction). If you've
never done that before, that's great! Check this free short video tutorial to
learn how: http://kcd.im/pull-request
-->

- `emotion` version: 5.1.0

Relevant code.

```javascript
const PlaygroundWrapper = styled('div')`
  color: attr(color, #343a40);
`
```

What you did:

Ran latest code when using attr

What happened:
X.emotion.css will contain attr values


Pull request description:
Fix attr in extract mode
<!--
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**:

<!-- Why are these changes necessary? -->
**Why**:

<!-- 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 -->
Closes #84 

For now this just doesn't extract css that has `attr`, this can be improved later.


Patch:
diff --git a/src/babel.js b/src/babel.js
index c9bfc4c0..39015ebc 100644
--- a/src/babel.js
+++ b/src/babel.js
@@ -156,7 +156,7 @@ export default function (babel) {
           parent && t.isIdentifier(parent.node.id) ? parent.node.id.name : ''
 
         function buildCallExpression (identifier, tag, path) {
-          let { hash, rules, name, hasOtherMatch, composes } = inline(
+          let { hash, rules, name, hasOtherMatch, composes, hasCssFunction } = inline(
             path.node.quasi,
             identifierName,
             'css',
@@ -184,7 +184,7 @@ export default function (babel) {
             t.arrayExpression(inputClasses),
             t.arrayExpression(vars)
           ]
-          if (!hasOtherMatch && !state.inline) {
+          if (!hasOtherMatch && !state.inline && !hasCssFunction) {
             state.insertStaticRules(rules)
           } else {
             const inlineContentExpr = t.functionExpression(
diff --git a/src/inline.js b/src/inline.js
index 903ac1ba..9a42f0ba 100644
--- a/src/inline.js
+++ b/src/inline.js
@@ -56,7 +56,8 @@ export function inline (
   rules: string[],
   hasVar: boolean,
   hasOtherMatch: boolean,
-  composes: number
+  composes: number,
+  hasCssFunction: boolean
 } {
   let strs = quasi.quasis.map(x => x.value.cooked)
   let hash = hashArray([...strs]) // todo - add current filename?
@@ -70,7 +71,8 @@ export function inline (
     rules,
     hasVar,
     hasOtherMatch,
-    composes
+    composes,
+    hasCssFunction
   } = parseCSS(`.${name}-${hash} { ${src} }`, {
     inlineMode: inlineMode,
     matches,
@@ -78,7 +80,7 @@ export function inline (
     hash,
     canCompose: true
   })
-  return { hash, name, rules, hasVar, hasOtherMatch, composes }
+  return { hash, name, rules, hasVar, hasOtherMatch, composes, hasCssFunction }
 }
 
 export function keyframes (
diff --git a/src/parser.js b/src/parser.js
index 79854cfc..146f2d94 100644
--- a/src/parser.js
+++ b/src/parser.js
@@ -21,15 +21,19 @@ export function parseCSS (
     hash: 'hash',
     canCompose: false
   }
-): { rules: string[], hasOtherMatch: boolean, hasVar: boolean, composes: number } {
+): { rules: string[], hasOtherMatch: boolean, hasVar: boolean, composes: number, hasCssFunction: boolean } {
   // todo - handle errors
   const root = parse(css)
   if (options.nested !== false) postcssNested(root)
 
   let vars = 0
   let composes = 0
+  let hasCssFunction = false
   root.walkDecls(decl => {
     if (decl.prop === 'name') decl.remove()
+    if (decl.value.match(/attr/)) {
+      hasCssFunction = true
+    }
     if (options.canCompose) {
       if (decl.prop === 'composes') {
         if (decl.parent.selector !== `.${options.name}-${options.hash}`) {
@@ -54,7 +58,7 @@ export function parseCSS (
       }
     }
   })
-  if (!options.inlineMode && vars === options.matches) {
+  if (!options.inlineMode && vars === options.matches && !hasCssFunction) {
     root.walkDecls((decl) => {
       decl.value = decl.value.replace(/xxx(\S)xxx/gm, (match, p1) => {
         return `var(--${options.name}-${options.hash}-${p1})`
@@ -67,7 +71,8 @@ export function parseCSS (
     rules: stringifyCSSRoot(root),
     hasOtherMatch: vars !== options.matches,
     hasVar: (!!vars && vars !== composes) || !!(options.inlineMode && options.matches),
-    composes
+    composes,
+    hasCssFunction
   }
 }
 


Test patch:
diff --git a/test/__snapshots__/parser.test.js.snap b/test/__snapshots__/parser.test.js.snap
index 5bf52a56..c1a88a6a 100644
--- a/test/__snapshots__/parser.test.js.snap
+++ b/test/__snapshots__/parser.test.js.snap
@@ -3,6 +3,7 @@
 exports[`parser basic 1`] = `
 Object {
   "composes": 0,
+  "hasCssFunction": false,
   "hasOtherMatch": false,
   "hasVar": false,
   "rules": Array [
@@ -25,6 +26,7 @@ Object {
 exports[`parser fancy 1`] = `
 Object {
   "composes": 0,
+  "hasCssFunction": false,
   "hasOtherMatch": false,
   "hasVar": false,
   "rules": Array [
@@ -67,6 +69,7 @@ Object {
 exports[`parser static 1`] = `
 Object {
   "composes": 0,
+  "hasCssFunction": false,
   "hasOtherMatch": false,
   "hasVar": false,
   "rules": Array [
diff --git a/test/extract/__snapshots__/extract.test.js.snap b/test/extract/__snapshots__/extract.test.js.snap
index 16fc1ced..7b33f602 100644
--- a/test/extract/__snapshots__/extract.test.js.snap
+++ b/test/extract/__snapshots__/extract.test.js.snap
@@ -1,13 +1,23 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`styled another attr 1`] = `
+.css-PlaygroundWrapper-1shjoz3-tra3vx {
+  color: #343a40;
+}
+
+<div
+  className="css-PlaygroundWrapper-1shjoz3-tra3vx css-PlaygroundWrapper-1shjoz3"
+/>
+`;
+
 exports[`styled attr 1`] = `
-.vars-1sp61ak {
-  --css-H1-15vt02v-0: 48;
-  --css-H1-15vt02v-1: 4rem;
+.css-H1-nt9mdf-vddukf {
+  font-size: 48;
+  margin: 4rem;
 }
 
 <h1
-  className="css-H1-15vt02v vars-1sp61ak"
+  className="css-H1-nt9mdf-vddukf css-H1-nt9mdf"
   fontSize={48}
 />
 `;
@@ -117,8 +127,6 @@ exports[`styled writes the correct css 1`] = `
 .css-H1-1t8i2zo { font-size: var(--css-H1-1t8i2zo-0); }
 .css-FancyH1-1o9p8a2 {
       font-size: var(--css-FancyH1-1o9p8a2-0); }
-.css-H1-nt9mdf { font-size: attr(fontSize);
-      margin: attr(margin rem, 4); }
 .css-H1-rs9k70 { font-size: var(--css-H1-rs9k70-0); }
 .css-H1-rs9k70 { font-size: var(--css-H1-rs9k70-0); }
 .css-H2-vxb7tq { font-size: var(--css-H2-vxb7tq-0) }
diff --git a/test/extract/extract.test.emotion.css b/test/extract/extract.test.emotion.css
index d4361515..a83d4411 100644
--- a/test/extract/extract.test.emotion.css
+++ b/test/extract/extract.test.emotion.css
@@ -2,8 +2,6 @@
 .css-H1-1t8i2zo { font-size: var(--css-H1-1t8i2zo-0); }
 .css-FancyH1-1o9p8a2 {
       font-size: var(--css-FancyH1-1o9p8a2-0); }
-.css-H1-nt9mdf { font-size: attr(fontSize);
-      margin: attr(margin rem, 4); }
 .css-H1-rs9k70 { font-size: var(--css-H1-rs9k70-0); }
 .css-H1-rs9k70 { font-size: var(--css-H1-rs9k70-0); }
 .css-H2-vxb7tq { font-size: var(--css-H2-vxb7tq-0) }
diff --git a/test/extract/extract.test.js b/test/extract/extract.test.js
index 3069f5ea..bddc1c86 100644
--- a/test/extract/extract.test.js
+++ b/test/extract/extract.test.js
@@ -59,6 +59,13 @@ describe('styled', () => {
     expect(tree).toMatchSnapshotWithEmotion()
   })
 
+  test('another attr', () => {
+    const PlaygroundWrapper = styled('div')`
+      color: attr(color, #343a40);
+    `
+    expect(renderer.create(<PlaygroundWrapper />).toJSON()).toMatchSnapshotWithEmotion()
+  })
+
   test('call expression', () => {
     const fontSize = '20px'
     const H1 = styled('h1')`