Review 5363

Review 5363 — Table: a plugin can suppress a body cell's content

#5363 · ours (carries @ernestt's diagnosis) · merged Verdict approve — posted as comment, GitHub refuses self-approval Supersedes #5317, closed with credit.

Problem

A grouped table crashes as soon as a cell renderer keys a lookup off a field: renderCell: item => <StatusDot status={STATE_META[item.state].color} />. useTableGroupedRows injects synthetic header rows, and BaseTable evaluates every column's renderCell against every row before transformBodyRow can discard a header's cells. The header Proxy answering '' only rescued a renderer that prints a field; '' is no more a member of that map than undefined.

Solution

BodyCellRenderProps.isContentSuppressed?: boolean. A plugin sets it in transformBodyCell; BaseTable honors it before the renderer, so neither renderCell nor the default renderer runs. transformColumns + its WeakMap are deleted.

Why not #5317's shape

#5317 wrapped columns in transformColumns, which only sees the columns that exist when it runs. Verified by applying it to main: {{rowStatus, grouped}} passes, {{grouped, rowStatus}} still throws — and that second order is what the repo's own table lab uses. Plugin order decided correctness; here it cannot.

API

+ BodyCellRenderProps.isContentSuppressed?: boolean   (public, @astryxdesign/core/Table)

Only new public surface. An earlier revision also returned isGroupHeader from the hook; it was pitched as a nice-to-have with no user journey and was removed before merge (R1g-surface) in b8c00b1, −65 lines.

Theme targets · Breaking · Performance

No targets. API additive · Visual none (three grouped stories byte-identical, same sha256, same rendered HTML) · Theme none. One predicate per cell; real rows get the same props object back — strictly less work than main on header rows. The invariant transformColumns' WeakMap protected is locked instead by useTableGroupedRows-perf.test.tsx, asserting render counts.

Judgement

approve — no blocking findings.

1. Both plugin orders verified by running them: on main all three cases throw, on the branch all three pass.
2. R3d same-shape probe came back a different shape — grouped + rowExpansion with a lookup-keyed getIsItemExpandable still throws, but inside applyPlugins' try/catch, so it degrades to a dev error rather than a blank page. Uncaught consumer code reached with a synthetic row is exactly col.renderCell, and that is now closed.
3. Pre-existing, not this PR's: Table.perf.test.tsx asserts milliseconds (< 500, < 100) against our own "render counts, never milliseconds" rule.

What this review cost us to learn

Two findings landed on it before the loop ever ran, because we wrote it ourselves and skipped our own process:

• shipped with no visual evidence for a crash fix → R15e
• carried unnecessary public surface → the "nice to have is a confession" clause of R1g-surface

Our own PRs go through the loop.