-
Notifications
You must be signed in to change notification settings - Fork 672
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Spike out potential TypeScript conversion #662
Conversation
This "reworks" `@theme-ui/core` to be written in TypeScript and export its own type definitions. Thankfully, `microbundle` has built-in support for TypeScript so we barely need to do any build system changes. I also checked in the generated `index.d.ts` file (normally we would not check this into git) to show what it looks like.
Here's where I am at: the theme-ui/packages/core/index.d.ts Lines 1 to 23 in 92e8090
theme-ui/packages/core/theme.d.ts Lines 1 to 134 in 92e8090
@hasparus does this look okay-ish to you, both the types and the general build process of how this works (automatically)? Any thoughts/feedback? Once we are happy with where the types & the build process are at, then we ship this conversion and start tackling the other packages from there. I will be opening a planning issue with a clear outline what needs to be converted so all the contributors can help with this 馃檹 |
packages/core/src/theme.d.ts
Outdated
@@ -0,0 +1,143 @@ | |||
import * as CSS from 'csstype' | |||
import { SystemStyleObject } from '@styled-system/css' | |||
import { Theme as StyledSystemTheme } from 'styled-system' |
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'm guessing the plan is to eventually have the entire Theme interface defined in this repo, right?
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.
@hasparus does this look okay-ish to you, both the types and the general build process of how this works (automatically)? Any thoughts/feedback?
Wow. Hello, imposter syndrome, my old friend.
It looks great! I've left 3 notes for the future.
BTW This is super weird that I can post a review to a repo where I'm not a contributor.
@@ -28,10 +31,13 @@ const parseProps = props => { | |||
return next | |||
} | |||
|
|||
export const jsx = (type, props, ...children) => | |||
export const jsx: typeof React.createElement = (type, props, ...children) => |
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.
Although it isn't 100% correct (jsx handles sx
and css
props, createElement doesn't), focusing on this now would be bikeshedding.
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.
Feel free to start submitting PRs to clean this up too 馃槈
Add resolveJsonModule to @theme-ui/core tsconfig
This "reworks"
@theme-ui/core
to be written in TypeScript and export its own type definitions. Thankfully,microbundle
has built-in support for TypeScript so we barely need to do any build system changes. I also checked in the generatedindex.d.ts
file (normally we would not check this into git) to show what it looks like.cc @hasparus the DefinitelyTyped maintainer for the theme-ui types. No action items right now, just wanted you to be aware of this effort 馃槈