feat(ui): Add mosaic design systems foundations#8755
Open
alexcarpenter wants to merge 18 commits into
Open
Conversation
🦋 Changeset detectedLatest commit: 761ad3a The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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.
|
Contributor
|
Linter diff in the way? Review this PR in Change Stack to focus on meaningful changes and expand context only when needed. Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Mosaic design-system foundations: frozen token defaults and resolver, MosaicProvider with Emotion cache and optional CSS layer wrapping, a theme-aware cva utility with tests, a reference Button component, architecture documentation, and a release changeset. ChangesMosaic Design System Implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/expo
@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: |
alexcarpenter
force-pushed
the
carp/mosaic-infra
branch
from
486b377 to
26eecbd
Compare
Contributor
|
Break Check: no API changes detected across the tracked packages. Last ran on |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
packages/ui/src/mosaic/__tests__/cva.test.ts (1)
255-282: ⚡ Quick winAdd an
sxmerge test to cover a keycvaexecution path.This suite is strong, but it doesn’t currently assert
sxoverride/deep-merge behavior, which is part ofcvaruntime composition.Suggested additional test
+ it('applies sx overrides after variant and compound resolution', () => { + const styles = cva({ + base: { color: 'black', '&:hover': { color: 'blue' } }, + variants: { + size: { + sm: { fontSize: 12, '&:hover': { textDecoration: 'underline' } }, + }, + }, + }); + + const res = styles({ + size: 'sm', + sx: { color: 'red', '&:hover': { color: 'green' } }, + })(mockTheme); + + expect(res).toEqual({ + color: 'red', + fontSize: 12, + '&:hover': { color: 'green', textDecoration: 'underline' }, + }); + });As per coding guidelines: "Implement comprehensive testing including unit, integration, and E2E tests" and "Verify proper error handling and edge cases."
🤖 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 `@packages/ui/src/mosaic/__tests__/cva.test.ts` around lines 255 - 282, Add a unit test that asserts cva's `sx` deep-merge and override behavior: define a `styles` via `cva(...)` (or the existing theme-function variant) that sets `base` and a `size` variant, then call the returned function with `{ size: 'sm', sx: { ... } }` and pass `mockTheme`; assert the final result merges `sx` over base/variant values (i.e., `sx` overrides same keys and deeply merges nested objects like `padding`, `color`, or `fontSize`). Use the existing test pattern (symbols: `cva`, `styles`, `mockTheme`, `sx`) and add a new `it` block verifying the merged output matches expected object shape.
🤖 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 `@packages/ui/src/mosaic/__tests__/cva.test.ts`:
- Line 309: The test line uses an unsafe cast (`as any`) when calling styles;
remove `as any` and instead cast via `unknown` and narrow to the expected props
shape or use a typed helper variable so the call to styles receives the
correctly typed props (e.g., declare the test input as unknown, perform a
narrow/type assertion to the styles props type, then call
styles(input)(mockTheme)). Update the test around the styles(...) invocation
(and its mockTheme usage) to use the narrowed/typed value rather than `any`.
In `@packages/ui/src/mosaic/cva.ts`:
- Around line 70-73: The applyBase function currently mutates nested objects
from base by using Object.assign; change it to deep-clone the base StyleRule
before merging so nested objects are not shared—inside applyBase (function
applyBase(target: StyleRule, base?: StyleRule)) create a deep copy of base
(e.g., structuredClone(base) or a safe cloneDeep utility/fallback) and then
merge/assign that clone into target instead of assigning base directly to
prevent variant merges from mutating shared nested objects.
In `@packages/ui/src/mosaic/MosaicProvider.tsx`:
- Line 10: The module reads document at top-level (const el =
document.querySelector(...)) causing SSR/SSG crashes; update MosaicProvider.tsx
to guard all DOM access (the top-level const el and the logic around the style
insertion at the block covering lines ~27-29) by either wrapping DOM queries in
a typeof document !== 'undefined' check or moving them into a React effect
(e.g., useEffect inside the MosaicProvider component) so they only run in the
browser; ensure the unique identifiers (the variable el and the style insertion
logic in MosaicProvider) are the only places changed and preserve existing
behavior when running in the browser.
In `@packages/ui/src/mosaic/variables.ts`:
- Around line 54-56: The loop over overrides prematurely stops because it uses
break when encountering dangerous keys; change the logic in the for (const k in
overrides) loop so that it skips unsafe properties (e.g., '__proto__',
'constructor', 'prototype') with continue instead of break and only processes
own properties (use Object.prototype.hasOwnProperty.call(overrides, k)) before
reading const value = overrides[k]; this preserves processing of subsequent
valid override keys.
In `@references/mosaic-architecture.md`:
- Around line 18-26: The public architecture doc references outdated Mosaic
API/file names and token shapes: replace mentions of
packages/ui/src/mosaic/tokens.ts and parseVariables with
packages/ui/src/mosaic/variables.ts and resolveVariables(...), update the token
examples to use the actual runtime theme types (e.g., spacing as a function
accessed via theme.spacing(n) instead of theme.spacing.md), and adjust the
MosaicTheme example to match typeof defaultMosaicTokens and the current
defaultMosaicTokens shape; update all occurrences (lines referenced:
18/34/49/93/99/261/264/267) so examples and migration notes reflect the real
symbols resolveVariables, defaultMosaicTokens, and the spacing(n) API.
- Around line 220-228: The fenced code block starting with triple backticks in
the mosaic-architecture.md snippet lacks a language tag (violates MD040); update
the opening fence to include a language identifier (e.g., "text" or "markdown")
so the block becomes ```text and keep the existing content unchanged; ensure the
modified block surrounds the lines containing "StyleCacheProvider ...
MosaicComponent → css={styles({ intent: 'primary' })(theme)}" to satisfy the
linter.
---
Nitpick comments:
In `@packages/ui/src/mosaic/__tests__/cva.test.ts`:
- Around line 255-282: Add a unit test that asserts cva's `sx` deep-merge and
override behavior: define a `styles` via `cva(...)` (or the existing
theme-function variant) that sets `base` and a `size` variant, then call the
returned function with `{ size: 'sm', sx: { ... } }` and pass `mockTheme`;
assert the final result merges `sx` over base/variant values (i.e., `sx`
overrides same keys and deeply merges nested objects like `padding`, `color`, or
`fontSize`). Use the existing test pattern (symbols: `cva`, `styles`,
`mockTheme`, `sx`) and add a new `it` block verifying the merged output matches
expected object shape.
🪄 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), Repository UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: c13bf295-8dcc-4dfe-a8a5-d9780ead1a94
📒 Files selected for processing (7)
AGENTS.mdpackages/ui/src/mosaic/Button.tsxpackages/ui/src/mosaic/MosaicProvider.tsxpackages/ui/src/mosaic/__tests__/cva.test.tspackages/ui/src/mosaic/cva.tspackages/ui/src/mosaic/variables.tsreferences/mosaic-architecture.md
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
1 participant
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.
Description
cva(),MosaicProviderwith theme context,sxprop for one-off overridesspacing(n),alpha(color, %),mix(a, b, %),text(size)— all return literal CSS values, spreadable into style objectsButtonreference component demonstrating variant composition viacva()Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
Documentation
Tests
Chores