Skip to content

finding(plugin-kanban): resolveDisplay's isLookup guard is unreachable — the next line returns on the same condition unconditionally #6063

Description

@yinlianghui

Measured while implementing #5874 (converging this face's private copy of the reference-bearing field family). Out of that card's fence — that card converges a forked table, this is a redundant guard, a different defect class — so it is filed rather than fixed in passing.

What is there

resolveDisplay in packages/plugin-kanban/src/ObjectKanban.tsx (the card-description helper, inside the legacy semantic heuristic) ends:

constisLookup=isExpandableFieldType(def);// was an inline disjunction before #5874if(isLookup&&isOpaqueId(raw))returnundefined;if(isOpaqueId(raw))returnundefined;returnraw;

The second line subsumes the first for every input: whenever the first return fires, the second would have fired too. So isLookup cannot change any outcome of this function — it is computed, branched on, and discarded.

Why it matters rather than being a tidy-up

It cost #5874 a real verification hole, and hid it in the shape that reads like success.

The guard also reads as live capability: it names the reference-bearing rule and appears to suppress un-expanded foreign-key ids from card descriptions, which is a real thing the function does — just not through this line.

Deliberately NOT this issue

Fix — needs a decision, not just a deletion

Two readings, and the choice is behavioural, so it should be made deliberately rather than by whoever tidies first:

  • A. The guard is redundant — delete it.isOpaqueId(raw) already suppresses every opaque-looking id regardless of field type. Removes the dead branch and the @object-ui/core read on this path. Cost: the file stops declaring that relation columns are the reason ids get suppressed here.
  • B. The unconditional line is the mistake — narrow it. If the intent was that only relation columns suppress opaque ids (a non-relation text column holding an id-shaped string would then still render), the second return is over-broad and the guard was meant to gate it. This changes what cards show, so it wants a read of the original intent rather than an inference from the code.

Related: #5874, #5692, #3463 (two earlier dead-code sites in this same file).

Metadata

Metadata

Assignees

Labels

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

Type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions