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

Make StyledComponent type polymorphic #1587

Closed
FezVrasta opened this issue Oct 29, 2019 · 3 comments
Closed

Make StyledComponent type polymorphic #1587

FezVrasta opened this issue Oct 29, 2019 · 3 comments

Comments

@FezVrasta
Copy link
Member

FezVrasta commented Oct 29, 2019

I started to use the new Flow types into my project, and I figured out it would make sense to make StyledComponent a polymorphic type, so that we can do this:

const Foo = styled<Props>(props => <div {...props} />)`
  color: ${props => props.color};
`;

const Bar = styled<Props>('div')`
  color: ${props => props.color};
`;

Which would allow to:

  1. type props inside the styled component logic
  2. allow to workaround the lack of tagged template support of Flow

Unfortunately I doubt my Flow knowledge is good enough to implement this without breaking the existing types, which make sense because we also want to allow:

const Foo = styled.div<Props>`
  color: ${props => props.color};
`;

What do you think?

@Andarist
Copy link
Member

Well, I'm not sure if there is anyone other than you willing to jump on this. This definitely makes sense, I just don't have enough expertise with flow to implement this myself. I also don't understand how those types were used so far, so it's hard to assess what might be breaking and what is not.

I'm not against breaking changes in types though - I believe that we can release type changes each minor version~ (it's completely unergonomic to make them bump major versions).

@FezVrasta
Copy link
Member Author

FezVrasta commented Oct 29, 2019

I think it could be enough to change the CreateStyled call signature, while leaving the [key: string]: ... type as is it now.

That way we can still do styled.div<Props>(), we can do styled<Props>(fn)(), but we'll not be allowed to do styled(fn)<Props>() (which I think would be okay).

@Andarist
Copy link
Member

Andarist commented Nov 5, 2019

This got fixed by #1588 (merged to next branch)

@Andarist Andarist closed this as completed Nov 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants