Skip to content

finding(components): planDataTableRowMenu's predicate parameter is narrower than the object its only caller hands it #4354

Description

@yinlianghui

Found while type-checking @object-ui/components' tests for #4040 (tranche 4). Observation-class: no runtime symptom — the value flows through untouched either way.

What

planDataTableRowMenu (packages/components/src/renderers/complex/data-table.tsx) declares:

editPredicates?: { visibleWhen?: unknown };
deletePredicates?: { visibleWhen?: unknown };

Its only production caller passes schema.rowEditPredicates / schema.rowDeletePredicates, whose declared shape in @object-ui/types is:

rowEditPredicates?: { visibleWhen?: unknown; disabledWhen?: unknown };

and the row-menu ITEM component two functions down the same file declares the full pair (predicates?: { visibleWhen?: unknown; disabledWhen?: unknown }) because it evaluates both. So the planner's parameter is a third, narrower restatement of one authoring shape — the #3009 hand-copy family, in miniature.

It is arguably deliberate: the planner decides visibility only, so its parameter names only what it consumes. That reading is defensible; what makes it worth recording is the consequence below.

How it surfaced

data-table-row-menu-empty-guard.test.ts's case still counts an item that renders merely DISABLED passed editPredicates: { disabledWhen: 'record.frozen == true' } as a bare object literal. Excess-property checking rejects that against { visibleWhen?: unknown } — the first thing the new test project reported.

The case was also vacuous: the planner never reads disabledWhen, so the assertion { edit: true, count: 1 } holds identically for editPredicates: {}. It pinned "an object with no visibleWhen does not hide the item", which is worth pinning, but not what its name claims.

PR (tranche 4, components) annotates the fixture with DataTableSchema['rowEditPredicates'] — derived, not hand-copied — so the same value passes the way it does in production, and states in a comment which key is being ignored. The case name is left alone deliberately rather than quietly renamed.

The decision, when someone takes it

Either (a) widen the planner's parameter to the declared pair so the three restatements collapse to one, accepting that the function still reads only visibleWhen; or (b) keep it consumption-shaped and derive it — Pick< NonNullable< DataTableSchema['rowEditPredicates'] >, 'visibleWhen' > — so it cannot drift from the authoring type it is a subset of. Either beats a third hand-written copy. Also worth checking while there: whether the disabled-item counting behaviour the vacuous case was reaching for is pinned anywhere that can actually observe it.

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions