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

Fix typings for using components as selectors. #690

Closed
wants to merge 4 commits into from
Closed

Fix typings for using components as selectors. #690

wants to merge 4 commits into from

Conversation

mgroenhoff
Copy link
Contributor

@mgroenhoff mgroenhoff commented May 29, 2018

What: Fix the typings to allow react components to be used as component selectors (fixes #685).

Why: Because when using Typescript 2.8.3 i get the following error:

Argument of type 'TemplateStringsArray' is not assignable to parameter of type 'CSSProperties | { [key: string]: ObjectStyleAttributes; } | ((props: ThemedProps<RowProps, Theme>...'.
Type 'TemplateStringsArray' is not assignable to type '(props: ThemedProps<RowProps, Theme>) => ObjectStyleAttributes'.
Type 'TemplateStringsArray' provides no match for the signature '(props: ThemedProps<RowProps, Theme>): ObjectStyleAttributes'.

How: I added a type to the list of allowed interpolation types.

Checklist:

  • Documentation N/A
  • Tests N/A
  • Code complete N/A

Update: I changed it to only allow styled components and not regular (stateless) components.

@mgroenhoff mgroenhoff changed the title Fix typings for using components as selectors (fixes #685). Fix typings for using components as selectors. May 29, 2018
@tkh44 tkh44 added the merge me! label Jun 4, 2018
@Ailrun
Copy link
Member

Ailrun commented Jun 4, 2018

@tkh44
It looks like this and #678 have some level of redundancy. Could you check it too?

@tkh44
Copy link
Member

tkh44 commented Jun 4, 2018

@Ailrun @mgroenhoff can you 2 coordinate on this? It looks like #678 can be applied on top of this but I'm not a TypeScript expert.

@Ailrun
Copy link
Member

Ailrun commented Jun 4, 2018

@mgroenhoff #678 is for remove redundancy between create-emotion-styled and react-emotion. Since we already have typings for create-emotion-styled, we don't need to add complex typings in react-emotion.

@mgroenhoff
Copy link
Contributor Author

Okay, what do I need to change?

@Ailrun
Copy link
Member

Ailrun commented Jun 4, 2018

@mgroenhoff I mean... this PR is about changing the typings for react-emotion, and #678 is about removing typings (and replacing by typings from create-emotion-styled), so these intents make conflicts... don't they?

@mgroenhoff
Copy link
Contributor Author

I think the idea behind #676 (which I missed) is the same as this PR. #678 indeed invalidates this one because it makes use of those changes. I'll have to test it out tomorrow morning but I think this will do the job too.

@mgroenhoff
Copy link
Contributor Author

I have tested #678 and that solved my issue. Please merge that one 👍

@mgroenhoff mgroenhoff closed this Jun 5, 2018
@mgroenhoff mgroenhoff deleted the #685 branch June 5, 2018 08:34
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.

Typescript and component selectors
3 participants