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

Activity

  1. self-assigned this
    on Aug 13, 2026
  2. yinlianghui commented on Aug 13, 2026

    @yinlianghui
    CollaboratorAuthor

    CLAIM — session_017Qqyix2QcnpUC9XeYVDzx3, branch claude/issue-4528-forwardref-prop-erasure. Dispatching a dev agent now.

    PM ruling (delegated decision authority; maintainer veto window open — record objections here):

    1. Measure both survivors first — the card states plugin-list's case is by-inspection, not measured. Reproduce the probe for BOTH (keyof ComponentProps is string|number; the declared prop reads any) before touching either; if plugin-list's shape differs, treat it on its own measurement.
    2. Fix = remove the accidental index signature so the DECLARED interface is what the type system serves. This must be a type-only change: zero runtime diff, proven by the untouched runtime suites.
    3. Repo-wide type-check is the canary: run the FULL turbo type-check. In-repo consumers that passed accidental extra props get fixed in the same PR (each pinned); if any consumer measurably RELIED on arbitrary-prop passthrough as a feature, STOP and report — that inverts the fix shape.
    4. Grading, decided here so it does not wobble: the interfaces have always DECLARED these props; the index signature erased them from the resolved type. Restoring what the interface documents is a FIX to the published contract, not a contract break — grade MINOR for both packages with this reasoning quoted in the changeset (major is forbidden under the fixed group and not warranted: no documented capability is removed; any-typed accidental passthrough was never the documented surface). If measurement shows a plausible EXTERNAL consumer pattern that the repo's own docs/README endorse relying on the passthrough, STOP and report for a re-rule instead of shipping.
    5. Guard must prove it can fail: refactor(components): the action keys publish UIActionSchema, and every forwardRef renderer annotates its props (#4418, #4422) #4438's guard is scoped to packages/components/src and structurally cannot see either survivor. Extend it (or add per-package siblings following its exact pattern) to cover these two components, with the discrimination proof run against the pre-fix shape — the [P2] objectui: every package type-checks its tests or declares why not — and guards must prove they can fail (measured: 355 errors across 19 packages) #4040 convention.
    6. Pins: both packages' runtime suites byte-stable; ComponentProps probes as new type-level tests (expect the declared signature, not any); the chore(types): plugin-dashboard's tests compile — the #4040 program closes at 40/40 #4530-added (id: string | null) annotation in DashboardRenderer.domProps.test.tsx may become redundant — leave it unless it errors.

    Mutual exclusion: #4537 (armed) owns plugin-dashboard's DatasetWidget test files + tsconfig.test.json — do not touch those; DashboardRenderer.tsx/ListView.tsx are free. ⛔ In-flight surfaces off-limits: #4271 (metadata-client.ts, PermissionMatrixEditor, celAuthoring), #4527 (ConditionBuilder, ConditionalFormattingEditor, five inspectors), #4272 (fields src, ObjectGrid date cells, ObjectGantt), #4006 (fields/plugin-editor config). If the type-check canary demands a fix inside any of those files, STOP and report rather than editing them.


    Generated by Claude Code


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions