Repository: tkh44/emotion
Pull request #236 (2017-08-05 11:59:18)

Filter props on composed components
<!--
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**:

Filter props on components that are styled components that have string tags. For example the props on `Link` will now be filtered.
```jsx
const BaseLink = styled.a`background-color: hotpink;`
const Link = styled(BaseLink)`color: green;`
```
<!-- Why are these changes necessary? -->
**Why**:

Because otherwise there will be unknown prop warnings on composed styled components.

<!-- How were these changes implemented? -->
**How**:

Move the declaration of `spec` to be before `Styled` is defined and check wether the first spec's tag is a string instead of just the tag.

<!-- 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 -->

This is also a bit smaller now. 🎉 


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

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

```diff
@@            Coverage Diff             @@
##           master     #236      +/-   ##
==========================================
- Coverage   90.31%   90.26%   -0.06%     
==========================================
  Files          20       20              
  Lines         971      966       -5     
  Branches      263      261       -2     
==========================================
- Hits          877      872       -5     
  Misses         76       76              
  Partials       18       18
```


| [Impacted Files](https://codecov.io/gh/tkh44/emotion/pull/236?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/react/index.js](https://codecov.io/gh/tkh44/emotion/pull/236?src=pr&el=tree#diff-c3JjL3JlYWN0L2luZGV4Lmpz) | `100% <100%> (ø)` | :arrow_up: |

------

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

tkh44: This should fix factories as well! Awesome work.

Patch:
diff --git a/src/react/index.js b/src/react/index.js
index c4103dda..6b170044 100644
--- a/src/react/index.js
+++ b/src/react/index.js
@@ -27,14 +27,25 @@ export default function(tag, cls, objs, vars = [], content) {
   }
 
   const componentTag = tag.displayName || tag.name || 'Component'
+  const spec = {
+    vars,
+    content,
+    objs,
+    tag,
+    cls
+  }
+  const newSpec =
+    tag.__emotion_spec !== undefined ? tag.__emotion_spec.concat(spec) : [spec]
+  const localTag = newSpec[0].tag
+
   const omitFn =
-    typeof tag === 'string'
+    typeof localTag === 'string'
       ? testOmitPropsOnStringTag
       : testOmitPropsOnComponent
   function Styled(props, context) {
     const getValue = v => {
       if (v && typeof v === 'function') {
-        if (v.__emotion_class) {
+        if (v.__emotion_class !== undefined) {
           return `& .${v.__emotion_class}`
         }
         return v(props, context)
@@ -42,36 +53,24 @@ export default function(tag, cls, objs, vars = [], content) {
 
       return v
     }
-    let localTag = tag
 
     let finalObjs = []
 
-    if (tag.__emotion_spec) {
-      push(
-        finalObjs,
-        reduce(
-          tag.__emotion_spec,
-          (accum, spec) => {
-            push(accum, spec.objs)
-            if (spec.content) {
-              push(accum, spec.content.apply(null, map(spec.vars, getValue)))
-            }
-            accum.push(spec.cls)
-            return accum
-          },
-          []
-        )
+    push(
+      finalObjs,
+      reduce(
+        newSpec,
+        (accum, spec) => {
+          push(accum, spec.objs)
+          if (spec.content) {
+            push(accum, spec.content.apply(null, map(spec.vars, getValue)))
+          }
+          accum.push(spec.cls)
+          return accum
+        },
+        []
       )
-      localTag = tag.__emotion_spec[0].tag
-    }
-
-    push(finalObjs, objs)
-
-    finalObjs.push(cls)
-
-    if (content) {
-      push(finalObjs, content.apply(null, map(vars, getValue)))
-    }
+    )
 
     if (props.className) {
       push(finalObjs, props.className.split(' '))
@@ -92,16 +91,7 @@ export default function(tag, cls, objs, vars = [], content) {
   }
 
   Styled.displayName = `styled(${componentTag})`
-  const spec = {
-    vars,
-    content,
-    objs,
-    tag,
-    cls
-  }
-  Styled.__emotion_spec = tag.__emotion_spec
-    ? tag.__emotion_spec.concat(spec)
-    : [spec]
+  Styled.__emotion_spec = newSpec
   Styled.__emotion_class = cls
   return Styled
 }


Test patch:
diff --git a/test/__snapshots__/react.test.js.snap b/test/__snapshots__/react.test.js.snap
index aeed2502..1af0cb34 100644
--- a/test/__snapshots__/react.test.js.snap
+++ b/test/__snapshots__/react.test.js.snap
@@ -210,7 +210,6 @@ exports[`styled composes 1`] = `
 
 <h1
   className="glamor-0 glamor-1 legacy__class glamor-2"
-  scale={2}
 >
   hello world
 </h1>
@@ -259,7 +258,6 @@ exports[`styled composes with objects 1`] = `
 
 <h1
   className="glamor-0 glamor-1 legacy__class glamor-2"
-  scale={2}
 >
   hello world
 </h1>
@@ -302,7 +300,6 @@ exports[`styled function in expression 1`] = `
 
 <h1
   className="glamor-0 glamor-1 legacy__class glamor-2"
-  scale={2}
 >
   hello world
 </h1>
@@ -533,6 +530,23 @@ exports[`styled prop filtering 1`] = `
 </a>
 `;
 
+exports[`styled prop filtering on composed styled components that are string tags 1`] = `
+.glamor-2 {
+  background-color: hotpink;
+  color: green;
+}
+
+<a
+  aria-label="some label"
+  className="glamor-0 glamor-1 glamor-2"
+  data-wow="value"
+  href="link"
+  is={true}
+>
+  hello world
+</a>
+`;
+
 exports[`styled random expressions 1`] = `
 .glamor-1 {
   font-size: 1rem;
diff --git a/test/macro/__snapshots__/react.test.js.snap b/test/macro/__snapshots__/react.test.js.snap
index d4f35e84..0974c8e7 100644
--- a/test/macro/__snapshots__/react.test.js.snap
+++ b/test/macro/__snapshots__/react.test.js.snap
@@ -69,7 +69,6 @@ exports[`styled composes 1`] = `
 
 <h1
   className="glamor-0 glamor-1 glamor-2"
-  scale={2}
 >
   hello world
 </h1>
@@ -119,7 +118,6 @@ exports[`styled composes with objects 1`] = `
 
 <h1
   className="glamor-0 glamor-1 glamor-2"
-  scale={2}
 >
   hello world
 </h1>
@@ -144,7 +142,6 @@ exports[`styled function in expression 1`] = `
 
 <h1
   className="glamor-0 glamor-1 glamor-2"
-  scale={2}
 >
   hello world
 </h1>
diff --git a/test/react.test.js b/test/react.test.js
index b53a858d..beb0aac8 100644
--- a/test/react.test.js
+++ b/test/react.test.js
@@ -607,6 +607,31 @@ describe('styled', () => {
 
     expect(tree).toMatchSnapshot()
   })
+  test('prop filtering on composed styled components that are string tags', () => {
+    const BaseLink = styled.a`background-color: hotpink;`
+    const Link = styled(BaseLink)`color: green;`
+
+    const tree = renderer
+      .create(
+        <Link
+          a
+          b
+          wow
+          prop
+          filtering
+          is
+          cool
+          aria-label="some label"
+          data-wow="value"
+          href="link"
+        >
+          hello world
+        </Link>
+      )
+      .toJSON()
+
+    expect(tree).toMatchSnapshot()
+  })
   test('throws if undefined is passed as the component', () => {
     expect(
       () => styled(undefined)`display: flex;`