Repository: tkh44/emotion (js)

Issue:
Don't generate empty classes
<!--
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.2.0
- `react` version: 15.x.x

Relevant code.

```javascript
const bold = css`font-weight: 700;`
const italic = css`font-style: italic;`
const secondColor = css`color: #7C7C7C;`

const f1 = css`font-size: .75em;`
const f2 = css`font-size: .875em;`
const f3 = css`font-size: 1em;`
const f4 = css`font-size: 2em;`

export const Word = styled("h1")`
  composes: ${bold} ${f4};
`
```

What you did:

- Use the above component

What happened:

- emotion generates empty class

![image](https://user-images.githubusercontent.com/3049054/27998762-403dc86e-653e-11e7-8573-1812d40e718f.png)

![image](https://user-images.githubusercontent.com/3049054/27998764-4ba2a800-653e-11e7-8571-1a726fc51964.png)

<!-- Please provide the full error message/screenshots/anything -->

Reproduction:


Problem description:

As I understand, the redundant class were there for nesting selector. But I use that pattern a lot across the code base, I would hope the redundant class is removed when not neccessary


Suggested solution:


Pull request description:
Fix generating empty classes
Closes #116

<!--
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**:

Remove empty classes.

<!-- Why are these changes necessary? -->
**Why**:

#116 

<!-- How were these changes implemented? -->
**How**:

Rules that don't have any declarations are removed. This includes rules that have `composes` since they are removed earlier. If there are no rules in `css` or `styled`, the `createEmotionStyledRules` and `createEmotionRules` functions are removed.

<!-- 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 -->

Patch:
diff --git a/src/babel.js b/src/babel.js
index 5f736b27..1c1b7ec0 100644
--- a/src/babel.js
+++ b/src/babel.js
@@ -211,7 +211,7 @@ export default function (babel) {
           ]
           if (!hasOtherMatch && !state.inline && !hasCssFunction) {
             state.insertStaticRules(rules)
-          } else {
+          } else if (rules.length !== 0) {
             const inlineContentExpr = t.functionExpression(
               t.identifier('createEmotionStyledRules'),
               vars.map((x, i) => t.identifier(`x${i}`)),
@@ -279,7 +279,7 @@ export default function (babel) {
                   joinExpressionsWithSpaces(inputClasses, t)
                 )
               }
-            } else {
+            } else if (rules.length !== 0) {
               const expressions = path.node.quasi.expressions.map((x, i) =>
                 t.identifier(`x${i}`)
               )
diff --git a/src/parser.js b/src/parser.js
index 9d4ad71f..5767dd8e 100644
--- a/src/parser.js
+++ b/src/parser.js
@@ -59,7 +59,9 @@ export function parseCSS (
           throw new Error('composes must be the first rule')
         }
         const composeMatches = decl.value.match(/xxx(\d+)xxx/gm)
-        const numOfComposes: number = !composeMatches ? 0 : composeMatches.length
+        const numOfComposes: number = !composeMatches
+          ? 0
+          : composeMatches.length
         composes += numOfComposes
         vars += numOfComposes
         decl.remove()
@@ -74,20 +76,26 @@ export function parseCSS (
     }
   })
   if (!options.inlineMode && vars === options.matches && !hasCssFunction) {
-    root.walkDecls((decl) => {
+    root.walkDecls(decl => {
       decl.value = decl.value.replace(/xxx(\d+)xxx/gm, (match, p1) => {
         return `var(--${options.name}-${options.hash}-${p1})`
       })
     })
   }
 
+  root.walkRules(rule => {
+    if (rule.nodes.length === 0) {
+      rule.remove()
+    }
+  })
+
   autoprefix(root)
   return {
     rules: stringifyCSSRoot(root),
     hasOtherMatch: vars !== options.matches,
     hasVar:
       (!!vars && vars !== composes) ||
-        !!(options.inlineMode && options.matches),
+      !!(options.inlineMode && options.matches),
     composes,
     hasCssFunction
   }


Test patch:
diff --git a/test/__snapshots__/css-prop.test.js.snap b/test/__snapshots__/css-prop.test.js.snap
index 0383b948..f922c030 100644
--- a/test/__snapshots__/css-prop.test.js.snap
+++ b/test/__snapshots__/css-prop.test.js.snap
@@ -43,8 +43,6 @@ exports[`css prop react kitchen sink 1`] = `
   align-items: center;
 }
 
