Delegate smart_merge to the tailwind_merge gem

CHANGELOG.md

@@ -4,6 +4,14 @@
 
 ## [0.1.3] - Unreleased
 
+### Changed
+
+- `smart_merge` now delegates to the `tailwind_merge` gem. The public API is unchanged, but conflict resolution now matches upstream tailwind-merge semantics across every Tailwind utility group (previously only spacing, sizing, colors, display, justify, align, font-weight, rounded, and position were handled — shadow, ring, gap, space, divide, z, opacity, leading, tracking, transition, transform, blur, etc. now merge correctly too).
+
+### Breaking
+
+- `hidden` is now treated as part of the display-class conflict group. Strings like `"flex hidden"` collapse to `"hidden"` rather than keeping both. Previous behavior was non-standard — `tailwind-merge` (JS) and `tailwind_merge` (Ruby) have always treated `hidden` as a display utility. For JS-toggle visibility patterns, use the HTML5 `hidden` attribute (e.g., `attribute hidden: -> { … }` or pass `hidden: true` as an html_attr) and toggle it with `element.toggleAttribute('hidden')` instead of relying on the class merger.
+
 ## [0.1.2] - 2026-05-15
 
 ### Added

README.md

@@ -410,7 +410,7 @@ Renders:
 
 ## Smart merge behavior
 
-Smart-merge handles Tailwind's conventions so caller and component CSS can coexist sensibly. In every row below, the **Component** column is what the component declared via `css`, and the **Caller** column is what was passed in `class:` at the call site.
+Smart-merge handles Tailwind's conventions so caller and component CSS can coexist sensibly. Under the hood it delegates to the [`tailwind_merge`](https://github.com/gjtorikian/tailwind_merge) gem, which mirrors [tailwind-merge](https://github.com/dcastil/tailwind-merge) (JS) semantics. In every row below, the **Component** column is what the component declared via `css`, and the **Caller** column is what was passed in `class:` at the call site.
 
 | Component | Caller | Final classes | Why |
 | --- | --- | --- | --- |
@@ -427,6 +427,19 @@ Smart-merge handles Tailwind's conventions so caller and component CSS can coexi
 
 Modifier prefixes (`hover:`, `md:`, `dark:`, `group/`, `peer-checked:`, `aria-*`, arbitrary `[…]` values, etc.) form their own merge namespace, so `hover:bg-blue-500` never conflicts with a base `bg-white`.
 
+### JS-toggle visibility — use the `hidden` attribute, not the class
+
+`hidden` is treated as part of the display group, so `"flex hidden"` collapses to `"hidden"` (the same as upstream tailwind-merge). If you need to toggle visibility from JavaScript while preserving a base display class, use the HTML5 `hidden` attribute via the `attribute` DSL:
+
+```ruby
+class PaneComponent < ApplicationComponent
+  css "block"
+  attribute hidden: -> { collapsed? }
+end
+```
+
+Then toggle from JS with `element.toggleAttribute('hidden')` or `element.hidden = true/false`. The class merger stays out of the way and the element retains its `block`/`flex`/etc. layout when shown.
+
 ## Inheritance
 
 A child component's `css "..."` declaration is smart-merged with its parent's:

TODO.md

@@ -2,97 +2,7 @@
 
 Working notes for follow-up improvements. Likely deleted once items land.
 
-## 1. Investigate replacing `smart_merge` with the `tailwind_merge` gem
-
-Our `smart_merge` reimplements Tailwind class conflict resolution from scratch
-— see the `CATEGORIES` table, `spacing_info`, `extract_modifier_prefix`, and
-the modifier sets in `lib/view_component_css_dsl.rb`. The gem
-`gjtorikian/tailwind_merge` (Ruby port of the well-known JS `tailwind-merge`)
-ships an upstream-tracked version of the same logic and stays current with
-Tailwind releases.
-
-The DSL itself (`css "..."`, `css :method?`, `css variant:`, axis caching,
-inheritance) is **not** up for replacement. Only the internal merge step at
-the end is — `smart_merge` is the part that would delegate to the gem.
-
-### Primary concern to verify: shorthand vs longhand spacing parity
-
-This is the case most worth confirming before any other work. Our spacing
-logic treats `p-*` (all four sides) as shadowing `px-*`/`py-*`/`pl-*`/etc.
-via the axis `subset?` check around line 272 of `view_component_css_dsl.rb`.
-tailwind-merge claims to do the same, but we should prove it on concrete
-cases:
-
-- `p-4` then `pl-2` → drop `p-4`'s left coverage, keep both classes? Or drop
-  `p-4` entirely? Compare both implementations.
-- `pl-2` then `p-4` → drop `pl-2`, keep only `p-4`.
-- `px-4` then `pl-2` → drop `px-4`'s left coverage.
-- Same patterns for `m-*` and `border-*` widths.
-- Mixed modifiers: `md:p-4` then `pl-2` should leave both (different prefix
-  namespaces); `md:p-4` then `md:pl-2` should follow the rules above within
-  the `md:` namespace.
-
-Build a small parity test that runs the same inputs through both mergers
-and diffs the outputs. Any divergence is a blocker.
-
-### Coverage gaps to verify the gem closes
-
-Things our `smart_merge` does *not* currently handle but tailwind-merge does:
-
-- **Long-tail utility groups** not in `CATEGORIES`: `shadow-*`, `ring-*`,
-  `gap-*`, `space-*`, `divide-*`, `z-*`, `opacity-*`, `leading-*`,
-  `tracking-*`, `transition-*`, `transform`, `scale-*`, `rotate-*`,
-  `translate-*`, `blur-*`, `backdrop-*`, animation utilities. Today these
-  fall through as uncategorized and survive in the final string even when
-  they conflict.
-- **Arbitrary values**: `bg-[#1da1f2]`, `h-[42px]`, `w-[calc(100%-1rem)]`.
-- **Important marker**: `!bg-red-500` is currently grouped with `bg-red-500`.
-- **Negative values**: `-m-2` — `SPACING_REGEX` requires a digit immediately
-  after the prefix, so this slips through.
-
-### Performance work
-
-The hand-rolled merger is tuned for our patterns and has multiple cache
-layers. Before swapping, prove the gem doesn't regress real-world rendering:
-
-- Build a benchmark mirroring actual usage — many components per page, each
-  running merge against base + variants + custom. Measure cold-start, warm
-  calls on identical inputs, and memory footprint.
-- Hold the `Merger` instance once at class level (or in a constant). The gem
-  has measurable startup cost to build its conflict-group index — that cost
-  should be paid once per process, not per call.
-- Preserve `_css_cache` and `_css_merge_cache`. They cache the *result* of
-  merging keyed on component instance state, so they sit above whichever
-  merger is called internally. Confirm cache hit rates don't change if the
-  gem produces different (but equivalent) output orderings.
-- If we lose meaningful perf under our caching strategy, that's a stay
-  signal.
-
-### Risk and exit plan
-
-- 143-star gem; reasonably maintained but smaller than Rails-core stable.
-  Pin to a known version. Because the interface is essentially one method
-  (`merge(String) → String`), reverting to the hand-rolled version is
-  straightforward if the gem stalls or breaks.
-- Confirm the gem's update cadence keeps pace with Tailwind releases we
-  rely on.
-
-### Decision criteria
-
-Swap if **all** hold:
-- Spacing parity proven via direct comparison on real inputs.
-- Caching survives without measurable regression.
-- Audit confirms we hit utility groups the gem covers and we don't (worth
-  the swap), or that the maintenance burden of expanding `CATEGORIES`
-  ourselves is real and recurring.
-
-Stay hand-rolled if any of:
-- Behavior diverges on cases we can't easily reconcile.
-- Perf regresses materially even with the existing cache layers.
-- Audit shows we don't actually exercise the missing utility groups, so the
-  gain is small relative to the dependency cost.
-
-## 2. Eliminate the template splat boilerplate
+## 1. Eliminate the template splat boilerplate
 
 Even with first-class declarations, templates still need `<%= tag.div **html_attrs do %>`. One direction worth researching: a helper that takes just the tag name and injects attrs automatically:
 
@@ -104,18 +14,18 @@ Even with first-class declarations, templates still need `<%= tag.div **html_att
 
 Defaulting to `div` when no tag is given. Open question whether there are real cases where the dev needs to intervene between the helper and the splat.
 
-## 3. Lightweight in-template styling for sub-elements
+## 2. Lightweight in-template styling for sub-elements — decided: NOT doing this (2026-05-18)
 
-The DSL's discipline today is: top-level element gets `css` declarations; sub-elements that need dynamic styling get promoted to their own ViewComponent. Clean, but can feel out of proportion to the change — especially for "tweener" sub-elements that only react to a single boolean from the parent. Real example: `PerceptionCommentButton` has sub-elements whose CSS is dynamic but trivially derived from parent state; not complex enough to warrant a sub-component, but no DSL-shaped path to express it inline.
+**Decision:** The DSL stays scoped to the top-level component element. No `sub_css`, `icon_css`, `*_html_attrs`, or inline-slot styling shortcut. Sub-elements that need dynamic CSS get extracted into their own ViewComponent.
 
-Possible directions worth exploring:
+**Reason:** API stickiness is a one-way door. Shipping a sub-element API now and removing it later is a breaking change requiring caller migration; adding it later is purely additive and breaks nothing. Combined with thin sample data — only one real-world case examined (`PerceptionResponseButtonComponent` in FORGE) and even that one was ambiguous (drag-handle wants extraction, comment-button is wrapper-shaped) — conservative is correct.
 
-- **In-line declarative methods** — class-level declarations the template can call to get a merged class string for a sub-element.
-- **DSL extension** like `sub_css :icon, base: "...", variant: {primary: "..."}` returning a merged string.
-- **"Inline slot"** — render as a slot without the ceremony of a separate ViewComponent file.
+**May revisit if:** the same "trivial sub-element reacting to one parent prop" pattern recurs 3+ times across real components with the same shape. At that point we'd have enough data to design the narrowest possible API (strict `base:` + `when: :predicate?`, no `variant:`/axes) with confidence the shape fits.
 
-Tension: any of these can scope-creep into a parallel mini-DSL and undermine the discipline that real components are the unit of composition.
+**Counter-arguments to expect when this resurfaces:**
 
-Counterpoint to itself: maybe that discipline IS the point, and the right answer is better extraction tooling/guidance rather than making it easier to avoid extraction.
+- *"Components are over-ceremonious for trivial cases."* True cost, but the right fix is scaffold/generator tooling, not a parallel styling DSL.
+- *"Refactoring sub-elements back to components is easy."* Refactor cost is roughly symmetric in both directions; the real asymmetry is API commitment, which is one-way.
+- *"A sub-element DSL would be cleaner than today's `class_names` methods."* True, but milder than the original messiness argument and not enough by itself to justify the one-way door.
 
-Research goal: find the smallest pattern that handles the "trivial sub-element reacting to a single parent prop" case without enabling sprawl. Or conclude there isn't one and lean in.
+**Follow-up:** add a short "when to extract a sub-component" note to README so library users understand the discipline is intentional, not an oversight.