Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test: withComponent(...).attrs is not a function #851

Merged

Conversation

btmills
Copy link
Contributor

@btmills btmills commented May 29, 2017

🚨 This pull request contains a failing test and should not be merged. Edit: Test is no longer failing!

Should it be possible to add attrs after overriding withComponent? I found a test for withComponent followed by extend, but none for attrs, so I added one:

const Parent = styled.button`
  color: red;
`
const Child = Parent.withComponent('a').attrs({
  href: '/test'
})``

Currently this throws TypeError: Parent.withComponent(...).attrs is not a function.

I haven't looked into the implementation to see what might be causing this, but if it's an actual bug, I'd be willing to spend some time on a fix given a couple hints about where to start looking!

@mxstbr-bot
Copy link

mxstbr-bot commented May 29, 2017

Warnings
⚠️ There are library changes, but not tests. That's OK as long as you're refactoring existing code

Generated by 🚫 dangerJS

@mxstbr
Copy link
Member

mxstbr commented May 29, 2017

That definitely sounds like something that should work! Maybe this is related to #780?

@kitten
Copy link
Member

kitten commented May 29, 2017

withComponent is a method on styled components and attrs is one on the factory, so this indeed won't work as is :( I'll take a look and see what's required to make this happen

@geelen
Copy link
Member

geelen commented May 30, 2017

Yeah withComponent doesn't chain like the others, it's not designed to return the constructor, but the fully formed Component. You'll need extend in there somewhere:

const Child = Parent.withComponent('a').extend.attrs({
  ...
})`
  ...
`

or

const Child = Parent.extend.attrs({
  ...
})`
  ...
`.withComponent('a')

Should it be possible to add `attrs` after overriding `withComponent`? I
found a test for `withComponent` followed by `extend`, but none for
`attrs`, so I added one:

```js
const Parent = styled.button`
  color: red;
`
const Child = Parent.withComponent('a').attrs({
  href: '/test'
})``
```

Currently this throws `TypeError: Parent.withComponent(...).attrs is not
a function`.
@btmills btmills force-pushed the with-component-attrs-failure branch from c91fc4d to 3d455b6 Compare June 2, 2017 06:34
@btmills btmills changed the title Failing test: withComponent(...).attrs is not a function Test: withComponent(...).attrs is not a function Jun 2, 2017
@btmills
Copy link
Contributor Author

btmills commented Jun 2, 2017

Thanks for the tip, @geelen! It sounds like this is the intended behavior, so I've inserted extend in this PR in case you want to have the test, but otherwise feel free to close this as successfully resolved! ✨

Copy link
Member

@mxstbr mxstbr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't have enough tests, thanks @btmills!

@mxstbr mxstbr merged commit 291c93a into styled-components:master Jun 2, 2017
@btmills btmills deleted the with-component-attrs-failure branch June 5, 2017 06:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants