From 0a4b8bd710fd04cb7aa9f8be0e9eebc20ea82576 Mon Sep 17 00:00:00 2001 From: Jeff Lange Date: Mon, 18 May 2026 16:46:06 -0400 Subject: [PATCH] Delegate smart_merge to the tailwind_merge gem Drops the hand-rolled CATEGORIES/SPACING/modifier tables in favor of gjtorikian/tailwind_merge, which mirrors tailwind-merge (JS) semantics and tracks Tailwind utility groups upstream. The public API is unchanged (smart_merge still takes a list of strings and returns a merged string), but conflict resolution now covers every Tailwind utility group rather than the subset previously encoded in CATEGORIES. Breaking: `hidden` is now part of the display conflict group, so `"flex hidden"` collapses to `"hidden"`. The previous carve-out was non-standard; the recommended replacement for JS-toggle visibility is the HTML5 `hidden` attribute via the existing `attribute` DSL. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 8 ++ README.md | 15 ++- TODO.md | 110 ++----------------- lib/view_component_css_dsl.rb | 189 +++------------------------------ view_component_css_dsl.gemspec | 11 ++ 5 files changed, 55 insertions(+), 278 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ad8e9bf..93309c5 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/README.md b/README.md index 7f40bba..abf4669 100644 --- a/README.md +++ b/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: diff --git a/TODO.md b/TODO.md index ba79521..cd282e8 100644 --- a/TODO.md +++ b/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. diff --git a/lib/view_component_css_dsl.rb b/lib/view_component_css_dsl.rb index 88c6f8d..dbd33b3 100644 --- a/lib/view_component_css_dsl.rb +++ b/lib/view_component_css_dsl.rb @@ -6,12 +6,17 @@ require "active_support/core_ext/hash/except" require "active_support/core_ext/hash/slice" require "active_support/core_ext/object/inclusion" +require "tailwind_merge" require_relative "view_component_css_dsl/version" module ViewComponentCssDsl extend ActiveSupport::Concern + # Single shared merger. tailwind_merge builds a conflict-group index on + # initialization, so we pay that cost once per process rather than per call. + MERGER = TailwindMerge::Merger.new + HTML_ATTR_KEYS = Set[ :alt, :aria, :autofocus, :class, :colspan, :contenteditable, @@ -29,97 +34,6 @@ module ViewComponentCssDsl :tabindex, :target, :title, :type, :value ].freeze - # Single combined regex for padding/margin spacing (replaces 14 separate patterns) - # Captures: type (p/m), axis (x/y/t/r/b/l or nil for all), value - SPACING_REGEX = /\b(p|m)(x|y|t|r|b|l)?-(\d+)\b/ - - # Maps axis character to Set of affected sides - SPACING_AXIS_MAP = { - nil => Set[:t, :r, :b, :l], # p-4, m-4 = all sides - "x" => Set[:l, :r], - "y" => Set[:t, :b], - "t" => Set[:t], - "r" => Set[:r], - "b" => Set[:b], - "l" => Set[:l] - }.freeze - - # Border width patterns (kept separate due to anchoring requirements) - BORDER_REGEX = /^border(?:-(x|y|t|r|b|l))?(?:-\d+)?$/ - - # Other category patterns (non-spacing, simple override by category) - # IMPORTANT: Use anchored patterns (^/$) to avoid matching substrings within - # compound classes (e.g., `h-8` within `min-h-8`, `flex` within `inline-flex`) - CATEGORIES = { - background: /^bg-/, - text_color: /^text-((\w+-\d+)|white|black|transparent|current|inherit|action|success|danger|warning|brand)(\/\d+)?$/, - text_size: /^text-(xs|sm|base|lg|xl|\d*xl)$/, - border_color: /^border-(?!t|r|b|l|x|y|\d)(\w+-\d+|\w+)(\/\d+)?$/, - width: /^w-/, - height: /^h-/, - min_width: /^min-w-/, - min_height: /^min-h-/, - max_width: /^max-w-/, - max_height: /^max-h-/, - # Display classes - note: `hidden` is intentionally excluded because it's - # commonly used as a visibility toggle alongside other display classes - # (e.g., "inline-flex hidden" where JS removes "hidden" to show element) - display: /^(block|inline-block|inline-flex|inline-grid|inline|flex|grid|table-cell|table-row|table|contents|flow-root|list-item)$/, - justify: /^justify-/, - align: /^items-/, - font_weight: /^font-(thin|extralight|light|normal|medium|semibold|bold|extrabold|black)$/, - rounded: /^rounded(-none|-sm|-md|-lg|-xl|-2xl|-3xl|-full)?$/, - position: /^(static|relative|absolute|fixed|sticky)$/ - }.freeze - - # Known Tailwind modifiers (prefixes like hover:, md:, first:, etc.) - # Classes with different modifiers should NOT conflict with each other - KNOWN_MODIFIERS = Set[ - # Responsive - "sm", "md", "lg", "xl", "2xl", - "max-sm", "max-md", "max-lg", "max-xl", "max-2xl", - # Interactive state - "hover", "focus", "focus-within", "focus-visible", "active", "visited", "target", - # Structural - "first", "last", "only", "odd", "even", - "first-of-type", "last-of-type", "only-of-type", "empty", - # Form state - "disabled", "enabled", "checked", "indeterminate", "default", - "required", "valid", "invalid", "in-range", "out-of-range", - "placeholder-shown", "autofill", "read-only", - # Pseudo-elements - "before", "after", "first-letter", "first-line", - "marker", "selection", "file", "backdrop", "placeholder", - # Media/Preference - "dark", "print", "portrait", "landscape", - "motion-safe", "motion-reduce", "contrast-more", "contrast-less", - "forced-colors", - # Direction - "rtl", "ltr", - # Attribute - "open", - # Direct children - "*" - ].freeze - - # Patterns that match dynamic modifiers (with optional names/arbitrary values) - # These use flexible regex to match ANY valid Tailwind modifier syntax - MODIFIER_PATTERNS = [ - /^group(?:\/\w+)?$/, # group, group/ - /^group-\w+(?:\/\w+)?$/, # group-hover, group-/ - /^peer(?:\/\w+)?$/, # peer, peer/ - /^peer-\w+(?:\/\w+)?$/, # peer-checked, peer-/ - /^aria-\w+$/, # aria-checked, aria- - /^aria-\[.+\]$/, # aria-[] - /^data-\[.+\]$/, # data-[] - /^supports-\[.+\]$/, # supports-[] - /^has-\[.+\]$/, # has-[] - /^group-has-\[.+\]$/, # group-has-[] - /^peer-has-\[.+\]$/, # peer-has-[] - /^min-\[.+\]$/, # min-[] - /^max-\[.+\]$/ # max-[] - ].freeze - included do class_attribute :_css_base, instance_writer: false, default: "" class_attribute :_css_axis_rules, instance_writer: false, default: [] @@ -341,99 +255,20 @@ def initialize_params_info end end - # Overwrites base css with custom css from the caller, but only if they actually - # interfere with each other. Modifier prefixes (hover:, md:, first:, etc.) create - # separate "namespaces" so they don't conflict with base classes. + # Merges Tailwind utility classes, resolving conflicts so the last-declared + # value wins. Delegates to the `tailwind_merge` gem, which mirrors + # tailwind-merge (JS) semantics and tracks Tailwind utility groups upstream. + # # Examples: # - base: "pt-2", custom: "pt-4", result => "pt-4" # - base: "pb-2", custom: "pt-4", result => "pb-2 pt-4" # - base: "pb-2", custom: "p-4", result => "p-4" # - base: "block", custom: "first:hidden", result => "block first:hidden" def smart_merge(*css_strings) - categorized = {} - uncategorized = [] - # Store spacing classes grouped by modifier prefix - # {prefix => [{class: "p-4", info: {type: :padding, axes: Set[:t,:r,:b,:l]}}, ...]} - spacing_by_prefix = Hash.new { |h, k| h[k] = [] } - - css_strings.compact.each do |str| - str.to_s.split.each do |cls| - prefix, base_class = extract_modifier_prefix(cls) - - spacing = spacing_info(base_class) - if spacing - # Remove any existing spacing classes that overlap on same type and axes - # within the same modifier prefix - spacing_by_prefix[prefix].reject! do |existing| - existing[:info][:type] == spacing[:type] && - existing[:info][:axes].subset?(spacing[:axes]) - end - spacing_by_prefix[prefix] << {class: cls, info: spacing} - else - category = detect_category(base_class) - if category - key = "#{prefix}:#{category}" - categorized[key] = cls - else - uncategorized << cls unless uncategorized.include?(cls) - end - end - end - end - - spacing_classes = spacing_by_prefix.values.flatten.map { |s| s[:class] } - (uncategorized + spacing_classes + categorized.values).join(" ") - end - - def spacing_info(css_class) - # Check padding/margin with single regex (replaces 14 pattern checks) - if (match = css_class.match(SPACING_REGEX)) - type = (match[1] == "p") ? :padding : :margin - axes = SPACING_AXIS_MAP[match[2]] - return {type:, axes:} - end - - # Check border width (needs separate handling due to anchoring) - if (match = css_class.match(BORDER_REGEX)) - axes = SPACING_AXIS_MAP[match[1]] - return {type: :border, axes:} - end - - nil - end - - def detect_category(css_class) - CATEGORIES.find { |_name, pattern| css_class.match?(pattern) }&.first - end - - # Extracts the modifier prefix from a Tailwind class - # e.g., "md:hover:bg-blue-500" → ["md:hover", "bg-blue-500"] - # e.g., "bg-white" → ["", "bg-white"] - def extract_modifier_prefix(css_class) - # Fast path: most classes don't have modifiers - return ["", css_class] unless css_class.include?(":") - - parts = css_class.split(":") - return ["", css_class] if parts.size == 1 - - # Find where modifiers end and the actual class begins - modifier_parts = [] - parts.each_with_index do |part, i| - if known_modifier?(part) && i < parts.size - 1 - modifier_parts << part - else - base_class = parts[i..].join(":") - return [modifier_parts.join(":"), base_class] - end - end - - # Fallback (shouldn't reach here) - ["", css_class] - end + input = css_strings.flat_map { |s| s.to_s.split }.reject(&:empty?).join(" ") + return "" if input.empty? - def known_modifier?(str) - return true if KNOWN_MODIFIERS.include?(str) - MODIFIER_PATTERNS.any? { |pattern| str.match?(pattern) } + MERGER.merge(input) end end diff --git a/view_component_css_dsl.gemspec b/view_component_css_dsl.gemspec index eca9f8e..2f88afa 100644 --- a/view_component_css_dsl.gemspec +++ b/view_component_css_dsl.gemspec @@ -28,9 +28,20 @@ Gem::Specification.new do |spec| end spec.require_paths = ["lib"] + ################################################################################### + # Dependencies + ################################################################################### + spec.add_dependency "activesupport", ">= 7.0", "< 9" + + # https://github.com/gjtorikian/tailwind_merge + spec.add_dependency "tailwind_merge", "~> 1.0" + + # https://github.com/ViewComponent/view_component + # https://viewcomponent.org/ spec.add_dependency "view_component", "~> 4.0" + # Development dependencies spec.add_development_dependency "reissue", "~> 0.4" spec.add_development_dependency "rspec", "~> 3.0" spec.add_development_dependency "standard", "~> 1.0"