Uh oh!
There was an error while loading. Please reload this page.
refactor(ui): share one base props type across Mosaic components - #9267
Conversation
🦋 Changeset detectedLatest commit: ae6a5ae 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.
|
71384a1 to
27b88a3Compareaecb873 to
2f669ecCompare2f669ec to
a934303CompareHeading, Text and Badge each derived their props differently: two base types (`React.ComponentPropsWithRef` vs headless `ComponentProps`), two things omitted (`color` vs `render`), the render union restated inline in one and imported in the other, and two ways of narrowing `color` — `Omit` then redeclare in the interfaces, silent intersection in the type alias. Collapse all of it into `MosaicComponentProps<Tag>`, mirroring Base UI's `BaseUIComponentProps`. `color` is omitted once, so the next component to expose it as a variant inherits the narrowing instead of rediscovering it. Type-only: all three components resolve to the same props as before.
a934303 to
ae6a5aeCompare@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/vuecommit: |
API Changes Report
Summary
No API Changes DetectedAll packages have stable APIs with no detected changes. Report generated by Break Check Last ran on |
📝 WalkthroughWalkthroughAdds the exported generic Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.changeset/mosaic-component-props.md:
- Around line 1-2: Make .changeset/mosaic-component-props.md intentional: either
add the appropriate `@clerk/ui` package entry and release note describing the
public Mosaic prop type changes, or remove the empty changeset file when this
change should not be released.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 4913f54e-d13a-4996-b8d2-87c731507673
📒 Files selected for processing (5)
.changeset/mosaic-component-props.mdpackages/ui/src/mosaic/components/badge/badge.tsxpackages/ui/src/mosaic/components/heading/heading.tsxpackages/ui/src/mosaic/components/text/text.tsxpackages/ui/src/mosaic/props.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual)clerk/dashboard(manual)clerk/accounts(manual)clerk/backoffice(manual)clerk/clerk(manual)clerk/clerk-docs(manual)clerk/cloudflare-workers(manual)
| --- | ||
| --- |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bashset -euo pipefail
echo"== changeset file =="
cat -n .changeset/mosaic-component-props.md ||trueechoecho"== changeset files =="
find .changeset -maxdepth 1 -type f -print | sort | sed -n '1,80p'echoecho"== package manifests mentioning `@clerk/ui` / packages/ui =="
rg -n '"`@clerk/ui`"|packages/ui|`@clerk/ui`' package.json packages .changeset --glob '!pnpm-lock.yaml' --glob '!node_modules/**'| sed -n '1,200p'echoecho"== recent diff stat =="
git diff --stat HEAD~1 HEAD ||trueRepository: clerk/javascript
Length of output: 26894
Make the changeset intentional.
.changeset/mosaic-component-props.md only has empty Changeset delimiters, so it does not version or document @clerk/ui. Add the appropriate package entry/release note for the public Mosaic prop type changes, or remove the no-op file if this change is intentionally excluded from the release.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.changeset/mosaic-component-props.md around lines 1 - 2, Make
.changeset/mosaic-component-props.md intentional: either add the appropriate
`@clerk/ui` package entry and release note describing the public Mosaic prop type
changes, or remove the empty changeset file when this change should not be
released.
Source: Coding guidelines
Description
Heading,TextandBadgeeach acceptrenderand each exposecoloras a variant, but all three derived their props differently: two base types (React.ComponentPropsWithRefvs headlessComponentProps), two different things omitted (colorvsrender), the render union restated inline in one and imported in the other, and two mechanisms for narrowingcolor—Omitthen redeclare in the interfaces, silent intersection in the type alias.This introduces one shared base in
mosaic/props.ts, mirroring Base UI'sBaseUIComponentProps:coloris omitted once rather than per component, so the next component to expose it as a variant inherits the narrowing instead of rediscovering the collision with the non-standard HTMLcolorattribute.Badgealso stops omittingrenderonly to re-add it.Type-only — all three components resolve to the same props as before, so no changeset body.
Stacked on
carp/mosaic-typography-stylex(#9244); merge that first.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change