Add typing for create-emotion-styled #668
Conversation
756c02a to
9cdd376
Compare
|
Wow, what a nice suprise! Thank you, It will come useful 👍 // Btw. please please fix StyledOptions in react-emotion typings
export interface Options {
string?: string;
} |
9cdd376 to
759a293
Compare
|
@vadistic My pleasure. Actually, it's broken in many ways, wrong typed |
7236a0e to
68398c7
Compare
68398c7 to
1757288
Compare
| e?: string; | ||
| label?: string; | ||
| target?: string; | ||
| shouldForwardProp?: (name?: string) => boolean; |
There was a problem hiding this comment.
This should be (name: string) => boolean, the name argument will always be there.
| }, (props) => ({ | ||
| height: props.theme ? '200px' : '100px', | ||
| })); | ||
|
|
There was a problem hiding this comment.
Could you add some tests with the string syntax?
There was a problem hiding this comment.
Ah, tests for strings. Thx! I missed them!
|
@mitchellhamilton I added/fixed those! Could you suggest more tests I missed? I don't think those tests are enough... |
| const StyledClassComp0 = createStyled(TestClassComp)({ | ||
| width: '200px', | ||
| }, (props) => ({ | ||
| height: props.theme ? '200px' : '100px', |
There was a problem hiding this comment.
props.theme will never be falsy, if there isn't a ThemeProvider, props.theme will be an empty object.
There was a problem hiding this comment.
Oh, I didn't know that. I should fix typing also.
| display: inline; | ||
| position: fixed; | ||
| flexGrow: 20; | ||
| ${(props: StyledFunProps, context) => ` |
There was a problem hiding this comment.
Returning a string with css properties isn't something we recommend because it breaks syntax highlighting, formatting and minifying. Could you change it to this?
css`
content: ${context.name};
color: ${props.color};
`|
@mitchellhamilton It's now fixed! Thank you for reviewing my PR! |
**What**: 1. All types introduced in #668 live in single file, so it's hard to write preact type using common things. ~I do really hate global JSX namespace idea.~ 1. It used ReactHTML, but it's not an interface for props, but for factories. Therefore it does not provide HTML attributes like `onClick` as props. 1. ReactHTML problem occurs because of lack of tests, so I add some tests too. **Why**: Typing introduced in #668 is wrong. **How**: By using `JSX.IntrinsicElements` **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" --> - [N/A] Documentation - [x] Tests - [x] Code complete
What: Add typing for create-emotion-styled package
Why: To allow TS users to use create-emotion-styled
How: By consulting with glamorous typing and old react-emotion typing
Checklist:
I'm not sure this is complete typing. Could you give me more idea for testing? What usecases did I miss?
After typing for this package is completed and #667 is merged without complex problems, I want to fix
react-emotiontyping and addpreact-emotiontyping.