Skip to content
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

Tailwind class prefix support #12

Closed
zakstucke opened this issue Mar 10, 2024 · 15 comments · Fixed by #21
Closed

Tailwind class prefix support #12

zakstucke opened this issue Mar 10, 2024 · 15 comments · Fixed by #21

Comments

@zakstucke
Copy link

zakstucke commented Mar 10, 2024

Cool library! Can prefixes be supported?

I use "tw-" prefix on tailwind classes.
tw_merge!() seems to not recognise prefixed classes correctly, and acts like tw_join!() instead.

E.g. tw_merge!("tw-bg-red-500", "tw-bg-black-500") comes out as "tw-bg-red-500 tw-bg-black-500"

@zakstucke zakstucke changed the title Tailwind class prefix. Tailwind class prefix support Mar 10, 2024
@nicoburniske
Copy link
Member

I have to improve the docs for this.

The macro tw_merge! does not support prefix, but the inner function does.

Take a look at this https://docs.rs/tailwind_fuse/latest/tailwind_fuse/merge/fn.tw_merge_with_options.html

@nicoburniske
Copy link
Member

I am not sure how I could support this with the macro. If you have any ideas lmk

@nicoburniske
Copy link
Member

what do you think about this? It's pretty annoying to supply the options everywhere though. The arrow symbol can be replaced for something else if there's a better alternative

#[test]
fn test_override_config_macro() {
    let config = MergeOptions {
        prefix: "tw-",
        separator: "|",
    };

    let result = tw_merge!(config => "hover|lg|tw-bg-blue-100", "hover|lg|tw-bg-red-500");

    assert_eq!("hover|lg|tw-bg-red-500", result);
}

@zakstucke
Copy link
Author

zakstucke commented Apr 12, 2024

Hi @nicoburniske, sorry for only looping back now.

I'm not a big fan of your config => experiment, it think it would at a lot of bloat and mental overhead around a codebase.

Here's my full solution I'm currently using:

// prelude.rs

macro_rules! tw_mymerge {
    ($($item:expr),+ $(,)?) => {{
        let joined = tailwind_fuse::tw_join!($($item),+);
        tailwind_fuse::merge::tw_merge_with_options(joined.as_str(), tailwind_fuse::merge::MergeOptions {
            prefix: "tw-",
            separator: ":",
        })
    }};
}
# clippy.toml

disallowed-methods = ["tailwind_fuse::IntoTailwindClass::with_class"]
disallowed-macros = ["tailwind_fuse::tw_merge"]

Effectively I'm just banning usage of tw_merge and with_class (with_class obvs broken too as uses it under hood so should use tw_mymerge(Class.to_class(), ".."), to make sure I'm only using my tw_mymerge!() version that has the config built into it. I'm not seeing a better way than this right now, but this works great in the sense I don't have to worry about it again!

Feel free to close this if I've hit on the least painful way to deal with this :)

@zakstucke
Copy link
Author

I've actually had to disable the disallowed-methods = ["tailwind_fuse::IntoTailwindClass::with_class"] in clippy though (I only added the clippy linting whilst writing this) as it get's triggered by the TwClass macro, might be a clippy bug, doesn't take away from general solution though!

@nicoburniske
Copy link
Member

Hey, thanks for using the crate and giving me feedback. It's clear that this needs to be improved.

You're spot on with the tw_mymerge impl, though I am not sure it's a good idea to assume that users can write their own macros 😄

In order for you to use the derive macros TwClass and TwVariant, I recommend using the merger attribute (bad name, should be called fuser) in the TwClass macro. It's undocumented, and I also think is a little verbose. I will propose an alternate design after.

Using TailwindOptions with Derive Macros

Right now, you can supply a custom merger like this

This lets you opt out of the merge logic if you don't need it.

#[derive(TwClass, Default)]
#[tw(merger = TailwindJoin)]
struct Btn {
    size: BtnSize,
    color: BtnColor,
}

