fix(types): DashboardRenderer and ListView serve their declared props — the index signature stops erasing them (#4528) - #4551
Merged
Conversation
… — the index signature stops erasing them (#4528) Both components declared a full props interface and neither was enforced. A `[key: string]: any` on `DashboardRendererProps` and `ListViewProps` puts `string` into `keyof Props`, so `'ref' extends keyof Props` is always true, React's `PropsWithoutRef` takes its `Omit` branch, and `Omit` over a type carrying a string index signature keeps only the index signature. Every declared property was erased on both sides: the render function received `{ [x: string]: any }` (so even `schema` was `any` inside the component) and every JSX call site was unchecked. Measured on the pre-fix source: `keyof ComponentProps<typeof DashboardRenderer>` was `string | number` and `...['onWidgetClick']` was `any`, while the interface declared `(widgetId: string | null) => void`. plugin-list, asserted by inspection in the card, measured identically for `onRowClick`. Type-only: the emitted JS for both packages is byte-identical before and after (sha256 on dist/index.js and dist/index.umd.cjs), and both runtime suites are untouched and green (87 files, 939 tests). Props each component genuinely reads but never declared are now declared by name at the type each lands on. DashboardRenderer's DOM pass-through keys are derived from `toDomProps`' whitelist constant so declaration and runtime filter cannot drift. Three latent defects the erasure hid, each surfaced by the repo-wide canary: DashboardWithConfig typed its handler `(widgetId: string)` while the renderer calls `onWidgetClick(null)` to deselect; InterfaceListPage built a schema whose `viewType` was a bare `string`; StudioDesignSurface forwarded a `refreshKey` that nothing in the chain declares or reads. Per-package structural guards pin the shape, covering the public `forwardRef` that takes its props whole — the spelling #4438's schema-destructuring scan could not see. Refs #4422, #4438, #4426, #4040. Findings filed: #4548, #4549, #4550.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
Collaborator
Author
|
PM step-7 复核 — ACCEPT (session_017Qqyix2QcnpUC9XeYVDzx3)
Auto-merge armed (squash) — landing verified per the merge-queue discipline. Generated by Claude Code Generated by Claude Code |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4528
Sweeps the two packages #4422 / PR #4438 left unswept. Both components declared a full props interface and neither was enforced.
Both survivors measured first
The card measured plugin-dashboard and asserted plugin-list by inspection. Both were probed on the pre-fix source before anything was edited, through each package's own
tsconfig.test.json. They are identical — plugin-list's by-inspection claim is now measured:Reading those: the interface declares real keys, the resolved call-site
keyofisstring | number, the named prop readsany, and the interface still declares the real signature. Declaration right, nobody held to it. The probes were throwaway and are not in this PR; the permanent pins are the two*.propsResolution.test.tsfiles.The fix
The index signature is removed from both declared interfaces, so
PropsWithoutReftakes its identity branch instead of itsOmitbranch. Visible directly in the shipped.d.ts— note the spaces after each<, which GitHub's body sanitizer requires:That
Omit< …, "ref" >is the erasure itself, materialized in the published artifact:Omitover a type carrying a string index signature keeps only the index signature.The runtime pass-through is unchanged: each render function's parameter carries the signature instead (PR #4438's remedy), so
...propsstill collects arbitrary keys.Props each component genuinely reads but never declared are now declared by name, at the type each lands on —
dataSourceon both, plusonAddRecord/onBulkAction/onPageSizeChange/onEdit/onDelete/onBulkDeleteonListView.DashboardRenderer's DOM pass-through keys are derived fromtoDomProps' whitelist constant itself, so declaration and runtime filter cannot drift — the direction@object-ui/core'sdom-propsdoctrine already asks for: "Deliberate DOM pass-through beyond this set stays available the objectui#4435 way — DECLARE it and forward it by name. Do not reopen the spread."Type-only, measured rather than asserted
The emitted JS is byte-identical before and after:
plugin-dashboard/dist/index.jsd31056cf85ae…plugin-dashboard/dist/index.umd.cjscf7ba45fa822…plugin-list/dist/index.js8a8c3d017a32…plugin-list/dist/index.umd.cjs738e2a86cbac…Both packages' runtime suites are untouched: 87 files, 939 tests, all green.
Canary: the full repo-wide type-check
Baseline on
origin/mainwas 80/80. After the fix it named exactly three latent defects the erasure had been hiding, and is 80/80 again with them fixed:DashboardWithConfigtyped its handler(widgetId: string)whileDashboardRenderercallsonWidgetClick(null)to deselect on a design-mode background click. The state behind it is alreadyuseState< string | null >, so widening the annotation is what the contract always said.InterfaceListPagebuilt a list schema whoseviewTypewas a barestring(allowedVisualizationsarrives asstring[]), never checked againstListViewSchema.StudioDesignSurfaceforwardedrefreshKeytoListView, which no component in the chain declares or reads — it rode the{...props}forward and was dropped. Removed (behaviour-preserving); wiring it is a behaviour change, filed as Studio Data pillar ignores itsrenderListViewslot'srefreshKey— the prop was forwarded to a component that never declared it #4549.No consumer relies on arbitrary-prop passthrough as a feature: every in-repo call site passes specific named props, no README or doc endorses the passthrough, and the one
< ListView objectName= fields= navigation= />inapps/consolelives inside a template-literal string compiled at runtime by thekind:'react'tier, so the type system never saw it and cannot affect it.Guards, with the discrimination proof
#4438's ratchet resolves its scan root as
packages/components/srcand structurally could not see either survivor. Per-package siblings are added, with a scope wider than the original's: the original judges onlyforwardRefcalls whose render function destructuresschema, which misses the publicDashboardRenderer— it takes(props, ref)whole, and is precisely the call-site half this card measured.Run against
origin/main's shape (fix removed via patch file, restored and sha256-verified), they go red on both assertions, and the dashboard guard names both sites including the one the original's heuristic cannot see:Against this branch: 8 passed (8). The type-level pins fail the same way on the pre-fix shape (6 assertions red in plugin-dashboard, 10 in plugin-list).
#4528 direction 3 — one guard over every package
src— is deliberately NOT done here and is blocked, not skipped: a repo-wide widening goes red onpackages/react/src/SchemaRenderer.tsx, which is outside this card's surface. Both guard headers say so and point at the finding.Grading: minor, not major
Quoting the ruling on #4528, which the changeset also carries:
Findings filed
Record< string, any >instead of[key: string]: any#4548 —SchemaRenderercarries this same erasure spelledRecord< string, any >, which every sweep's grep and both guards' syntactic detector are blind to. Blocks direction 3.renderListViewslot'srefreshKey— the prop was forwarded to a component that never declared it #4549 — Studio Data pillar ignores itsrenderListViewslot'srefreshKey.NavigationConfigre-declaresmodeas required whileuseNavigationOverlayitself defaults it — every spec-typed caller has to assert past the alias #4550 —NavigationConfigrequiresmodewhileuseNavigationOverlayitself defaults it, forcing spec-typed callers to assert past the alias.Verification
turbo run type-check(full, as CI runs it): 80/80, baseline-matchingtsc -p tsconfig.jsonand-p tsconfig.test.json: cleanvitest run packages/plugin-dashboard/ packages/plugin-list/: 87 files, 939 tests greenvitest runapp-shell consumer suites: 22 files, 143 tests greenturbo run linton the three touched packages: 0 errorscheck-control-bytes/check-changeset-presence/check-changeset-no-major/check-changeset-fixed/check-phantom-dependencies: all greenLeft as draft for PM step-7 review.
Generated by Claude Code