-
Notifications
You must be signed in to change notification settings - Fork 12
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
TypeScript #49
Comments
No problem at all! I have considered adding types, even started a couple times but couldn't get it to be as intuitive as I want. I feel like this is pretty close but I don't really like having to do |
I'm happy to jump in as well if you'd like. 🙂 Given the size of the repo, it might be easier to rewrite the source itself in TypeScript. Then the compiler generates the types for you and you don't have the extra maintenance burden of keeping the types in sync with the code. I understand if you don't want to go down that road though. |
I played with your config a bit and here's what I came up with. I wouldn't call myself a TypeScript expert though, and I'm not deeply familiar with the options in this repo, so I'd take that with a grain of salt. |
In terms of preventing extra properties, I dug a bit and I don't think that's a thing TypeScript supports currently (GitHub issue). It seems like it could be possible using some mangled type magic, but it doesn't seem worth it to me. |
Yeah, it probably wouldn't hurt to write it in TypeScript. I mostly didn't because I didn't want to add a build step. It's likely worth it, though. Your types are close but I think an important thing to make sure of is that the child themes can't declare theme values that aren't in the base theme which mine does but I don't think well. I'll start playing around with converting to TypeScript and maybe make some breaking changes I've wanted to. |
Sounds good. 👍
I don't think I quite understand this req. Maybe you could give a quick example. No worries if you just want to tackle it yourself as well—I don't mean to backseat drive. 🙂 |
Oh it's all good sorry if it sounds otherwise! Don't think you're backseat driving at all. I mean like if the base theme has |
Ah, I see what you were going for. I think this works. It uses generics to pass through the base theme config to the other types, ensuring that the other configs are always a deep partial of it. The only downside of this approach I've found so far is that the error message isn't necessarily the easiest to understand. |
Got started on this. Getting pretty tripped up using it with Tailwind's |
Had some time today and sit and work on this. I have to get the tooling set up around it but think the hard part is done. I still can't get the types passed properly to |
Thank you for this plugin!
I was wondering, have you considered converting the plugin to TypeScript or adding types to this repo to support Tailwind's TypeScript configuration?
The text was updated successfully, but these errors were encountered: