Skip to content

finding(plugin-dashboard, plugin-list): the two packages #4422 left unswept still erase every declared prop — and the #4438 guard cannot see them #4528

Description

@yinlianghui

Observation-class finding, surfaced while wiring @object-ui/plugin-dashboard's tests into tsc for #4040 (closing tranche). Nothing a user meets today; no fix proposed here.

#4422 measured this exact hole in packages/components and closed it via PR #4438. That issue ended with an explicit remainder:

Not swept beyond this package. The pattern is not package-specific, so other packages' forwardRef renderers are worth the same grep before anyone sizes a fix.

This is that grep. Two survivors.

The two files

packages/plugin-dashboard/src/DashboardRenderer.tsx   DashboardRendererProps, index signature at :189, forwardRef at :192 and :1057
packages/plugin-list/src/ListView.tsx                 ListViewProps,          index signature at :56,  forwardRef at :634

Sweep method — every non-test source under packages/*/src that mentions forwardRef and declares [key: string]: any: 18 files, of which 16 are the packages/components renderers that #4438 already fixed.

Why #4438's fix does not cover them

#4438's remedy is precise, and worth restating because it is what these two are missing. It moved the index signature OFF the forwardRef type argument and ONTO the render function's parameter annotation:

const ActionBarRenderer = forwardRef< HTMLDivElement, { schema: ActionBarSchema; className?: string } >(
  ({ schema, className, ...props }: { schema: ActionBarSchema; className?: string; [key: string]: any }, ref) => {

The type argument is now index-signature-free, so PropsWithoutRef takes its identity branch and declared props survive; the parameter annotation still carries the signature, so the spread still type-checks. Both packages here still put the signature INSIDE the type argument, so both halves of the erasure are open.

Measured, in the direction #4422 did not state

#4422 measured the erasure the render function sees. There is a second one, at every CALL SITE, and it is the one this card is really about: ForwardRefExoticComponent's public props also come through PropsWithoutRef, so a consumer writing JSX gets no checking at all.

Measured on DashboardRenderer with a throwaway probe compiled by the new packages/plugin-dashboard/tsconfig.test.json (deliberate errors, so the compiler prints the resolved types):

error TS2322: Type 'keyof DashboardRendererProps' is not assignable to type 'never'.
error TS2322: Type 'string | number' is not assignable to type 'never'.
error TS2322: Type 'any' is not assignable to type 'never'.
error TS2322: Type '((widgetId: string | null) => void) | undefined' is not assignable to type 'never'.

Reading those four in order:

  • the interface itself still declares real keys;
  • keyof React.ComponentProps< typeof DashboardRenderer > is string | number — every named prop erased;
  • React.ComponentProps< typeof DashboardRenderer >['onWidgetClick'] is any;
  • while DashboardRendererProps['onWidgetClick'] is still ((widgetId: string | null) => void) | undefined.

So the declaration is right and no consumer is held to it. The live consequence in this repo: DashboardRenderer.domProps.test.tsx wrote onWidgetClick={(id) => selections.push(id)} and id was implicitly any — a prop typo or a wrong-arity handler at any DashboardRenderer JSX call site passes today.

packages/plugin-list/src/ListView.tsx is NOT separately probed: it is asserted here by inspection to carry the identical two ingredients at the file:line above, not measured. Whoever sizes the fix should measure it the same way rather than trusting this paragraph.

The guard exists and is package-scoped

packages/components/src/__tests__/forwardref-props-annotation.guard.test.ts pins #4438's shape, and its own header says so:

SCOPE — packages/components/src, production sources only

It resolves its scan root as path.resolve(here, '..'), i.e. packages/components/src. So the ratchet that keeps the 16 fixed files fixed structurally cannot see these two, and would not have caught them arriving.

Directions, not a recommendation

  1. Apply refactor(components): the action keys publish UIActionSchema, and every forwardRef renderer annotates its props (#4418, #4422) #4438's shape to both files. Smallest diff, proven remedy, and it closes the call-site half too. ListViewProps is exported and DashboardRendererProps is exported, so moving the signature off the type argument is a public-type change worth a compile of the consumers.
  2. Drop the index signature and name the pass-through props. What commandment Fix documentation deployment for www.objectui.org #6 asks for; DashboardRenderer already destructures 14 named props before spreading the rest, so its real surface is closer to hand than components' was. Note finding(plugin-dashboard): MetricWidgetProps / MetricCardProps declare no DOM passthrough, so the id / role / aria-* the spread accepts at runtime are a type error for a TS consumer #4426 is the same package's adjacent DOM-passthrough finding and would likely be answered by the same work.
  3. Widen the guard to every packages/*/src rather than leaving one package's ratchet to protect a repo-wide pattern — otherwise the third survivor arrives the same way these two did.

Whichever is chosen, (3) is what stops this recurring: a fix without it leaves the pattern legal everywhere outside packages/components.

Refs #4422, #4438, #4426, #4040.


Generated by Claude Code

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions