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

Add types to react-emotion #398

Merged
merged 19 commits into from
Oct 16, 2017
Merged

Add types to react-emotion #398

merged 19 commits into from
Oct 16, 2017

Conversation

renatorib
Copy link
Contributor

@renatorib renatorib commented Oct 13, 2017

What:
TypeScript declarations for react-emotion package

Why:
Support people who use TypeScript and need typings.

How:

Checklist:

  • Documentation
  • Tests
  • Code complete

Supports

Props Inference by Tag Name:

image


Supports template string and object as styles:

image


Passing Custom Props:

image
Note1: needs a second type parameter passing tag name because it stop to infer when you pass props at first parameter
Note2: glamorous do not support custom props with shorthands, like Link2 😛. It was a little tricky, but works...


Create styled with another component:

image


Pass custom props to style another component:

image
Note3: as well as Note1 it's not infer 'MyComponent's Props' when you pass props as first type parameter, so you need to pass 'MyComponent' props together

type Props = MyNewProps & MyComponentProps

Theming props:

By default props.theme is any, but you can create another styled instance passing theme props.
Basically the same approach from styled-components

image


withComponent:

image

Also works passing another component instead of tag name

@@ -0,0 +1,14 @@
{
"compilerOptions": {
"target": "es5",

Choose a reason for hiding this comment

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

For glamorous we've added declaration: true to help make sure we're exporting everything required for use in libraries microsoft/TypeScript#5938.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done! Added.
No warnings
😄

@luke-john
Copy link

Awesome work doing these types. They're looking great 👍 .


export type Interpolation<Props = {}> =
| InterpolationFn<Props>
| EmotionInterpolation
Copy link
Member

Choose a reason for hiding this comment

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

I don't know much about typescript but I don't think this handles the fact that there can be nested function interpolations and they will receive props.

const SomeComponent = styled.div`
  display: ${(p) => props => props.display};
  color: ${[props => 'hotpink']};
`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It works on emotion?
props => props_ => ...

It doesn't make any sense to me. What's this use case for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know exactly all edge cases of emotion API. I'll try to study it to improve the coverage support.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mitchellhamilton I've added support to these two cases you mentioned.

export type InterpolationFn<Props = {}> =
  (props: Props) =>
    | EmotionInterpolation
    | InterpolationFn<Props>

export type InterpolationTypes<Props = {}> =
  | InterpolationFn<Props>
  | EmotionInterpolation

export type Interpolation<Props = {}> =
  | InterpolationTypes<Props>
  | InterpolationTypes<Props>[]

If you know more cases, just tell me, please.

@screendriver
Copy link

But you still need Babel in the build pipeline as I already asked here #163?

@renatorib
Copy link
Contributor Author

@screendriver This is only typings, do not deal with build, or any emotion code. I don't even know the internals of emotion yet.

@screendriver
Copy link

Ah ok. But TypeScript definitions for emotion are a little but useless when you can't use emotion without Babel 😉 (emotion only works as Babel plugin so Babel is mandatory)

Apart from that: thank you for the typings 😎👍

@renatorib
Copy link
Contributor Author

renatorib commented Oct 13, 2017

If you don't want to use babel (or can't) just don't use emotion. Pick another one: styled-components, glamor/ous, fela, etc.

Anyway, this is another issue. Don't have any relation with typescript or this pull request.

@renatorib
Copy link
Contributor Author

Someone can help me with documentation? I think it's the only thing missing to this pr.

My suggestion is to write a section "TypeScript" in emotion website, but you are creating a new website, right? @tkh44

@tkh44
Copy link
Member

tkh44 commented Oct 13, 2017

@renatorib any new markdown file in the /docs folder is added to the website. Just add a new file there called typescript.md and do your thing 👍

@renatorib
Copy link
Contributor Author

That's great! I'll try to write something. :)

@adamseckel
Copy link

This is awesome, I just started using typescript as a replacement for PropTypes, and this will go nicely with that 👌 Thanks @renatorib

@screendriver
Copy link

screendriver commented Oct 13, 2017

If you don't want to use babel (or can't) just don't use emotion. Pick another one: styled-components, glamor/ous, fela, etc.

I am not complaining about that. It is much more a question how to use emotion with TypeScript 😉 Because you found obviously a way that I am not aware of at the moment and I really want to use emotion as well. Great work 👍

@renatorib
Copy link
Contributor Author

renatorib commented Oct 13, 2017

For now I'm 'blocked' from using emotion in my company codebase because we use TypeScript and emotion don't have typings.
So I sent a PR so I (and all of us) can use it with TypeScript.

If you really needs use emotion and can't use Babel, you can:

  1. Sent a PR that makes emotion shorthands work without babel, so we can work together in that.
  2. Use it without babel, abdicating the shorthands feature (using styled('div')).

You also can use Babel and TypeScript together, I guess.

Btw,
I still think we should not be talking about it here. This has no connection to the PR thread.

And thanks for the compliment.

@tkh44
Copy link
Member

tkh44 commented Oct 14, 2017

@renatorib Is this ready except for documentation?

@renatorib
Copy link
Contributor Author

renatorib commented Oct 14, 2017

@tkh44 yep!

I did not have free time to write it yet, but it's the only thing missing.

@renatorib
Copy link
Contributor Author

renatorib commented Oct 14, 2017

Btw, If you want, you can merge it and I send another PR with documentation.

@renatorib
Copy link
Contributor Author

renatorib commented Oct 15, 2017

I've added a typescript documentation

Unfortunately my english is not as good as I wish it were. 😢
Someone please give a review so nothing's going wrong 😄

Thanks!

@tkh44
Copy link
Member

tkh44 commented Oct 15, 2017

Great work @renatorib! I did a quick edit but I'd like someone who knows more about typescript do a review.

@renatorib
Copy link
Contributor Author

Looks great @tkh44, thanks!

@tkh44 tkh44 merged commit 3f58674 into emotion-js:master Oct 16, 2017
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.

6 participants