refactor(ui): type Mosaic render callbacks from MosaicComponentProps - #9302
Conversation
MosaicComponentProps omitted the native color from a component's own props but still typed render with the headless primitive's props, so a render callback handed back a color?: string that collided with a color variant. Every call site worked around it by destructuring the prop away. Type render from MosaicElementProps so the omission is inherited everywhere, and derive Menu's trigger props and Dialog's trigger from MosaicComponentProps rather than re-deriving them from the headless types. Widening back to the primitive's props stays safe, so the callbacks remain assignable to headless.
🦋 Changeset detectedLatest commit: 716a4ee The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/electron
@clerk/electron-passkeys
@clerk/eslint-plugin
@clerk/expo
@clerk/expo-google-signin
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
API Changes Report
Summary
No API Changes DetectedAll packages have stable APIs with no detected changes. Report generated by Break Check Last ran on |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR adds a shared Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
# Conflicts: # packages/ui/src/mosaic/components/menu/menu.tsx # packages/ui/src/mosaic/props.ts
…entProps Tabs and Dialog took their parts' props straight from the headless primitives, so their render callbacks still handed back the native color?: string that collides with a color variant. Add MosaicPartProps to re-expose a headless part's props the Mosaic way: the part's own additions kept, color and render swapped for Mosaic's. Item redeclared render as RenderProp<React.HTMLAttributes<HTMLElement>> after omitting the inherited one. Nothing depended on the widening: the inherited render types the link case its docblock and tests already cover, so drop the override.
The inherited render pins ref to the default tag's HTMLDivElement, which a callback cannot spread onto the <a> or <button> a row renders. Restore the override and say why it exists, so the next reader doesn't take it for redundant.
`RenderPropOrElement<Tag>` pinned a `render` callback's props to the part's default tag, so every Mosaic component worked around the same two problems: a `ref` that would not spread onto a different element, and the non-standard `color` attribute colliding with a `color` variant. Splits the two contracts at the source. `ComponentProps<Tag>` stays pinned to the tag for the component's own props; the new `RenderProps` is tag-agnostic, matching Base UI's `HTMLProps`. Both drop `color`. Deletes the workarounds this made unnecessary: `MosaicPartProps`, Item's redeclared `render`, box's hand-rolled render prop, and the last `color` destructure in swingset.
Description
MosaicComponentPropsomitted the nativecolorfrom a component's own props, but still typedrenderwith the headless primitive's props:So a render callback received a
color?: stringthat collided with a component'scolorvariant, and each call site worked around it by destructuring the prop away before spreading:renderis now typed from the same color-omitted props the component itself accepts, so the omission is inherited everywhere instead of being reapplied per call site. Widening back to the primitive's props stays safe (dropping an optional prop leaves the callback assignable), so these still compose with the headless parts, which continue to mirror Base UI.Menu's trigger props andDialog'striggernow derive fromMosaicComponentPropsrather than re-deriving from the headless types, which is what routed them around the contract in the first place. Both workarounds are removed.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change