ben-rogerson / twin.macro

๐Ÿฆนโ€โ™‚๏ธ Twin blends the magic of Tailwind with the flexibility of css-in-js (emotion, styled-components, solid-styled-components, stitches and goober) at build time.
MIT License
7.92k stars 183 forks source link

allow omitting DEFAULT when using theme #749

Closed ajmnz closed 1 year ago

ajmnz commented 1 year ago

Fixes https://github.com/ben-rogerson/twin.macro/discussions/679#discussioncomment-3998674

@ben-rogerson I wanted to prevent modifying the theme parsing function, in order to align with your comment

so the theme function is more predictable when it's used to grab objects from the config

That's why I added the check right before replacing the correct value to prevent it being replaced as bg-[Object object] instead of bg-[#whatever].

Let me know if that works!

ben-rogerson commented 1 year ago

Thanks for adding this feature.

~If I add this back in, how could we allow people to bring back an object?~

~eg: If I want an object of all the yellow color values from my config, how would I grab that with the theme import?~ Sorry - all good! this is my concern for the theme import rather than a theme value replacement in an arbitrary value/property.

ajmnz commented 1 year ago

Thanks for adding this feature.

~If I add this back in, how could we allow people to bring back an object?~

~eg: If I want an object of all the yellow color values from my config, how would I grab that with the theme import?~ Sorry - all good! this is my concern for the theme import rather than a theme value replacement in an arbitrary value/property.

Awesome!

I don't think there's anything we can really do with the theme import to support the DEFAULT resolution. Though since I don't know the codebase, please make sure that my changes do not affect the theme import as well. (I didn't know how to import theme in tests in order to check that)

ben-rogerson commented 1 year ago

Yeah this affects the theme import, it doesn't use this function but this change would mean theme import now acts differently. I'm pretty keen to allow objects to be returned with the theme import - it's best we align it to the behaviour of the tailwind theme() function.

eg:

theme`colors.black`
tw`bg-[theme(colors.black)]` // This one was updated

// โ†“ โ†“ โ†“ โ†“ โ†“ โ†“

({
  "DEFAULT": "#111111",
  "pure": "#000000 "
});
({
  "--tw-bg-opacity": "1",
  "backgroundColor": "rgb(17 17 17 / var(--tw-bg-opacity))"
});
ajmnz commented 1 year ago

Yep, I agree. Having two functions named the same and acting differently is quite confusing. We can close this PR then.

ben-rogerson commented 1 year ago

Leave it with me - I'll use your PR as a base for any work ๐Ÿ‘ Cheers