-
Notifications
You must be signed in to change notification settings - Fork 671
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
Convert to TypeScript #668
Comments
Awesome work on this so far! For the preset packages, I wonder if it'd make sense to make a generic theme interface that most of these could follow, or at least use as a base interface |
For every package which is typed in DT, we’ll have to clean up and remove it from DT after merge to master. This would be components and theme-ui. I’m gonna take color (#672). I'll add http://www.typescriptlang.org/docs/handbook/migrating-from-javascript.html#getting-stricter-checks
|
I'm gonna take:
|
I'm thinking about taking a stab at typing import { Interpolation, SerializedStyles } from '@emotion/serialize';
import { SystemStyleObject } from '@styled-system/css';
// ...
/**
* A utility from @styled-system/css for theming styles to be passed to Emotion's
* css prop.
*
* Refer:
* 1. https://styled-system.com/css/
* 2. https://emotion.sh/docs/object-styles#with-css
*/
export function css(styleObject: Interpolation): (theme: Theme) => SerializedStyles;
/**
* The `sx` prop accepts a `SxStyleProp` object and properties that are part of
* the `Theme` will be transformed to their corresponding values. Other valid
* CSS properties are also allowed.
*/
export type SxStyleProp = SystemStyleObject; So |
I want to work on "preset-base" package. |
@hasparus I'm not 100% sure, but I suspect the DT typings might be a little outdated -- I think the typings for this should be completely contained within the theme-ui repo and not rely on Styled System, since that was part of the older implementation. It now uses |
I'd like to take How should we handle updating tests to TS? |
I’d like to give |
Hey @PaulieScanlon just fyi, there is a package in DT for components. It may be useful to you. https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/theme-ui__components/index.d.ts |
@hasparus hey, wow yeah that’s looks pretty complete! Is it just a case of moving it over and maybe just checking it’s online with the patterns core has set out? |
Do it! 💯
Once #672 lands we will be using
Kinda, but we don't want the |
Another todo:
|
I will do |
This comment has been minimized.
This comment has been minimized.
Love the project, looking to contribute- I'll take a stab at |
@LA1CH3 do it! 👍 |
I can work on gatsby-plugin-theme-ui as no one has claimed it yet. |
I am sorry for my "off-topic" comment, admittedly made as a knee-jerk reaction to coming across this issue. The thing is, I've been using theme-ui since its inception and have also been a fairly active contributor. I care a lot about the project because I really believe in the ideas behind it. However, I feel like this decision was made without thought to perhaps give time to gain feedback from the community and contributors. While I have no doubt that it's likely most people would favour TypeScript (at least at this point in time), I still think it would have been a reasonable thing to do, as there are valid reasons to stick with JavaScript too! Also, I've been aware of #121 for a while and up until a week ago, it's mostly been discussion around exporting type definitions. It seems like we are at the point now where it's no longer even a question whether it's reasonable to just drop JavaScript in an existing codebase and swap everything over to TypeScript. To me at least, that doesn't feel right. Anyway that's just my 2c! |
I've opened a new issue to discuss the conversion of the Gatsby-related packages in #950. I've also updated the list in the description here and removed it from the details/summary so that the progress is visible in GitHub's UI. |
I'd also suggest we decouple converting the docs site, since that isn't a published package and won't block a v0.4 release once the other packages are ready. |
are all done now. @hasparus can you update? |
Hey friends! So, running into some issues. When I import this library, I get a lot of typescript errors right now that prevent me from using it. Specifically errors like this:
I cloned down this repo and ran Seems like ya'll need to:
|
Hey @blainekasten 👋
All packages in the monorepo have separate TypeScript configs. Many of them are not strict, while root tsconfig is. We could make root TSConfig less rigid, and override it for stricter options in packages, but I suppose the intention was to recommend Right now to use theme-ui in TypeScript, one has to turn on
I think this should be fixed in #1002. I made @theme-ui/css strict there.
This is a good idea. Right now, the tests and the tested API surface is typechecked in GitHub Actions (ts-jest fails on type errors), but the tsconfig for tests also isn't strict. I think we should run I have a PR open to add |
Knock knock :D Let's regroup. We have
We can totally leave the On top of it:
|
Another thing that I'm not sure whether we need to address for 0.4 or not is upgrading microbundle. It looks like there are some TS/testing related issues in #1022 -- any advise there would be appreciated |
Hey @mxstbr, some unchecked packages can be checked in the original comment so it's easier to see the progress. Only 2 are left.
|
any thoughts on porting styled-system to typescript as well? styled-system seems like an important foundational library that theme-ui is built upon |
@jxnblk we just converted our theme-ui mono-repo based project from microbundle to https://preconstruct.tools which is the same as what emotion uses. It really sped up our build times (not insignificantly either, from 5 mins down to 12-15s) so it's worth considering. It also simplifies the build in that there only needs to be a single tsconfig and everything passes through babel and let's you use whatever typescript version you want. |
Okay, most code in the repo is in TypeScript for a long time already. Let's close this issue. |
Hey will there be types definitions (smth like index.d.ts) in this repo in the future? |
@MarcStdt Yes, from |
@MarcStdt 0.4.0 is not released yet (you can use |
@mxstbr I've been implementing themeui with a project and really enjoying using it apart from the typescript integration.. |
Thanks so much @hasparus Still getting type issues.. Initially we had this:
Now we have this:
Basically just trying to add a mixture of themeui and user defined container layout variants without getting type errors? |
@george-clark-s, could you create a new issue, discussion or ask for help on Discord? This issue is from 2020 and it's pretty big, so any conversation here will land in notifications for a bunch of people. |
thanks @hasparus Cant find anything on discord related to theme-ui.. do you know the channel/server? Edit (@hasparus here): Here you go, mate — https://discord.gg/theme-ui — it's linked in the readme if you need it again. |
Based on the discussions around #121 and the test we just merged in #662, we are going to convert theme-ui to TypeScript! 🎉 The plan is to go package-by-package and gradually move all
.js
files to.ts
files.As you can see there is a fair amount of packages to convert, so we would love your help! 💪
Here is how to convert a package (take a look at #662 for an example):
--tsconfig tsconfig.json
option to the "prepare" and "watch" commands in thepackage.json
and copy thetsconfig.json
from the core package."types": "dist/index.d.ts"
and"source": "src/index.ts"
to thepackage.json
.js
to.ts
and fix all type errors andany
types in the generated typedef (dist/index.d.ts
).To avoid duplicate work please comment which package you want to work on (as long as nobody else is also working on it) so we can mark it as taken.
Packages that still need to be converted:
These packages will be handled separately (see #950)
gatsby-plugin-theme-ui(WIP: refactor(gatsby-plugin-theme-ui): add typescript #705)gatsby-theme-code-recipes(refactor(gatsby-theme-code-recipes): convert to typescript #709)gatsby-theme-style-guidegatsby-theme-ui-blog( Convert gatsby-theme-ui-blog to TypeScript #711)gatsby-theme-ui-layout(Convert gatsby-theme-ui-layout to TypeScript #710)The text was updated successfully, but these errors were encountered: