feat(components): compile under noImplicitAny — type the 26 renderer signatures + 2 test sites - #4417
Merged
Conversation
`packages/components/tsconfig.json` was the only place in the workspace
relaxing a `strict` sub-flag, under a comment that explained the adjacent
`rootDir` removal rather than the flag. `tsconfig.test.json` mirrored the
one flag deliberately, so the test project could not become the compiler
of record for a source strictness decision the build config owns.
Both configs now inherit `strict: true` from the root. The flag flip
reported 26 implicitly-`any` sites in five renderer sources and 2 in the
package's own tests; all 28 are typed. Types only — no runtime change.
The sidebar entry points follow the package's measured convention (an
inline `{ schema: <X>Schema; [key: string]: any }` naming the registered
component's schema type, 21 occurrences). The ten sidebar PARTS have no
schema type of their own and take `BaseSchema`; `SidebarSchema` would
assert `type: 'sidebar'` on a `'sidebar-header'` node.
The action callbacks are typed from `UIActionSchema`, not the legacy
`ActionSchema` these files import for their declarations: the legacy
interface has no `locations` (which `actionRendersAt` requires), no
`'primary'` variant (which the #2339 tie-break compares against), and a
literal `type: 'action'` where the actions flowing through carry
`'form' | 'script' | 'url' | 'flow' | 'api' | 'modal'`. That was
unverifiable before, because `forwardRef` routes props through
`PropsWithoutRef`, whose `Omit` collapses a props type carrying
`[key: string]: any` to the bare index signature — so `schema` arrived
as `any` and every callback under it inferred `any`.
Nothing this package publishes changes shape: the three action schema
interfaces and the leaf components are not re-exported from `src/index.ts`.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
|
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 #4353
@object-ui/componentswas the only package in the workspace relaxing astrictsub-flag. This turnsnoImplicitAnyon and gives real types to every signature that depended on it being off. Types only — zero runtime change; all 1077 of the package's tests pass untouched.The stale comment above the flag (it explained the adjacent
rootDirremoval, not the flag) is gone with the flag.tsconfig.test.json's deliberate mirror of the same flag is removed too, and its inline reasoning rewritten rather than deleted: that mirror existed so a TEST project could not become the compiler of record for a SOURCE strictness decisiontsconfig.jsonowns. With the build config no longer relaxing the flag, a mirror would invert exactly that hazard — the test project would be relaxing what the source config tightened. Both projects now simply inheritstrict: truefrom the root config.Pre-fix measurement — the red
Baseline on
origin/main@0b49d6032, both commands ofpnpm type-check: 0 errors. Removing the flag from both configs, unchanged sources:src/renderers/navigation/sidebar.tsxschema+ 1classNamebinding elements)src/renderers/action/action-bar.tsxsrc/renderers/action/action-menu.tsxsrc/renderers/action/action-group.tsxsrc/renderers/data-display/tree-view.tsxsrc/__tests__/page-header-predicate-dialect.test.tsxsrc/__tests__/div-deprecation-warn-once.test.tsxThis reproduces the card's table exactly — same files, same counts,
mainmovement notwithstanding. No site indata-table.tsx, so nothing here overlaps #4354; that file is untouched by this PR.Measured convention — the sidebar entry points
The card left open whether the ten
{ schema }entry points want a real schema type orComponentRendererProps. Measured across the renderer tree:({ schema, ...props }: { schema: BadgeSchema; [key: string]: any }), withclassName?: stringspelled out whenclassNameis destructured (10 files do exactly that).ComponentRendererPropsin this package — it exists in both@object-ui/coreand@object-ui/types, and nothing here uses it.React.FC< any >on named renderer components incontainers.tsx, which types nothing and is not a model worth copying.So: the inline per-schema-type spelling, which
sidebar.tsxline 49 already uses for its one typed registration. No third spelling invented.Which schema type, though. Only
'sidebar'itself is in the registry map (@object-ui/types,registry.ts). The other ten registrations are sidebar parts —sidebar-header,sidebar-menu-button, … — and have no schema type of their own. They takeBaseSchema, the type every registered node satisfies and the one that actually declares thebody/labelkeys they read. Annotating themSidebarSchemawould asserttype: 'sidebar'on a node whose type is'sidebar-header', which is simply false and would make an honestschema.type === 'sidebar-header'test a compile error later.The action callbacks, and what typing them exposed
Typing these surfaced a real defect that the flag had been hiding, so this section is longer than the card anticipated.
Why they were untyped at all. Not an oversight at the callback. All three renderers are
forwardRefcomponents whose props type carries[key: string]: any.forwardRefroutes its props throughPropsWithoutRef, which is"ref" extends keyof Props ? Omit< Props, "ref" > : Props. An index signature putsstringinkeyof Props, so the first branch always wins, andOmitover a type with a string index signature collapses every declared property into the bare index signature.schematherefore arrived asany— proved in situ, not inferred: an addedconst probe: null = schema;raised no error, which onlyanydoes understrictNullChecks. Everyfilter/some/mapcallback below it inherited thatany.So the fix annotates each action list once, where it enters, and the chains below infer. That is 2 annotations in
action-bar.tsxcovering its 6 sites, 1 inaction-group.tsxcovering 4, and 1 inaction-menu.tsxcovering 4.Which action type. The card says "typed from
ActionDef— the type they receive at runtime". Annotating with theActionSchemathese files already import went red in four places, and every one of them says the same thing: that import is the wrong type.@object-ui/typesexports two action types —ActionSchema(fromcrud.ts, carrying its own@deprecated Use UIActionSchema for new code) andUIActionSchema(fromui-action.ts, the modern one). These renderers import the legacy one for their declarations but are written against the modern one:actionRendersAt, the shared placement predicate, takes{ locations?: readonly string[] }. LegacyActionSchemahas nolocations— TS2559, weak-type detection, in bothaction-barandaction-group.a.variant === 'primary'. Legacyvariantis'default' | 'outline' | 'ghost' | 'link'— TS2367, three times.action-groupandaction-menualready carry(action.variant as string)casts written to get around exactly this.typeis the literal'action'. The actions flowing through these renderers carry'form' | 'script' | 'url' | 'flow' | 'api' | 'modal'— TS2322 at every leaf handoff.action-bar's own documented example at the top of the file is aUIActionSchema(type: 'script').The callbacks are therefore typed from
UIActionSchema, and the internal wiring that carries those values — thecombinedOverflowmemo's type argument, the leaf components'action/onExecute/onSelectprops, the twohandleExecutecallbacks — moves with them, because a value cannot be one type in the list and another in the leaf.What deliberately did NOT move: the
actions?: ActionSchema[]keys onActionBarSchema/ActionMenuSchema/ActionGroupSchema. Reconciling those declarations with the type the implementation receives reaches ~46 references across 12 files and is a contract decision in its own right, not part of turning a compiler flag on. Filed separately (see below); the annotations name the mismatch in prose at each site so the next reader is not left guessing.Per-file typing
sidebar.tsxBaseSchemafor the ten parts,SidebarSchemauntouched on'sidebar'tree-view.tsxTreeViewSchema— already imported, and previously flagged unusedaction-bar.tsxaction-group.tsxaction-menu.tsxunknown[], the row typedeprecationCallsalready declares it returns.ReturnType< typeof vi.spyOn >erases the spied signature, somock.callsarrives asanyand the parameters had nothing to infer fromVerification
pnpm exec tsc --noEmitandtsc -p tsconfig.test.json, flag ON: both exit 0. (Both were exit 2 with 26 / 28 errors before the typings.)pnpm exec vitest run packages/components/ --maxWorkers=2: 121 files, 1077 tests, all passed, exit 0. No test added or removed, so the counts are structurally unchanged.eslinton the 7 touched source files: 0 errors (46 warnings — see below).node scripts/check-control-bytes.mjs: OK, 4125 files scanned. Plus a direct control-byte scan of the 10 changed files: clean.node scripts/check-phantom-dependencies.mjs: green.node scripts/check-changeset-presence.mjs/check-changeset-no-major.mjs: green.src/index.tsre-exports none of the changed symbols — not the three action schema interfaces, notInlineActionButton/DropdownActionItem/ActionMenuItem, not the registered renderers (which are side-effect registrations). The barrels it does export from —./ui,./custom,./notifications,./debug,./share— contain none of them. With no exported declaration moving, a downstream sweep has nothing to detect.Reverse verification
Direction predicted before running, and it is the plain red one:
noImplicitAnyjudges each binding independently, so there are no counts to move and no predicate to invert. Method was commit-then-revert viagit checkout origin/main -- FILE(nevergit stash).sidebar.tsxalone, flag still on → predicted 10 × TS7031 at lines 29, 74, 87, 100, 124, 138, 151, 174, 187, 200 and nothing elsewhere. Got exactly that, exit 2.action-menu.tsxalone → predicted 4 × TS7006 at 278,23 / 278,31 / 315,27 / 315,35. Got exactly that, exit 2.Both restored; the flag is now load-bearing rather than decorative.
Lint delta — measured, and it goes the other way
The dispatch expected the
no-explicit-anywarning count to drop. It rises: 31 to 42 (+11), and reporting that honestly matters more than the expectation.Every one of the +11 is a
[key: string]: anyindex signature in a newly added annotation — 10 insidebar.tsx, 1 intree-view.tsx— because that index signature is part of the measured convention, which exists so the registry can spread arbitrary props onto the underlying Shadcn component. The action files add zero (4 / 14 / 10, all unchanged): typing a list at its entry point introduces noanyat all. Spelling the index signatureunknowninstead would break every{...props}spread and would be the third spelling the ruling forbids.So the trade is exact and worth naming: 26 implicit anys — invisible, unbounded, and silently propagating into every downstream inference — become 11 explicit, bounded ones confined to a props index signature, while every
schemaand every action callback gains a real type. Total warnings 36 to 46, 0 errors either way.One warning also disappeared:
'TreeViewSchema' is defined but never used. The type was imported and dead; typing the renderer put it to work.Changeset
patch, per the card's rule for pure inference-tightening with unchanged exported declarations — and the published surface is verifiably unchanged, as set out under Verification. The #4403minorprecedent does not apply because no exported declaration visibly changes shape.Out of scope, filed separately
actions?: ActionSchema[]declarations vs theUIActionSchemathe implementation receives).PropsWithoutRefprops-collapse trap, which silently erases declared prop types from anyforwardRefcomponent whose props type carries an index signature — 11 files in this package alone.Generated by Claude Code