crswll / tailwindcss-theme-swapper

A helper for getting tailwind values into css custom properties and switching them between media queries and classes. You can try it out here: https://play.tailwindcss.com/Gt21fePNvv
320 stars 15 forks source link

TypeScript #49

Open LandonSchropp opened 10 months ago

LandonSchropp commented 10 months ago

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?

Screenshot 2024-01-01 at 10 50 30 AM

crswll commented 10 months ago

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 Swapper<typeof baseTheme>. Will take another crack at it soon and if you have any suggestions let me know.

LandonSchropp commented 10 months ago

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.

LandonSchropp commented 10 months ago

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.

LandonSchropp commented 10 months ago

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.

crswll commented 10 months ago

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.

LandonSchropp commented 10 months ago

Sounds good. 👍

the child themes can't declare theme values that aren't in the base theme

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. 🙂

crswll commented 10 months ago

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 colors: { a: '#f00', b: '#0f0' } the following themes can only have a and/or b but not c... well at least in my brain. I guess that's technically not true but it's safer unless we want to nest themes instead of have a list.

LandonSchropp commented 10 months ago

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.

crswll commented 10 months ago

Got started on this. Getting pretty tripped up using it with Tailwind's plugin.withOption(...) API so will need to figure that out. Definitely in the works though.

crswll commented 9 months ago

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 plugin.withOptions<...>(plugin, extend) and might just need to make the plugin a preset instead.