Skip to content

finding(console): Approvals Inbox declares its row cells inside the page component, so every render remounts them #5348

Description

@os-support-ai

Observation-class finding, recorded while implementing #5211 (PR #5347). Filed unassigned, no pm:queue — for triage to grade. Not fixed there: out of that card's scope, and it is a behavioural/perf question rather than the affordance that card was about.

What I noticed

apps/console/src/pages/system/ApprovalsInboxPage.tsx declares RequestCell, RecordCell and InlineActions as function components insideApprovalsInboxPage's own body. React identifies components by reference, so each render produces a new component type and React unmounts and remounts those subtrees rather than updating them — every row, every render. The page re-renders often by construction: a minute clock (setNow), search typing, drawer open/close, and (as of #5347) the readability probe resolving.

How it showed up, measured

A test clicked a row's inline Approve button and nothing happened — no confirmation dialog, no state change, no error. The button was present, enabled, and correctly scoped to its row; document.body.children.length stayed at 1 after the click.

The cause is the remount: userEvent.click() spans several ticks (pointerdown → mouseup → click, with awaits between), a re-render lands in the middle, the captured node is replaced by a fresh one, and the rest of the pointer sequence is delivered to a detached node. The click is swallowed silently — the failure surfaces only as "the thing I clicked did nothing".

Workaround used in that PR (documented at the call site): query and click in one synchronous step with fireEvent. That makes the test pass but does not address the page.

Why it may be worth grading rather than shrugging at

  • The same node churn is what a real user's interaction is exposed to, not only a test's. A click landing between a re-render and its replacement node is a genuine (if narrow) dropped-input window, and the minute clock guarantees a re-render every 60s regardless of user activity.
  • Remounting discards subtree state — focus, hover, any transient state inside those cells.
  • It is a per-row cost on every render of a list page.
  • The fix is mechanical (hoist the three components to module scope and pass what they need as props, or memoize them), but it is a real diff on a 2000-line page and wants its own verification — which is exactly why it is not a rider on fix(console): suppress the Approvals Inbox record link when the viewer cannot read the target #5347.

Scope check

Only ApprovalsInboxPage.tsx was examined. Whether other console pages do the same was not surveyed — treat that as unknown, not as zero.

Metadata

Metadata

Assignees

No one assigned

    Type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions