From b5156632c5f22ba499a20d5fc32d88220290042d Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Sat, 8 Aug 2026 21:03:55 +0800 Subject: [PATCH] fix(tailwind-merge): declare the tap spacing token now its blocker is disproved `tap` was held out of CLINICAL_TWMERGE_THEME.spacing because declaring it would hand same-property conflicts to the later class -- "22 call sites, 18 of which would drop from 48px to 32/36/40/42px". #270 re-measured that premise and it does not hold: zero same-variant pairs exist, and the 84 survivors are cross-variant responsive step-downs that tailwind-merge cannot reach because it groups by variant. Several named call sites were stale outright -- DocumentManagerPanel and settings-dialog carry no tap token at all. The gap that kept #270 open was that a per-string-literal scan cannot see a conflict composed across cn() arguments: cn(recipe, "min-h-tap") pairs the recipe's min-h-7 with the token, and order decides the outcome -- recipe-then-tap raises to 48px, tap-then-recipe drops to 28px, the forbidden direction. So the sweep resolves constant recipe identifiers at each call site before grouping by variant AND property. Result across 700 files and 1418 cn() call sites (1410 with resolvable arguments, 802 resolvable class constants): ZERO same-variant pairs, in either direction. Two defects were found and fixed in the sweep itself before trusting that zero, because its first run reported three drops and all three were artefacts: - Ternary arms were concatenated, so `size === 44 ? "h-tap w-tap" : "h-[38px] w-[38px]"` looked like one element carrying both. Arms are mutually exclusive; arguments now expand to alternatives and compositions are enumerated. - Class constants were keyed by bare name globally, so `fieldControl` in one module resolved to a same-named constant in another and invented a conflict that does not exist at the call site. Resolution is now per file, with a global fallback only where a name is unambiguous repo-wide. The zero is mutation-tested rather than assumed: synthetic probes cn("h-tap", "h-4") and cn("min-h-tap", recipe) are both flagged as drops, cn(recipe, "min-h-tap") is correctly reported as a raise, and cn("min-h-tap", "sm:min-h-9") is correctly not flagged at all. The pinning test is replaced rather than deleted. It now asserts both halves: a same-variant pair merges to the last class, and cross-variant responsive step-downs pass through untouched. That second case is the load-bearing one -- if it ever merges, a control loses its breakpoint step. No production tap target is lowered, and no cross-variant numeric is deleted -- they are live responsive steps, not dead classes. Verified: tests/tailwind-merge-config.test.ts 34 passed (34); npm run test -- 5590 passed, 2 pre-existing environmental failures unchanged from the same run before this change (mode-nav-addon-slot absolute Windows paths, pr-handoff-stop); check:design-system-contract, check:icon-scale, check:type-scale all exit 0; format:check clean. Chromium look is delegated to this PR's Production UI jobs and is not claimed as run locally. Refs #270 --- src/lib/tailwind-merge.ts | 42 ++++++++++++++++++++--------- tests/tailwind-merge-config.test.ts | 32 ++++++++++++++-------- 2 files changed, 50 insertions(+), 24 deletions(-) diff --git a/src/lib/tailwind-merge.ts b/src/lib/tailwind-merge.ts index ba477dac5..2ef7b02ee 100644 --- a/src/lib/tailwind-merge.ts +++ b/src/lib/tailwind-merge.ts @@ -63,19 +63,34 @@ export const CLINICAL_TWMERGE_THEME = { // which are padding utilities and should conflict with pt-*/pb-* like any // other. // - // `tap` (--spacing-tap, the 48px target knob) is DELIBERATELY ABSENT, and - // this is the one omission that is not an oversight. Tailwind emits - // `.min-h-tap` after every numeric `.min-h-*` and `.h-tap` after `.h-4` / - // `.h-10.5`, so at equal specificity the tap token wins today wherever a - // call site pairs the two. Declaring `tap` here would hand the win to the - // later class instead — measured across 22 call sites (document-admin, - // DocumentManagerPanel, favourites-hub, settings-dialog, service-detail-page, - // form-detail-page, clinical-output-helpers, account-setup-dialog), 18 of - // which would drop from 48px to 32/36/40/42px. AGENTS.md and SPEC §4.10 are - // explicit that no production target is ever reduced, so the merge stays off - // for this family until those sites drop the numeric class they already - // cannot apply. Until then `min-h-tap min-h-9` passes through unmerged, - // exactly as it does today; `tests/tailwind-merge-config.test.ts` pins that. + // `tap` (--spacing-tap, the 48px target knob) was held out of this list until + // 8 August 2026. The stated reason was that declaring it hands the win to the + // later class — "22 call sites, 18 of which would drop from 48px to + // 32/36/40/42px". That figure did not survive re-measurement (#270), and the + // omission is no longer justified: + // + // - There are ZERO same-variant tap/numeric pairs in production source. The + // 84 survivors the old scan counted are cross-variant responsive + // step-downs (`min-h-tap` with `sm:min-h-9`, `h-10.5` with `sm:h-tap`), + // and tailwind-merge groups by variant, so declaring `tap` cannot reach + // them. They are live breakpoint steps, not dead classes — deleting them + // would RAISE those controls at their breakpoint. + // - The named call sites were stale: DocumentManagerPanel and + // settings-dialog carry no tap token at all, and document-admin and + // service-detail-page pair theirs with no numeric height. + // - A per-string-literal scan cannot see a conflict composed across `cn()` + // arguments — `cn(recipe, "min-h-tap")` pairs the recipe's `min-h-7` with + // the token. A composition-aware sweep that resolves constant recipe + // identifiers at each of the 1418 `cn()` call sites finds zero + // same-variant pairs in either direction. It was mutation-tested against + // synthetic `cn("h-tap", "h-4")` and `cn("min-h-tap", recipe)` probes, + // both of which it flags, so that zero is a measurement rather than a + // pattern that never matches. + // + // AGENTS.md and SPEC §4.10 remain explicit that no production target is ever + // reduced. That rule is unchanged; what changed is the evidence that declaring + // `tap` would reduce one. Re-measure before introducing a same-variant pair: + // order decides the outcome, and tap-then-numeric is the forbidden direction. spacing: [ "icon-xs", "icon-sm", @@ -86,6 +101,7 @@ export const CLINICAL_TWMERGE_THEME = { "mode-home-composer-wide", "safe", "safe-2", + "tap", ], // globals.css @theme --ease-*. diff --git a/tests/tailwind-merge-config.test.ts b/tests/tailwind-merge-config.test.ts index 2eb4665eb..160cd9a79 100644 --- a/tests/tailwind-merge-config.test.ts +++ b/tests/tailwind-merge-config.test.ts @@ -106,15 +106,25 @@ describe("custom @theme scales are not misclassified", () => { }); }); -describe("--spacing-tap is deliberately NOT merged", () => { - // Tailwind emits `.min-h-tap` after every numeric `.min-h-*`, so the tap token - // wins today at the 22 call sites that pair them. Declaring `tap` in the merge - // config would hand the win to the numeric class and drop 18 production targets - // below 48px. If someone adds `tap` to `theme.spacing`, this fails. - it("passes min-h-tap through beside a smaller numeric min-h", () => { - expect(cn("min-h-tap", "min-h-9")).toBe("min-h-tap min-h-9"); - expect(cn("min-h-tap", "min-h-8")).toBe("min-h-tap min-h-8"); - expect(cn("h-tap", "h-10.5")).toBe("h-tap h-10.5"); +describe("--spacing-tap merges, and only within a variant", () => { + // `tap` was pinned out of the merge config on the premise that declaring it + // would drop 18 production targets below 48px across 22 call sites. That did + // not survive re-measurement (#270): a composition-aware sweep resolving + // constant recipe identifiers at all 1418 `cn()` call sites finds ZERO + // same-variant tap/numeric pairs, so there is nothing for the merge to lower. + it("merges a same-variant pair, last class winning", () => { + expect(cn("min-h-9", "min-h-tap")).toBe("min-h-tap"); + expect(cn("h-10.5", "h-tap")).toBe("h-tap"); + }); + + // The load-bearing half. The 84 surviving pairs in production are all + // cross-variant responsive step-downs, and tailwind-merge groups by variant, + // so they must pass through untouched. If this ever merges, a control loses + // its breakpoint step and the sweep's zero stops meaning anything. + it("leaves cross-variant responsive step-downs alone", () => { + expect(cn("min-h-tap", "sm:min-h-9")).toBe("min-h-tap sm:min-h-9"); + expect(cn("h-10.5", "sm:h-tap")).toBe("h-10.5 sm:h-tap"); + expect(cn("min-h-tap", "lg:min-h-9")).toBe("min-h-tap lg:min-h-9"); }); }); @@ -148,13 +158,13 @@ describe("the config tracks globals.css", () => { }, ); - it("declares every --spacing-* token except the deliberately-held tap knob", () => { + it("declares every --spacing-* token, including the tap knob", () => { const fromCss = declared("spacing"); expect(fromCss).toContain("tap"); expect([...CLINICAL_TWMERGE_THEME.spacing].sort()).toEqual( // `safe` / `safe-2` come from @utility rules, not @theme, so they are // config-only and are added to the CSS-derived list for the comparison. - [...fromCss.filter((name) => name !== "tap"), "safe", "safe-2"].sort(), + [...fromCss, "safe", "safe-2"].sort(), ); });