gaucho-labs/tailwind-fuse

Incorrectly merged styles

gjtorikian opened this issue · 5 comments

Hi there! I maintain tailwind_merge, which is just a straight Ruby port of the JS tailwind-merge.

Every so often, I think about writing a Rust crate to back my Ruby gem. Today, I found this project. I was able to quickly incorporate the crate into my own, and was delighted to see that it works!

...for the most part.

When I ran my test suite (which again, is a port of tailwind-merge's), it came back with these errors, suggesting that in some cases tailwind-fuse isn't handling a few merge cases:

❯ be rake test  
Run options: --seed 33297

# Running:

..FFFFFF....SS.....F..FF..FFFF......FF.F..............FFF.

Fabulous run in 0.112988s, 513.3288 runs/s, 2363.0828 assertions/s.

  1) Failure:
TestModifiers#test_conflicts_across_prefix_modifiers [test/test_modifiers.rb:13]:
--- expected
+++ actual
@@ -1 +1 @@
-"hover:block focus:hover:inline"
+"hover:block hover:focus:inline focus:hover:inline"


  2) Failure:
TestModifiers#test_conflicts_across_postfix_modifiers [test/test_modifiers.rb:20]:
Expected: "text-lg/none"
  Actual: "leading-9 text-lg/none"

  3) Failure:
TestArbitraryProperties#test_handles_important_modifier_correctly [test/test_arbitrary_properties.rb:31]:
--- expected
+++ actual
@@ -1 +1 @@
-"[some:one] ![some:another]"
+"![some:prop] [some:other] [some:one] ![some:another]"


  4) Failure:
TestArbitraryProperties#test_handles_arbitrary_property_conflicts_correctly [test/test_arbitrary_properties.rb:11]:
--- expected
+++ actual
@@ -1 +1 @@
-"[paint-order:normal]"
+"[paint-order:markers] [paint-order:normal]"


  5) Failure:
TestArbitraryProperties#test_handles_arbitrary_property_conflicts_with_modifiers_correctly [test/test_arbitrary_properties.rb:18]:
--- expected
+++ actual
@@ -1 +1 @@
-"hover:[paint-order:normal]"
+"hover:[paint-order:markers] hover:[paint-order:normal]"


  6) Failure:
TestArbitraryProperties#test_handles_complex_arbitrary_property_conflicts_correctly [test/test_arbitrary_properties.rb:26]:
--- expected
+++ actual
@@ -1 +1 @@
-"[-unknown-prop:url(https://hi.com)]"
+"[-unknown-prop:::123:::] [-unknown-prop:url(https://hi.com)]"


  7) Failure:
TestArbitraryValues#test_handles_arbitrary_length_conflicts_with_labels_and_modifiers_correctly [test/test_arbitrary_values.rb:31]:
--- expected
+++ actual
@@ -1 +1 @@
-"focus:hover:m-[length:var(--c)]"
+"hover:focus:m-[2px] focus:hover:m-[length:var(--c)]"


  8) Failure:
TestPrefixes#test_prefix_working_corectly [test/test_prefixes.rb:11]:
Expected: "tw-hidden"
  Actual: "tw-block tw-hidden"

  9) Failure:
TestTailwindMerge#test_it_basically_works [test/test_tailwind_merge.rb:18]:
Expected: "stroke-[3]"
  Actual: "stroke-2 stroke-[3]"

 10) Failure:
TestSeparator#test_single_character_separator_working_correctly [test/test_seperator.rb:12]:
--- expected
+++ actual
@@ -1 +1 @@
-"focus_hover_!inset-0"
+"hover_focus_!right-0 focus_hover_!inset-0"


 11) Failure:
TestSeparator#test_multiple_character_separator_working_correctly [test/test_seperator.rb:22]:
--- expected
+++ actual
@@ -1 +1 @@
-"focus__hover__!inset-0"
+"hover__focus__!right-0 focus__hover__!inset-0"


 12) Failure:
TestTailwindCSSVersions#test_tailwind_3_4_features [test/test_tailwind_css_versions.rb:39]:
--- expected
+++ actual
@@ -1 +1 @@
-"has-[[data-potato]]:p-2 group-has-[:checked]:flex"
+"has-[[data-potato]]:p-1 has-[[data-potato]]:p-2 group-has-[:checked]:grid group-has-[:checked]:flex"


 13) Failure:
TestTailwindCSSVersions#test_tailwind_3_3_features [test/test_tailwind_css_versions.rb:14]:
Expected: "text-red text-lg/8"
  Actual: "text-lg/8"

 14) Failure:
TestArbitraryVariants#test_arbitrary_variants_with_modifiers [test/test_arbitrary_variants.rb:18]:
--- expected
+++ actual
@@ -1 +1 @@
-"dark:hover:lg:[&>*]:line-through"
+"dark:lg:hover:[&>*]:underline dark:hover:lg:[&>*]:line-through"


 15) Failure:
TestArbitraryVariants#test_arbitrary_variants_with_arbitrary_properties [test/test_arbitrary_variants.rb:45]:
--- expected
+++ actual
@@ -1 +1 @@
-"[&>*]:[color:blue]"
+"[&>*]:[color:red] [&>*]:[color:blue]"


 16) Failure:
TestArbitraryVariants#test_multiple_arbitrary_variants [test/test_arbitrary_variants.rb:40]:
--- expected
+++ actual
@@ -1 +1 @@
-"dark:hover:[&>*]:disabled:focus:[&_div]:line-through"
+"hover:dark:[&>*]:focus:disabled:[&_div]:underline dark:hover:[&>*]:disabled:focus:[&_div]:line-through"


 17) Failure:
TestNegativeValues#test_handles_conflicts_across_groups_with_negative_values_correctly [test/test_negative_values.rb:22]:
--- expected
+++ actual
@@ -1 +1 @@
-"focus:hover:inset-x-1"
+"hover:focus:-right-1 focus:hover:inset-x-1"


 18) Failure:
TestNegativeValues#test_handles_conflicts_between_positive_and_negative_values_correctly [test/test_negative_values.rb:17]:
Expected: "-top-69"
  Actual: "top-12 -top-69"

 19) Failure:
TestNegativeValues#test_handles_negative_value_conflicts_correctly [test/test_negative_values.rb:12]:
Expected: "-top-2000"
  Actual: "-top-12 -top-2000"

58 runs, 267 assertions, 19 failures

If you'd like to see for yourself, you can clone my repo, switch to the get-rusty branch, and run bundle install && bundle exec rake compile test (assuming you have Ruby installed).

Let me know if I can provide any more info to resolve these mismatches!

Thanks for reporting this! I can go through these one by one. There are definitely some bugs

1) Failure:
TestModifiers#test_conflicts_across_prefix_modifiers [test/test_modifiers.rb:13]:
--- expected
+++ actual
@@ -1 +1 @@
-"hover:block focus:hover:inline"
+"hover:block hover:focus:inline focus:hover:inline"

These two prefix modifiers actually create different tailwind classes, so I elected to handle them separately. If they were the same, wouldn't they only generate one tailwind class? I am open to hearing why they should be considered the same though.

.focus\:hover\:inline:hover:focus{
  display: inline
}

.hover\:focus\:inline:focus:hover{
  display: inline
}

This is a bug

  2) Failure:
TestModifiers#test_conflicts_across_postfix_modifiers [test/test_modifiers.rb:20]:
Expected: "text-lg/none"
  Actual: "leading-9 text-lg/none"

All the arbitrary values ones are not supported rn and should be fixed.

  8) Failure:
TestPrefixes#test_prefix_working_corectly [test/test_prefixes.rb:11]:
Expected: "tw-hidden"
  Actual: "tw-block tw-hidden"
  
 10) Failure:
TestSeparator#test_single_character_separator_working_correctly [test/test_seperator.rb:12]:
--- expected
+++ actual
@@ -1 +1 @@
-"focus_hover_!inset-0"
+"hover_focus_!right-0 focus_hover_!inset-0"

Prefixes and separators do not work with the macro. You need to configure options with this method https://docs.rs/tailwind_fuse/latest/tailwind_fuse/merge/fn.tw_merge_with_options.html

See #12

  9) Failure:
TestTailwindMerge#test_it_basically_works [test/test_tailwind_merge.rb:18]:
Expected: "stroke-[3]"
Actual: "stroke-2 stroke-[3]"

This is a bug

I think the rest of them are bugs

Hi, sorry for the late reply!

These two prefix modifiers actually create different tailwind classes, so I elected to handle them separately. If they were the same, wouldn't they only generate one tailwind class? I am open to hearing why they should be considered the same though.

I dug up the original commit to see if there was any additional context on this test: dcastil/tailwind-merge@68ad612

Since there isn't any additional context, I'll note what I'm observing. I'm not a CSS expert, but here's my logical breakdown:

  • According to this test, the classes hover:block hover:focus:inline focus:hover:inline should yield hover:block focus:hover:inline; that is, hover:focus:* is overwritten by focus:hover:*
  • Here's a Tailwind playground example which defines a similar arrangement: hover:bg-indigo-700 hover:focus:bg-red-700 focus:hover:bg-green-700
    • I think the inline / bg-color change here is irrelevant, I only chose them to more easily see the visual change
  • When all is said and done, only bg-red is activated:
Screenshot 2024-05-01 at 13 34 10

I think the issue here is that focus:hover and hover:focus end up representing the same two pseudoclasses, and so whatever version comes last is the one that should be applied. What do you think?

Prefixes and separators do not work with the macro. You need to configure options with this method

Apologies! I needed to do this in Ruby as well but neglected to port that config logic over to my Rust FFI.

Prefixes and separators do not work with the macro. You need to configure options with this method

Apologies! I needed to do this in Ruby as well but neglected to port that config logic over to my Rust FFI.

I can't for the life of me get this to work. Even a simple reproduction of the example from the README, like this

const OPTIONS: MergeOptions = MergeOptions {
    prefix: "",
    separator: "_",
};

fn merge(str1: String, str2: String) -> String {
    assert_eq!(
        "tw-bg-black tw-bg-white",
        tw_merge!("tw-bg-black", "tw-bg-white"),
    );

    set_merge_options(OPTIONS);

    assert_eq!("tw-bg-white", tw_merge!("tw-bg-black", "tw-bg-white"),);
    tw_merge!(str1, str2)
}

fails with

fatal: assertion `left == right` failed
  left: "tw-bg-white"
 right: "tw-bg-black tw-bg-white"

Any idea what could be different?

Any idea what could be different?

It was a classic PEBCAC—got this working, too.

The remaining test failures I see fall into the camp of new Tailwind 3.3/3.4 features, and this question of focus:hover versus hover:focus:

  1) Failure:
TestArbitraryVariants#test_arbitrary_variants_with_arbitrary_properties [test/test_arbitrary_variants.rb:46]:
--- expected
+++ actual
@@ -1 +1 @@
-"[&[data-foo][data-bar]:not([data-baz])]:noa:nod:[color:blue]"
+"[&[data-foo][data-bar]:not([data-baz])]:nod:noa:[color:red] [&[data-foo][data-bar]:not([data-baz])]:noa:nod:[color:blue]"


  2) Failure:
TestArbitraryVariants#test_arbitrary_variants_with_modifiers [test/test_arbitrary_variants.rb:18]:
--- expected
+++ actual
@@ -1 +1 @@
-"dark:hover:lg:[&>*]:line-through"
+"dark:lg:hover:[&>*]:underline dark:hover:lg:[&>*]:line-through"


  3) Failure:
TestArbitraryVariants#test_multiple_arbitrary_variants [test/test_arbitrary_variants.rb:40]:
--- expected
+++ actual
@@ -1 +1 @@
-"dark:hover:[&>*]:disabled:focus:[&_div]:line-through"
+"hover:dark:[&>*]:focus:disabled:[&_div]:underline dark:hover:[&>*]:disabled:focus:[&_div]:line-through"


  4) Failure:
TestSeparator#test_multiple_character_separator_working_correctly [test/test_seperator.rb:22]:
--- expected
+++ actual
@@ -1 +1 @@
-"focus__hover__!inset-0"
+"hover__focus__!right-0 focus__hover__!inset-0"


  5) Failure:
TestSeparator#test_single_character_separator_working_correctly [test/test_seperator.rb:12]:
--- expected
+++ actual
@@ -1 +1 @@
-"focus_hover_!inset-0"
+"hover_focus_!right-0 focus_hover_!inset-0"


  6) Failure:
TestPrefixes#test_prefix_working_corectly [test/test_prefixes.rb:11]:
Expected: "tw-hidden"
  Actual: "tw-block tw-hidden"

  7) Failure:
TestModifiers#test_conflicts_across_prefix_modifiers [test/test_modifiers.rb:13]:
--- expected
+++ actual
@@ -1 +1 @@
-"hover:block focus:hover:inline"
+"hover:block hover:focus:inline focus:hover:inline"


  8) Failure:
TestArbitraryValues#test_handles_arbitrary_length_conflicts_with_labels_and_modifiers_correctly [test/test_arbitrary_values.rb:31]:
--- expected
+++ actual
@@ -1 +1 @@
-"focus:hover:m-[length:var(--c)]"
+"hover:focus:m-[2px] focus:hover:m-[length:var(--c)]"


  9) Failure:
TestArbitraryProperties#test_handles_arbitrary_property_conflicts_with_modifiers_correctly [test/test_arbitrary_properties.rb:20]:
--- expected
+++ actual
@@ -1 +1 @@
-"focus:hover:[paint-order:normal]"
+"hover:focus:[paint-order:markers] focus:hover:[paint-order:normal]"


 10) Failure:
TestNegativeValues#test_handles_conflicts_across_groups_with_negative_values_correctly [test/test_negative_values.rb:22]:
--- expected
+++ actual
@@ -1 +1 @@
-"focus:hover:inset-x-1"
+"hover:focus:-right-1 focus:hover:inset-x-1"


 11) Failure:
TestTailwindCSSVersions#test_tailwind_3_3_features [test/test_tailwind_css_versions.rb:16]:
--- expected
+++ actual
@@ -1 +1 @@
-"start-1 end-1 ps-1 pe-1 ms-1 me-1 rounded-s-md rounded-e-md rounded-ss-md rounded-ee-md"
+"start-1 end-1 ps-0 ps-1 pe-0 pe-1 ms-1 me-1 rounded-s-md rounded-e-md rounded-ss-md rounded-ee-md"


 12) Failure:
TestTailwindCSSVersions#test_tailwind_3_4_features [test/test_tailwind_css_versions.rb:39]:
--- expected
+++ actual
@@ -1 +1 @@
-"has-[[data-potato]]:p-2 group-has-[:checked]:flex"
+"has-[[data-potato]]:p-1 has-[[data-potato]]:p-2 group-has-[:checked]:grid group-has-[:checked]:flex"