-
-
 .css-tree-17jiuxc-pi0syi {
   color: red;
 }
@@ -72,7 +70,7 @@ exports[`css prop react kitchen sink 1`] = `
 }
 
 <div
-  className="css__legacy-stuff css-tree-1ii4sc4-um9pk4 css-tree-1ii4sc4 css-bold-15qcrjv-18w8kdp css-bold-15qcrjv css-flexCenter-mdk64u-llada3 css-flexCenter-mdk64u"
+  className="css__legacy-stuff css-tree-1ii4sc4 css-bold-15qcrjv-18w8kdp css-bold-15qcrjv css-flexCenter-mdk64u-llada3 css-flexCenter-mdk64u"
 >
   <h1
     className="css-tree-17jiuxc-pi0syi css-tree-17jiuxc css-small-1wh34bm-yjtkkb css-small-1wh34bm"
diff --git a/test/__snapshots__/react.test.js.snap b/test/__snapshots__/react.test.js.snap
index ff8edea2..11649055 100644
--- a/test/__snapshots__/react.test.js.snap
+++ b/test/__snapshots__/react.test.js.snap
@@ -109,11 +109,9 @@ exports[`styled composes based on props 1`] = `
   color: blue;
 }
 
-
-
 <h1
   a={true}
-  className="css-H1-16dw226-196nx3w css-H1-16dw226 css-cssA-cy897j-1p4ppuj css-cssA-cy897j"
+  className="css-H1-16dw226 css-cssA-cy897j-1p4ppuj css-cssA-cy897j"
 >
   hello world
 </h1>
@@ -124,10 +122,8 @@ exports[`styled composes based on props 2`] = `
   color: green;
 }
 
-
-
 <h1
-  className="css-H1-16dw226-196nx3w css-H1-16dw226 css-cssB-7o6h5j-dcpp5a css-cssB-7o6h5j"
+  className="css-H1-16dw226 css-cssB-7o6h5j-dcpp5a css-cssB-7o6h5j"
 >
   hello world
 </h1>
diff --git a/test/__snapshots__/vue.test.js.snap b/test/__snapshots__/vue.test.js.snap
index ae2d8ad0..d1999420 100644
--- a/test/__snapshots__/vue.test.js.snap
+++ b/test/__snapshots__/vue.test.js.snap
@@ -8,7 +8,7 @@ exports[`vue styled based on props 1`] = `
 
 exports[`vue styled based on props composes 1`] = `
 "
-      <div id=\\"app\\"><div thing=\\"true\\" class=\\"css-basedOnPropsComposes-17rcs0o-12iwoui css-basedOnPropsComposes-17rcs0o some-special-class\\"></div></div>
+      <div id=\\"app\\"><div thing=\\"true\\" class=\\"css-basedOnPropsComposes-17rcs0o some-special-class\\"></div></div>
     "
 `;
 
@@ -34,7 +34,7 @@ exports[`vue styled creates the correct styles 1`] = `
   -ms-flex-align: center;
   -webkit-box-align: center;
   align-items: center;
-  position: inherit; }.css-BasedOnProps-csm9t8-1qo3zsu { font-weight: bold; }.css-basedOnPropsComposes-17rcs0o-12iwoui { }.css-NonHtmlComponent-15287sz-1wq2ikh { background-color: purple;
+  position: inherit; }.css-BasedOnProps-csm9t8-1qo3zsu { font-weight: bold; }.css-NonHtmlComponent-15287sz-1wq2ikh { background-color: purple;
   color: green; }.css-ChildSelector-13pvedz-1e7xvxz { color: yellow; }.css-ChildSelector-13pvedz-1e7xvxz .css-StyledComponent-1brvt4i { display: none; }"
 `;
 
diff --git a/test/babel/__snapshots__/css-prop.test.js.snap b/test/babel/__snapshots__/css-prop.test.js.snap
index 38b17225..496ae76a 100644
--- a/test/babel/__snapshots__/css-prop.test.js.snap
+++ b/test/babel/__snapshots__/css-prop.test.js.snap
@@ -31,11 +31,7 @@ exports[`babel css prop className as expression string 1`] = `
 })} this={\`hello\`}></div>;"
 `;
 
-exports[`babel css prop css empty 1`] = `
-"<div className={css([\\"css-0\\"], [], function createEmotionRules() {
-  return [\`.css-0 {  }\`];
-})}></div>;"
-`;
+exports[`babel css prop css empty 1`] = `"<div className={css([\\"css-0\\"], [])}></div>;"`;
 
 exports[`babel css prop dynamic inline 1`] = `
 "<div className={\\"a\\" + \\" \\" + css([\\"css-jsqoaa\\"], [color], function createEmotionRules(x0) {
diff --git a/test/babel/__snapshots__/css.test.js.snap b/test/babel/__snapshots__/css.test.js.snap
index fc459457..30fbb58d 100644
--- a/test/babel/__snapshots__/css.test.js.snap
+++ b/test/babel/__snapshots__/css.test.js.snap
@@ -146,6 +146,26 @@ const cls2 = css(['css-cls2-hal2lb', 'one-class', 'another-class', 'another-clas
 });"
 `;
 
+exports[`babel css inline only composes 1`] = `
+"
+const cls1 = css(['css-cls1-1q8jsgx'], [], function createEmotionRules() {
+  return [\`.css-cls1-1q8jsgx { display: -webkit-box; display: -moz-box; display: -ms-flexbox; display: -webkit-flex; display: flex; }\`];
+});
+const cls2 = css(['css-cls2-64ycl', 'one-class', 'another-class', cls1], []);"
+`;
+
+exports[`babel css inline only styles on nested selector 1`] = `
+"
+const cls1 = css([\\"css-cls1-1q8jsgx\\"], [], function createEmotionRules() {
+  return [\`.css-cls1-1q8jsgx { display: -webkit-box; display: -moz-box; display: -ms-flexbox; display: -webkit-flex; display: flex; }\`];
+});
+const cls2 = css([\\"css-cls2-1nbi349\\"], [], function createEmotionRules() {
+  return [\`.css-cls2-1nbi349:hover {
+    background: pink;
+}\`];
+});"
+`;
+
 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 08c56846..fd732149 100644
--- a/test/babel/__snapshots__/styled.test.js.snap
+++ b/test/babel/__snapshots__/styled.test.js.snap
@@ -238,9 +238,7 @@ const H1 = styled('h1', [{
 
 exports[`babel styled component inline styled component as selector 1`] = `
 "
-const SomeComponent = styled(\\"div\\", [\\"css-SomeComponent-1rk3iho\\"], [], function createEmotionStyledRules() {
-  return [\`.css-SomeComponent-1rk3iho {  }\`];
-});
+const SomeComponent = styled(\\"div\\", [\\"css-SomeComponent-1rk3iho\\"], []);
 styled(\\"h1\\", [\\"css-byjn67\\"], [SomeComponent], function createEmotionStyledRules(x0) {
   return [\`.css-byjn67 { color: blue; }\`, \`.css-byjn67 \${x0} { color: green; }\`];
 });"
diff --git a/test/babel/css.test.js b/test/babel/css.test.js
index 29ed731b..de467ce9 100644
--- a/test/babel/css.test.js
+++ b/test/babel/css.test.js
@@ -71,6 +71,38 @@ describe('babel css', () => {
       expect(code).toMatchSnapshot()
     })
 
+    test('only composes', () => {
+      const basic = `
+        const cls1 = css\`
+          display: flex;
+        \`
+        const cls2 = css\`
+          composes: \${'one-class'} \${'another-class'}\${cls1}
+        \`
+      `
+      const { code } = babel.transform(basic, {
+        plugins: [[plugin]]
+      })
+      expect(code).toMatchSnapshot()
+    })
+
+    test('only styles on nested selector', () => {
+      const basic = `
+        const cls1 = css\`
+          display: flex;
+        \`
+        const cls2 = css\`
+          &:hover {
+            background: pink;
+          }
+        \`
+      `
+      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\`