Skip to content

finding(components): three near-identical menu-item recursions in renderers/overlay/ — the same icon bug has now been repaired in two of them separately #6327

Description

@os-support-ai

Observation recorded while repairing #6278. Filed unassigned; no fix attempted, and deliberately notpm:queue — see the last section.

What was measured, at ad54f7587

packages/components/src/renderers/overlay/ contains three separate recursive menu-item renderers over the same item shape (MenuItem from packages/types/src/overlay.ts, or an any[] standing in for it):

filehelperarms
dropdown-menu.tsxrenderMenuItemsleaf, submenu trigger (recursive)
context-menu.tsxrenderContextMenuItemsleaf, submenu trigger (recursive)
menubar.tsxinline items.mapleaf, submenu trigger, nested child (one level, non-recursive)

The first two differ only in the imported component names and in how a shortcut is drawn. The duplication is acknowledged in the source itself — context-menu.tsx carries the comment:

// Reuse helper for recursive menu items if I could share it, but for now duplicate concise logic

The cost, as actually paid

The same defect — an item's authored icon not reaching a glyph — has now been diagnosed and repaired once per copy, in sequence:

Each copy also had to be censused separately in scripts/check-lucide-icon-record-names.mjs, and menubar cannot be censused at all today because nothing authors the key.

So this is not a tidiness observation: it is a measured pattern of one bug costing three diagnoses, three repairs and three census entries, with the divergence risk that the three arms' behaviour now drifts independently.

Why this is filed as an observation, not queued

Two reasons, both arguing against acting on it now:

  1. It has no pull yet. Nothing is blocked on the duplication; the concrete defect it produced is already answered for two of the three files. Under the startup-scope principle (core capability first, no speculative refactors), a shared-helper extraction that serves only future hypothetical bugs should wait for a real one.
  2. It overlaps work already queued.Consolidate the eight lucide record-reading icon resolvers into one seam — three tokenisers and a 5-of-8 alias map mean the same authored name renders on one surface and not another #5935 consolidates the eight lucide icon resolvers into one seam and would rewrite the resolution half of these renderers anyway. Extracting a shared menu-item helper before that lands would mean touching the same lines twice, in the wrong order.

The natural moment to reconsider is after #5935 lands, or the next time a third defect has to be fixed once per copy. Recording it now so that moment is recognisable rather than rediscovered.

⚠️ Any consolidation must preserve what #5930 ruled: the icon resolves through resolveIcon (the lucide RECORD surface), notLazyIcon, which degrades an unknown name to the Database glyph and trades a no-icon failure for a wrong-icon one.

Metadata

Metadata

Assignees

No one assigned

    Labels

    domain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seatpm:on-hold

    Type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions