fix(react): SchemaRenderer states its real contract — typed schema, deliberate forwarding, no accidental erasure (#4548) - #4578
Merged
Conversation
…eliberate forwarding, no accidental erasure (#4548) `SchemaRenderer` handed `forwardRef` a props type of `{ schema: SchemaNode } & Record<string, any>`. A string index signature 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 prop was erased. Measured on the pre-fix source: `keyof ComponentProps<typeof SchemaRenderer>` was `string` and `ComponentProps<typeof SchemaRenderer>['schema']` was `any`, while the type argument went on declaring `SchemaNode`. `<SchemaRenderer />` with no schema at all, `schema={12345}`, and arbitrary misspelled props each type-checked in silence. The forwarding surface is KEPT, deliberately: this is the renderer loop, it forwards unread props to the component the schema names at runtime, and the package README documents that. The two halves are separated instead — the forwardRef type argument is the honest `SchemaRendererProps` (nothing for `PropsWithoutRef` to collapse), and the open surface is stated once in an explicit export annotation, which nothing routes through `Omit`. `SchemaRendererProps.schema` is declared as `BaseSchema | string | null | undefined` — what the component actually handles — replacing `@object-ui/core`'s `SchemaNode` interface, which required `type: string` and contradicted the component's own early returns for strings and nullish. One declared behaviour change: a non-object, non-string primitive schema now renders as its own text instead of falling through to `{ ...schema }`, which spread a primitive to an empty object and surfaced the red "Unknown component type: undefined" box. Latent defects the erasure hid, fixed at their call sites: DashboardRenderer's `Record<string, any>` cast dropped `type`; DashboardGridLayout's inferred union admitted a typeless shape; ReportViewer handed a section's `content` ARRAY to the renderer whole. A repo-wide structural guard replaces the two per-package siblings' blocked direction, with a detector that resolves `Record<string, ...>` and `string`-keyed mapped types — the spelling both shipped guards went blind on. It judges the forwardRef type argument only, never export annotations. Refs #4422, #4438, #4528, #4551.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
yinlianghui
marked this pull request as ready for review
August 13, 2026 13:19
This was referenced Aug 13, 2026
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 #4548
SchemaRendereris the renderer loop — every registered SDUI component is rendered through it, and it is the thing that hands widgets their props. Its own props were erased.Measured first, on the pre-fix source
Probed through
packages/react/tsconfig.test.jsonbefore anything was edited (the probe was throwaway; the permanent pin isSchemaRenderer.propsResolution.test.ts):Declaration right, nobody held to it. The other half is the same defect seen from the call site — these three compiled silently, which is the measurement:
forwardRef< T, P >routesPthroughPropsWithoutRef, which is'ref' extends keyof Props ? Omit< Props, 'ref' > : Props. A string index signature putsstringintokeyof Props, so theOmitbranch always runs, andOmitover a type carrying a string index signature keeps only the index signature. SpelledRecord< string, any >rather than[key: string]: any, which is why every previous sweep's grep and both shipped guards' detector reported this site as clean.The fix keeps the forwarding surface, deliberately
This component forwards every prop it does not read to the component the schema names, resolved at runtime from a plugin-extensible registry.
packages/react/README.mddocuments exactly that —< SchemaRenderer schema={formSchema} onSubmit={handleSubmit} / >— and@object-ui/components' form renderer destructures thatonSubmitas a React prop and invokes it. Closing the surface would state a false contract and force every leaf plugin's props into this package.So the two halves are separated:
SchemaRendererProps, with no index signature — nothing forPropsWithoutRefto collapse; andOmit, so it widens without erasing.Visible in the shipped
.d.ts(built both ways from a cleandist, spaces after each<for the sanitizer):That
Omit< …, "ref" >is the erasure itself, materialised in the published artifact.The declared input, and one declared behaviour change
SchemaRendererProps.schemaisBaseSchema | string | null | undefined— what this component actually handles. It previously declared@object-ui/core'sSchemaNodeinterface, which requirestype: stringand so contradicted the component's own early returns for strings and nullish, while every caller held@object-ui/types' wider union. The erasure hid that mismatch completely.A non-object, non-string primitive now renders as its own text. It previously fell through to
{ ...schema }, which spreads a primitive to an empty object, lost thetypethe renderer then looked up, and surfaced the red "Unknown component type: undefined" box — an accident of the spread, not a decision. The declared type still excludesnumber/booleanso no author is invited to pass them; the runtime handling is defence-in-depth. Strings,null,undefined,0andfalserender exactly as before, and an object naming an unregistered type still gets the error box — all pinned inSchemaRenderer.primitiveSchema.test.tsx.Canary: the full repo-wide type-check
Baseline on the unmodified worktree was 80/80. The fix is 80/80 again, with these latent defects — every one hidden by the erasure — fixed at their call sites:
DashboardRenderercast its widget schemaas Record< string, any >, dropping thetypethat every branch ofgetComponentSchemaactually sets.DashboardGridLayout's equivalent inferred a union admitting a shape with notype(the passthrough fallback spreads aDashboardWidgetSchema, whosetypeis optional), narrowed once at the definition site.ReportViewerhanded a section'scontentarray to the renderer whole — an array has notype, so a multi-node section rendered the unknown-component box instead of its content. Arrays are mapped, not widened into the declared input.Forwarding sites that hold
@object-ui/types' widerSchemaNodego throughtoRenderableSchema, a total function rather than a cast: it maps the two primitive members onto their text form, which is precisely what the renderer's own defensive branch does, so it changes no behaviour.No consumer relies on arbitrary passthrough in a way this breaks: the surface is kept, and the canary reports zero excess-prop errors.
Guard: repo-wide, with the Record-aware detector
scripts/__tests__/forwardref-props-erasure.guard.test.tsreplaces the two per-package siblings' blocked direction 3. Measured across everypackages/*src: 219 forwardRef sites own their props type, 1 carried the erasure, now 0.Discrimination proof, run against the pre-fix shape (fix removed via
git checkout, restored and sha256-verified byte-identical):Against this branch: 3 passed (3). The detector resolves
Record< string, … >by name plus its first type argument, and any mapped type keyed bystring, in addition to literal index signatures — the gap this card documents.It judges the type argument only, never an export annotation. That distinction is the claim, not an exemption: an index signature on the type argument is an accidental eraser (it is fed to
PropsWithoutRef), whereas one in an export annotation is a stated contract applied to the already-built component, where nothing collapses. The fixedSchemaRenderertherefore passes on the merits — there is no allowlist.#4551's second assertion (every destructuring forwardRef annotates its parameter) stays per-package and is deliberately not lifted: repo-wide it names 200 sites, overwhelmingly the shadcn/ui primitives in
packages/components/src/ui/*plusplugin-timeline. None is erased — their type arguments carry no index signature, soPropsWithoutReftakes its identity branch. Sweeping it would turn 200 non-defects red and bury the one assertion that names a defect. Recorded in the guard header.Grading
@object-ui/reactminor, on #4528's reasoning quoted in the changeset: the type argument has always declaredschema; the index signature erased it from the resolved type, and restoring what the declaration documents is a fix to the published contract, not a break. Consumers patch, per their own diffs. Never major.must-not-change
Bundle byte-identity does not apply here and is not claimed: the
SchemaNodesource and the new primitive guard change the emitted JS. It is replaced by behaviour pins — strings / nullish /0/falserender identically, unknown-type objects keep the error box, and the touched packages' runtime suites are green and untouched.Verification
All re-run after rebasing onto current
main(eb7f586b6), sincepackages/typesandpackages/coreboth moved under this branch:turbo run type-check(full, as CI runs it): 80 successful, 80 total — baseline-matchingtscpasses per package viatype-check@object-ui/react: 41 files, 566 tests green (all of them), plus the repo-wide guardcomponents,plugin-dashboard,plugin-detail,plugin-report,plugin-view): 273 files, 2475 tests greencheck-control-bytes,check-changeset-presence/-no-major/-fixed,check-phantom-dependencies: greengrep -naP) over every created and edited file, including untracked: cleanLeft as draft for PM step-7 review.
Generated by Claude Code