-
-
Notifications
You must be signed in to change notification settings - Fork 298
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
Feature/typescript support #267
Conversation
So this is my first guess at adding type definitions so it's probably wrong. Keeping it small and going to commit a component at a time.
Added what I think s the last for the typescript definitions. I just really need to test with another project to confirm.
src/js/Badges/index.d.ts
Outdated
badgeStyle?: React.CSSProperties; | ||
badgeClassName?: string; | ||
badgeId: string | number; | ||
children: React.ReactNode; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know its suboptimal for fields which require children, but this should be optional: children?: React.ReactNode
.
see: microsoft/TypeScript#6471
otherwise you have to do something like:
<TableHeader children={null as any}>
<TableRow children={null as any}>
<TableColumn numeric>x</TableColumn>
<TableColumn numeric>y</TableColumn>
</TableRow>
</TableHeader>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops... I thought I fixed all of them after you mentioned that about the TableColumn
. Thanks! The link and example definitely clears up why it is not ideal.
src/js/Helpers/index.d.ts
Outdated
defaultStyle?: React.CSSProperties; | ||
collapsed: boolean; | ||
springConfig: Object; | ||
children: React.ReactElement<any>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/js/Tabs/index.d.ts
Outdated
swipeableViewsClassName?: string; | ||
slideStyle?: React.CSSProperties; | ||
slideClassName?: string; | ||
children: React.ReactElement<Tabs>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/js/Tabs/index.d.ts
Outdated
interface TabsProps extends Props { | ||
tabId: number | string; | ||
component?: Function | string; | ||
children: React.ReactElement<Tab> | Array<React.ReactElement<Tab>>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/js/Helpers/index.d.ts
Outdated
interface IconSeparatorProps extends Props { | ||
labelStyle?: React.CSSProperties; | ||
labelClassName?: string; | ||
children: React.ReactNode; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/js/Tabs/index.d.ts
Outdated
children: React.ReactElement<Tabs>, | ||
component?: Function | string; | ||
panelComponent?: Function | string; | ||
headerComponent?: Function | string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can use Function
for now, but at some point we should fill out the specific function type signature. I would say to leave as is and improve in a future PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
but as i said, untested, it looks good though |
Yeah, thanks for your time! I'll hopefully release the |
#175
This will be the first pass-through of implementing typescript into react-md. In the future versions, this will hopefully be automated.