The two options provided to you are TailwindJoin and TailwindMerge. These are both structs with no fields that implement TailwindFuse trait

/// Used to Fuse Tailwind Classes together.
pub trait TailwindFuse {
    /// Strings are not guaranteed to be single class nor free of whitespace.
    fn fuse_classes(&self, class: &[&str]) -> String;
}

So you could implement your own version of this with your custom options

/// Will merge Tailwind classes and handle conflicts using [`tw_merge()`]
pub struct CustomMerge;

impl TailwindFuse for CustomMerge {
  fn fuse_classes(&self, class: &[&str]) -> String {
     tailwind_fuse::merge::tw_merge_with_override(class, tailwind_fuse::merge::MergeOptions {
       prefix: "tw-",
       separator: ":",
     })
  }
}

Then you could use it in all your structs like this

#[derive(TwClass, Default)]
#[tw(merger = CustomMerge)]
struct Btn {
    size: BtnSize,
    color: BtnColor,
}

@zakstucke
Copy link
Author

zakstucke commented Apr 12, 2024

Awesome! I'll give that a go, means for me with my macro and the clippy lint on tw_merge this seems to be all sorted.

You're spot on with the tw_mymerge impl, though I am not sure it's a good idea to assume that users can write their own macros 😄

Haha makes sense. To be honest I've never seen a different prefix than tw-, this is probably the most common case by far, could maybe make a tw-prefix feature which uses my macro instead of current when feature is enabled? This could be done for the most common prefixes people use (feature for each), and anyone else recommended to use their own macro. Not perfect but would probs cover most cases. This would also be possible to cover the merger need above too.

@zakstucke
Copy link
Author

zakstucke commented Apr 12, 2024

@nicoburniske I actually just thought of a cool way to deal with this:

use linkme::distributed_slice;

// UPSTREAM IN TAILWIND_FUSE
#[distributed_slice]
pub static PREFIX_OVERRIDE: [&'static str];

// UPSTREAM IN TAILWIND_FUSE
// Internally in tailwind fuse use this to get prefix, either the one from the slice a user set, or defaulting to nothing like normal.
pub fn get_prefix() -> &'static str {
    PREFIX_OVERRIDE.first().unwrap_or(&"")
}

/// UPSTREAM IN TAILWIND_FUSE
/// Hiding the implementation behind a simple macro.
macro_rules! set_tw_custom_prefix {
    ($prefix:expr) => {
        const _: () = {
            use linkme::distributed_slice;
            #[distributed_slice(PREFIX_OVERRIDE)]
            static TAILWIND_FUSE_DEFAULT_PREFIX: &str = $prefix;
        };
    };
}

// DOWNSTREAM IN USER CODE
set_tw_custom_prefix!("tw-");

https://github.com/dtolnay/linkme

It's pretty cool this crate and I use this approach for something else too. Could it work here and remove the need for any of this?

@nicoburniske
Copy link
Member

Wow that's insanely cool @zakstucke

This seems like a better approach to me than the env variables PR I linked because it's code first.

@zakstucke
Copy link
Author

Yeah I saw that come in at the same time I sent this! I'd agree imo this is a slightly cleaner approach, but I think either would work.

@nicoburniske
Copy link
Member

Hm @zakstucke linkme doesn't have WASM support unfortunately.

dtolnay/linkme#6

@zakstucke
Copy link
Author

Ah such a shame, I hadn't seen that.
Well, your approach still sounds like a good way forward!

@nicoburniske
Copy link
Member

@zakstucke see #21

@nicoburniske
Copy link
Member

Both your issues (Option in macros, and Prefix) should be solved in 0.3.0

@zakstucke
Copy link
Author

@nicoburniske OnceLock was a great middle ground!

I've just setup with 0.3 and both this prefixing and the re-support of Option work seamlessly, thanks so much for your hard work! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants