refactor(components): the action keys publish UIActionSchema, and every forwardRef renderer annotates its props (#4418, #4422) - #4438
Merged
yinlianghui merged 2 commits intoAug 12, 2026
Conversation
…renderers annotate their props (#4418, #4422) The three action schema interfaces plus ActionButtonProps/ActionIconProps migrate their action keys from the @deprecated legacy ActionSchema (crud.ts) to UIActionSchema (ui-action.ts) — the type the implementations were already written against. All fifteen schema-reading forwardRef renderers annotate their render function's first parameter directly, with the pass-through index signature on the annotation rather than on the forwardRef type argument, so PropsWithoutRef no longer collapses the declared props to a bare index signature. A structural guard pins both halves. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
…2-action-typing-integrity
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
Collaborator
Author
|
ACCEPT — PM 复核 (session
Flipping ready + arming auto-merge. 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 #4418
Fixes #4422
Two halves of one defect, on one branch because they share the same files and the same mechanism: the
forwardRefprops collapse (#4422) is why the action renderers could declare the deprecatedActionSchemawhile being written againstUIActionSchema(#4418) for as long as they did. Fixing the collapse turns that drift into a hard compiler error, so neither half lands alone.Red-first: the probe, both directions
The in-situ test #4422 specified,
const probe: null = schema;inaction-bar.tsx. UnderstrictNullChecksonlyanyaccepts it.tsc --noEmitexit 0, zero diagnostics — the holeerror TS2322: Type 'ActionBarSchema' is not assignable to type 'null'.exit 2Note the polarity: the healthy state is the one that raises an error. A green probe is the defect.
Swept mechanically across all 15 sites (probe inserted,
tscrun, file restored, one at a time):13 renderers recover a real declared type. The last two accept the probe honestly — their props type declares
schema: any(one factory over every raw HTML tag / seven semantic tags, none of which has a schema type). Nothing was erased there, so nothing is recovered; they are annotated anyway so the guard needs no per-file carve-out.#4422 — the shape, and why direction 1 needed one more move than the card said
The card's direction 1 is "annotate the render function's parameter directly in addition to the type argument". Measured: that does not compile.
The compiler states the mechanism itself:
Omithas already erasedschema, so an annotation that requiresschemais contravariantly incompatible with the collapsed parameter. The two halves have to move together. The shape that works, and that this PR applies uniformly:The index signature moves off the type argument and onto the parameter annotation. Both halves load-bearing:
PropsWithoutRefhas nothing to collapse and the declared props survive;...propsstill collects arbitrary keys for the DOM / Shadcn hand-off — unchanged spread, unchanged runtime.This is not the deferred direction 2. No component's real prop surface is enumerated and no pass-through is removed; the signature is relocated, not deleted. It is consumer-neutral, measured rather than assumed: none of the 15 renderer consts is exported, all 15 have 0 JSX call sites repo-wide, and
Registry.registertakesComponentRenderer< T = any > = T. The type argument's index signature had exactly one observable effect — the collapse.#4418 — the ref inventory, measured at the branch point
Re-measured on
origin/main@306c10136rather than taken from the card. The card's "~46 refs across 12 files" is the loose substring count (it also matchesUIActionSchema, which #4417 had just introduced); at my branch point that reads 54 occurrences / 12 files. The migration surface is the word-boundary count: 25 bareActionSchemareferences across 10 files, of which 11 are type positions and 14 are prose.action/action-bar.tsxactions,systemActionsaction/action-menu.tsxactionsaction/action-group.tsxactionsaction/action-button.tsxActionButtonProps.schemaaction/action-icon.tsxActionIconProps.schemaaction/index.tslayout/containers.tsx@objectstack/spec'sActionSchema.visibleand avariantnote — a different subject)action-button.tsxandaction-icon.tsxare the siblings the sweep found: the card's scope list missed them because their index signature and theirActionSchemaboth hide behind a named interface (ActionButtonProps/ActionIconProps) rather than an inline type. Same defect, same mechanism, same package, and they hold 4 of the 11 type positions — so the guard's matcher pins that spelling explicitly (test case 2).Per-ref runtime-acceptance check
The ruling's stop condition: if migration would change runtime acceptance of working metadata, stop that ref and report. Measured per ref rather than assumed.
The question reduces to a measurement, because the six migrated declarations have no type-checked consumer outside their own file. A repo-wide sweep for
ActionBarSchema|ActionMenuSchema|ActionGroupSchema|ActionButtonProps|ActionIconPropsreturns 12 hits, every one inside the declaring file (its owninterface, its ownforwardRef, and comments). None is re-exported frompackages/components/src/index.ts. Every production construction site —app-shell'sRecordDetailView/EnvironmentListToolbar/ObjectView,plugin-detail'sDetailView,core'spublic-blocks— builds a plain object and hands it toSchemaRenderer, whoseschemaprop isany, so no authoring site is judged by these types in either direction.ActionBarSchema.actionsActionBarSchema.systemActionsActionMenuSchema.actionsActionGroupSchema.actionsActionButtonProps.schemaactionType(the one legacy-shaped key the renderer reads) is kept on the intersectionActionIconProps.schemaRefs stopped: none. A repo-wide sweep for the legacy literal
type: 'action'finds 28 sites and not one of them feeds these keys — they are@object-ui/types' owncrud.tstests, andNavigationItem-shaped nav entries inlayout/app-shell(a different schema that happens to share the string). Examples and apps fixtures were included in the sweep.The direction of the incompatibility is worth recording, because it is not one of #4417's four proofs. Those were modern-value-into-legacy-annotation. This one is the reverse — legacy-declaration-into-modern-annotation — and it lands on
name:UIActionSchemarequiresname; legacy inherits it as optional fromBaseSchema. That is the tightening, and it reaches no authoring site for the reason measured above.The guard
packages/components/src/__tests__/forwardref-props-annotation.guard.test.ts— ratchet style, modelled onapp-shell/src/no-component-any-cast.ratchet.test.ts(the repo's structural-pin convention). It walkspackages/components/srcproduction sources with the real TypeScript AST rather than a regex (typescriptis already a declared devDependency of the package, socheck:phantom-depsstays green), and judges everyforwardRefwhose render function reads aschemaprop on two independent clauses:Plus two anti-vacuity tests: the scan must find the population (floor 12; 15 today), and the matcher must detect the shapes it bans — compiled in memory, no fixture files — pinning the exact pre-fix inline shape, the named-interface spelling that made the card undercount by two, an alias-plus-intersection spelling, the compliant shape reading as compliant, and a number index signature correctly not firing (it does not put
stringintokeyof, so it does not trigger the collapse).Scope limits are stated in the file rather than implied: production sources only,
schema-readingforwardRefs only, and index signatures detected syntactically (inline type, or a type/interface declared in the same file) — a props type imported from another module is out of a source scan's reach and is not claimed to be covered.Reverse verification — direction predicted before each run
Method:
git checkout/ scratch edit, nevergit stash.A. Revert one file's annotation. Predicted:
basic/div.tsxgoes red on both clauses, no other file moves, and the probe there starts passing again. Measured exactly that —2 failed | 2 passed, both failures naming onlyrenderers/basic/div.tsx:41:and with the probe re-inserted into the reverted file,
tsc --noEmitexit 0 —schemaisanyagain. Restored.B. Violate clause 2 only. Put
[key: string]: anyback onaction-menu.tsx's type argument while keeping the annotation. Predicted: clause 2 red naming that file alone, clause 1 stays green, andtscred with TS2345. Measured exactly that —1 failed | 3 passed,+ "renderers/action/action-menu.tsx:178", and:Restored; tree clean,
tscexit 0, guard 4/4 green.Together these cover both halves of the trap: A is the compiler-invisible case (guard red,
tscgreen — exactly the state this package shipped in), B is the compiler-visible one. A guard that only caught B would not have caught the bug that was actually there.Verification
pnpm exec vitest run packages/components/ --maxWorkers=2(repo root) — 122 files / 1082 tests passed, exit 0. Baseline was 121 / 1078; this adds one file with four tests.tsc --noEmitandtsc -p tsconfig.test.jsonfor@object-ui/components— both exit 0, before and after theorigin/mainmerge. Dependency closure built first (pnpm --workspace-concurrency=2 --filter '@object-ui/components^...' build) — the suffix^...form, i.e. the packages components depends on, since a staledist/*.d.tslies in both directions.turbo run type-check --concurrency=2— 78 successful, 78 total. This is the load-bearing no-downstream-red proof, and it is the downstream direction by construction: turbo runs every package, so it subsumes--filter '...@object-ui/components'(the prefix/consumers form) rather than the suffix/dependencies one.eslinton the 16 changed files — 0 errors. Warnings 81 → 83 (+2), measured against the same files at the branch point in a scratch worktree.check-control-bytes(4144 files) /check-phantom-dependencies(40 packages, 12759 specifiers) /check-action-forward-parity/check-changeset-presence/check-changeset-no-major— all green. Plus a direct control-byte scan of the changed files: clean.Lint delta, honestly
+2, and all of it is
layout/semantic.tsx(2 → 4). Every other one of the 15 files is net zero — the index signature moved rather than multiplied.semantic.tsxwasforwardRef< HTMLElement, any >: a single bareanycovering everything. Writing the shape out as{ schema: any; className?: string }plus the pass-through annotation spells that sameanythree times where the blanket spelled it once. Theanys are not new — the honesty is. Reporting it rather than suppressing it, per #4417's precedent.Changeset
minor, per the ruling and the #4403 precedent, with the breaking semantics written out in the body — six exported declarations change the action type they name, and the two types are not interchangeable in either direction. Nevermajor:.changeset/config.json'sfixedgroup means anymajorwould push all 39 packages off@objectstack's major, whichcheck-changeset-no-major.mjsenforces mechanically.patchwould have been wrong here in the way #4417'spatchwas right: that PR changed no exported declaration's shape, this one changes six.Scope
packages/components/**plus the changeset, nothing else.packages/plugin-chatbot,packages/plugin-grid,packages/plugin-dashboardandpackages/fieldsuntouched;@object-ui/typesread-only and untouched — the migration consumesUIActionSchemafrom it and needed no types-side change.origin/mainadvanced twice during the work (#4429/#4430, then #4424/#4436); merged in, neither touchescomponents,types,coreorreact, and the full suite was re-run after the merge.Deviations from the dispatch, all measured
forwardRefs touched in total. The card's file list was a grep artifact: it matched inline[key: string]: anyinside theforwardReftype argument and so missedaction-button.tsxandaction-icon.tsx, whose identical defect hides behind a named props interface. Both sit inside finding(components): the action renderers declare the deprecatedActionSchemabut are written againstUIActionSchema#4418's own migration set, so leaving them would have shipped a guard with a known hole on day one.html-elements.tsxandsemantic.tsxare annotated too — no type is recovered there (schemais genuinelyany), but it keeps the guard carve-out-free.forwardReftype argument. Direction 1 as literally worded does not compile (TS2345, quoted above). This is the minimum additional move that makes it compile, and it is not deferred direction 2 — see the measurement under finding(components):forwardRef+ a props index signature silently erases every declared prop type — 11 renderers affected #4422 above.Out of scope
No new findings to file. The two follow-ups this PR deliberately does not take are already the card's own deferred directions 2 and 3 (dropping index signatures per-component / a shared props helper), which interact with the #4425 whitelist question.
Generated by Claude Code