tailwindcss-theme-swapper icon indicating copy to clipboard operation
tailwindcss-theme-swapper copied to clipboard

TypeScript

Open LandonSchropp opened this issue 6 months ago • 10 comments

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

LandonSchropp avatar Jan 01 '24 18:01 LandonSchropp

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.

crswll avatar Jan 03 '24 00:01 crswll

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 avatar Jan 04 '24 20:01 LandonSchropp

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 avatar Jan 04 '24 20:01 LandonSchropp

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.

LandonSchropp avatar Jan 04 '24 20:01 LandonSchropp

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.

crswll avatar Jan 04 '24 22:01 crswll

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

LandonSchropp avatar Jan 05 '24 01:01 LandonSchropp

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.

crswll avatar Jan 05 '24 01:01 crswll

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.

LandonSchropp avatar Jan 06 '24 20:01 LandonSchropp

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 avatar Jan 09 '24 01:01 crswll

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.

crswll avatar Jan 15 '24 22:01 crswll