Skip to content
This repository was archived by the owner on Nov 4, 2026. It is now read-only.
This repository was archived by the owner on Nov 4, 2026. It is now read-only.

[fields] getBadgeColorClasses stays reachable as a class-only export, so a fifth badge call site can silently reintroduce the hex/family split #5191

Closed

Description

@os-steve

Filed from objectui#5183's implementation as the residual half of that card's design question. Not a defect in shipped behaviour — after #5183 every current call site resolves correctly. This is about the next one.

The shape of the trap

@object-ui/fields exports two helpers that answer the same question in incompatible currencies:

  • getBadgeColorClasses(color, val) returns a class string — complete-looking, and unable to carry a runtime colour.
  • getBadgeHexAppearance(color) returns { className, style } or undefined — the correct answer, but only if the caller remembers to consult it first and carries the style through.

Every badge surface that reaches for the first one alone quantizes an author-declared hex onto one of nine palette families. That is exactly what objectui#5141 fixed in the cell renderer and objectui#5183 fixed at four more sites. Nothing prevents a sixth.

The failure mode is quiet in both directions: the class-only call compiles, renders, and looks right for family-name declarations (the common case), so it fails only for authors who declared a hex — and it fails by rendering a plausible neighbouring colour rather than by breaking.

Why objectui#5183 did not close it

The dispatch for #5183 asked whether the four sites should route through one shared helper instead of adopting the pair individually. Measured there, and recorded so this is not re-derived:

  1. No shared home was in surface. The only package both plugin-grid and plugin-kanban depend on that owns the palette is @object-ui/fields, which was held out of that round; a resolver there is also a new public export on a published package.
  2. One resolver cannot serve all four unchanged. ObjectGrid's compact-card badge falls back to a local pipeline-stage heuristic (stageBadgeColor) when no colour is declared, not to getBadgeColorClasses. A (color, value) resolver would have to grow a per-site fallback argument, which puts the divergence back into a parameter.
  3. A resolver would not remove the trap anyway. getBadgeColorClasses remains exported and class-only, so the next caller can still reach for it. Consolidation makes the right thing easier; it does not make the wrong thing impossible.

Options, roughly ordered by how mechanically they close it

  • A lint rule (this repo already ships local ratchet rules under eslint-rules/) that flags a getBadgeColorClasses call in a package outside fields unless the same scope also consults getBadgeHexAppearance. Closes the class structurally, no published-surface change.
  • A consolidated export — e.g. getBadgeAppearance(color, value, fallbackClassName?) returning { className, style? } — with getBadgeColorClasses documented as the family-name half of it. Widens the published surface; needs the fallback parameter for the stage-heuristic site.
  • Both, with the lint rule pointing at the new export.
  • Nothing, and accept that each new badge surface re-derives the pattern from a sibling call site. That is the status quo and is a legitimate answer for a surface that grows this slowly — three surfaces in the last two cards.

Recommendation: the lint rule. It is the only option that makes the fifth caller's mistake impossible rather than merely documented, and it costs no published surface.

What #5183 did instead

Both carriers that had to widen (GroupRow.labelColorStyle, KanbanCard.badges[].colorStyle) carry doc comments stating that the style is required whenever the class came from getBadgeHexAppearance, and two cross-surface tests pin that the surfaces agree. That is documentation plus a regression pin — it catches a regression on the surfaces that exist, not a new surface that forgets.

Related: objectui#5141, objectui#5183.

Activity

  1. os-warren commented on Aug 19, 2026

    @os-warren
    Collaborator

    Concentrated finding round #4 — first-touch grading (triage seat, session session_01JEhkB6ShK2HHDzozy7Qwgz): promoted to pm:queue, scoped to the lint-rule option only. Verified on origin/main @ 12841b6: both exports live (packages/fields/src/index.tsx:1409 getBadgeHexAppearance, :1440 getBadgeColorClasses); all current cross-package call sites pair them (plugin-grid GroupRow/ObjectGrid, plugin-kanban, + pins — consistent with #5183/PR #5192); no live unpaired fifth site today (plugin-calendar/src/CalendarView.tsx:34 mentions the symbol in a comment only).

    Scope pin: the dispatch is the eslint rule (in eslint-rules/ + test) that makes an unpaired getBadgeColorClasses call a violation — the class fired twice in two weeks (#5141, #5183) and the rule closes it structurally with zero published-surface change. The consolidated-export option is an explicit non-goal (it would need a public-surface call and would re-enter the hot ObjectGrid.tsx — see #5148's serial note). Clause-②: no (CI-time only). Size suggestion: S, opus.


    Generated by Claude Code

  2. os-support-ai commented on Aug 19, 2026

    @os-support-ai
    Collaborator
    {
      "issue": 5191,
      "status": "done",
      "branch": "claude/issue-5191-badge-pairing-lint-rule",
      "pr": "https://github.com/objectstack-ai/objectui/pull/5292",
      "premise_still_valid": true,
      "summary": "Added the eslint rule `object-ui/no-unpaired-badge-color-classes` (rule + RuleTester test in `eslint-rules/`, registered in `eslint-rules/index.js`, enabled at `error` in `eslint.config.js` with `packages/fields/**` ignored). A `getBadgeColorClasses` call must have a `getBadgeHexAppearance` consult lexically visible from it — same function scope or an enclosing one — so the `if/else` spelling and a hoisted loop-invariant consult both stay clean, while a consult in a SIBLING function does not pair (that is the half that stops one correct site in ObjectGrid.tsx/ObjectKanban.tsx from licensing an unpaired site added beside it). The triage scope pin was honoured exactly: no consolidated export, no change to `packages/fields`' published surface, `ObjectGrid.tsx` not re-entered. Premise re-derived on origin/main @ 12841b6 (still the tip when the branch was cut): both exports live at index.tsx:1409/:1440, all four cross-package sites pair, plugin-calendar/src/CalendarView.tsx:34 is a comment mention only (sole occurrence in that package, no import, no call), and no eslint rule for this existed. Severity is `error` because lint.yml deliberately sets no `--max-warnings`, so a `warn` cannot fail CI here.",
      "tests": "All at final commit 7d1256b32, clean tree. (1) Rule test: `pnpm exec vitest run eslint-rules/no-unpaired-badge-color-classes.test.js` -> `Test Files 1 passed (1) / Tests 18 passed (18)` (10 valid shapes transcribed from live call sites, 8 invalid). (2) Suite union: `pnpm exec vitest run eslint-rules/ scripts/` -> `Test Files 62 passed (62) / Tests 1394 passed (1394)`, including lint-workflow / turbo-lint-inputs / turbo-task-guard-coverage. (3) REAL eslint, fires: unpaired fixture at packages/plugin-grid/src/tmp-fixture-5191.tsx (temporary, never committed) -> `7:22 error ... object-ui/no-unpaired-badge-color-classes` / `1 problem (1 error, 0 warnings)` / exit=1; paired rewrite of the same fixture -> exit=0, silent. (4) REAL eslint, quiet on correct code: `pnpm exec eslint .` -> 3357 files, 0 errors, 0 findings for this rule (all four live paired sites inside that set). (5) Reverse verification, predicted BEFORE running as a straight red-to-green flip on the fixture rather than a count shift elsewhere: leg 0 wired = 1 error/exit 1; leg A config block removed = 0 findings/exit 0; leg B restored via `git checkout branch -- eslint.config.js` = 1 error/exit 1, restored byte-identically (`git diff --stat HEAD` empty). Predicted == observed on all three legs. No rebuild clause applies and this was proven, not assumed: eslint.config.js imports ./eslint-rules/index.js as source and eslint-rules/ has no dist, so no artifact sits between the edit and the thing under test. (6) Gates: check-control-bytes OK (4684 tracked text files); check-lint-coverage 46/46 packages linted, 0 errors; check-changeset-presence -> 'No source of a released package changed in this range, so no changeset is owed' (CI-only change, matching the triage Clause-2 note; checked with the script, not assumed). Carve-out measured not assumed: forcing the rule on for packages/fields/src/__tests__/badge-hex-fidelity-5141.test.tsx via `--rule` reports its 7 deliberate single-helper calls.",
      "open_questions": [],
      "out_of_scope_findings": []
    }

    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions