feat(ui): Mosaic field component - #9322
Conversation
🦋 Changeset detectedLatest commit: e014d4e 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: |
57aeff6 to
fd49235
Compare
ca96e1b to
a6c399c
Compare
ee83a6e to
fcbd583
Compare
fcbd583 to
374b29c
Compare
0d8bf7c to
0aede7b
Compare
0aede7b to
59c0e38
Compare
b387d17 to
e014d4e
Compare
📝 WalkthroughWalkthroughAdded the Mosaic Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
packages/ui/src/mosaic/components/field/index.ts (1)
1-2: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the new nested barrel file.
The new
packages/ui/src/mosaic/components/field/index.tsre-export file conflicts with the repository rule that prohibits barrel files because they can create circular dependencies.
packages/ui/src/mosaic/components/field/index.ts#L1-L2: remove this barrel file.packages/ui/src/mosaic/styles/index.ts#L17-L18: re-exportFieldand its prop types directly from../components/field/field.🤖 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/components/field/index.ts` around lines 1 - 2, Remove the nested barrel file packages/ui/src/mosaic/components/field/index.ts (lines 1-2). Update the exports at packages/ui/src/mosaic/styles/index.ts (lines 17-18) to re-export Field and its prop types directly from ../components/field/field.Source: Coding guidelines
🤖 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/quiet-fields-compose.md:
- Around line 1-2: Replace the empty changeset frontmatter in
quiet-fields-compose.md with an `@clerk/ui` minor release entry, and add a concise
description of the new public Field compound component API.
In `@packages/swingset/src/stories/field.component.mdx`:
- Line 36: Update the Field description near the Field.Root usage note to remove
the claim that it does not propagate semantic state, while preserving the
statements that it does not validate controls or render errors automatically and
that its parts can be used independently.
In `@packages/swingset/src/stories/field.component.stories.tsx`:
- Line 1: Add the Emotion JSX import-source pragma at the beginning of the field
component story, before the existing Field import, so the styled Mosaic story
uses Emotion's JSX transform.
In `@packages/ui/src/mosaic/components/field/field.context.tsx`:
- Around line 4-10: Propagate disabled, required, and invalid state from
Field.Root to composed controls: in
packages/ui/src/mosaic/components/field/field.context.tsx lines 4-10, add typed
state fields to FieldContextValue; in
packages/ui/src/mosaic/components/field/field.tsx lines 20-37, add the states to
FieldRootProps and pass them through FieldProvider; in
packages/ui/src/mosaic/components/input/input.tsx lines 15-39, resolve explicit
control props before context values and apply the resulting disabled, required,
and aria-invalid attributes.
- Around line 67-74: Update the field context around the useSafeLayoutEffect
registration and the input relationship handling in
packages/ui/src/mosaic/components/field/field.context.tsx:67-74 and
packages/ui/src/mosaic/components/input/input.tsx:37-39 so generated Description
and Error IDs are derived during render rather than only populated by the
effect, while preserving client-side registration and cleanup. Add an SSR
assertion covering the generated aria-describedby relationships.
In `@packages/ui/src/mosaic/components/field/field.ssr.test.tsx`:
- Around line 48-62: Update Field.Root and its associated label/control ID
handling so an explicit Input id such as custom-control is preserved in SSR
markup and used for the label for attribute. Update
packages/ui/src/mosaic/components/field/field.ssr.test.tsx lines 48-62 to expect
custom-control in both locations, and update
packages/ui/src/mosaic/components/field/field.test.tsx lines 147-176 to verify
hydration preserves the server-rendered ID without adding it afterward.
---
Nitpick comments:
In `@packages/ui/src/mosaic/components/field/index.ts`:
- Around line 1-2: Remove the nested barrel file
packages/ui/src/mosaic/components/field/index.ts (lines 1-2). Update the exports
at packages/ui/src/mosaic/styles/index.ts (lines 17-18) to re-export Field and
its prop types directly from ../components/field/field.
🪄 Autofix
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: beecab09-0ad7-4c52-8e49-9ae6062c9e24
📒 Files selected for processing (14)
.changeset/quiet-fields-compose.mdpackages/swingset/src/components/DocsViewer.tsxpackages/swingset/src/lib/registry.tspackages/swingset/src/stories/field.component.mdxpackages/swingset/src/stories/field.component.stories.tsxpackages/ui/src/mosaic/components/field/field.context.tsxpackages/ui/src/mosaic/components/field/field.ssr.test.tsxpackages/ui/src/mosaic/components/field/field.styles.tspackages/ui/src/mosaic/components/field/field.test.tsxpackages/ui/src/mosaic/components/field/field.tsxpackages/ui/src/mosaic/components/field/index.tspackages/ui/src/mosaic/components/input/input.tsxpackages/ui/src/mosaic/icons/registry.tsxpackages/ui/src/mosaic/styles/index.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)clerk/clerk-ios(auto-detected)clerk/clerk-android(auto-detected)clerk/cli(auto-detected)
| --- | ||
| --- |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Add an @clerk/ui release entry.
This PR adds the public Field API, but this changeset has no package entry. Add an @clerk/ui minor bump and describe the new Field compound component API.
Proposed fix
---
+ '`@clerk/ui`': minor
---
+
+Add Mosaic Field compound components.Based on learnings, an empty changeset is only acceptable for documentation-only or non-published changes.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| --- | |
| --- | |
| --- | |
| '`@clerk/ui`': minor | |
| --- | |
| Add Mosaic Field compound components. |
🤖 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/quiet-fields-compose.md around lines 1 - 2, Replace the empty
changeset frontmatter in quiet-fields-compose.md with an `@clerk/ui` minor release
entry, and add a concise description of the new public Field compound component
API.
Source: Learnings
|
|
||
| Explicit `id`, `htmlFor`, `aria-labelledby`, and `aria-describedby` values remain supported. Field preserves explicit IDs after hydration and merges external ARIA references with its generated relationships. During server rendering, Field emits its generated control ID and native label relationship; explicit control IDs and generated label and message ARIA references finalize during hydration. `name` still identifies the submitted form value and is typically what form libraries use for registration. | ||
|
|
||
| Field does not validate controls, propagate semantic state, or render errors automatically. Its parts may also be used independently without `Field.Root`. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Correct the Field state contract.
Field does propagate invalid, disabled, and required state to composed Mosaic inputs. Keep the statement that Field does not validate controls or render errors automatically, but remove the claim that it does not propagate semantic state.
🤖 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/swingset/src/stories/field.component.mdx` at line 36, Update the
Field description near the Field.Root usage note to remove the claim that it
does not propagate semantic state, while preserving the statements that it does
not validate controls or render errors automatically and that its parts can be
used independently.
Source: Coding guidelines
| @@ -0,0 +1,35 @@ | |||
| import { Field } from '@clerk/ui/mosaic/components/field'; | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add the required Emotion JSX pragma.
This styled Mosaic story must begin with /** @jsxImportSource @emotion/react */.
Proposed fix
+/** `@jsxImportSource` `@emotion/react` */
import { Field } from '`@clerk/ui/mosaic/components/field`';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { Field } from '@clerk/ui/mosaic/components/field'; | |
| /** `@jsxImportSource` `@emotion/react` */ | |
| import { Field } from '`@clerk/ui/mosaic/components/field`'; |
🤖 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/swingset/src/stories/field.component.stories.tsx` at line 1, Add the
Emotion JSX import-source pragma at the beginning of the field component story,
before the existing Field import, so the styled Mosaic story uses Emotion's JSX
transform.
Source: Coding guidelines
| interface FieldContextValue { | ||
| controlId: string; | ||
| labelIds: string[]; | ||
| messageIds: string[]; | ||
| registerControlId: (source: symbol, id: string | null | undefined) => void; | ||
| setLabelIds: React.Dispatch<React.SetStateAction<string[]>>; | ||
| setMessageIds: React.Dispatch<React.SetStateAction<string[]>>; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Propagate shared field state to the control.
FieldContextValue contains only association IDs. Field.Root accepts only div props. Input reads only association props. Therefore, disabled, required, and invalid state cannot propagate from Field.Root to the composed control.
packages/ui/src/mosaic/components/field/field.context.tsx#L4-L10: add typed disabled, required, and invalid state to the context value.packages/ui/src/mosaic/components/field/field.tsx#L20-L37: add these states toFieldRootPropsand provide them throughFieldProvider.packages/ui/src/mosaic/components/input/input.tsx#L15-L39: resolve explicit control props before context defaults, and apply the resulting disabled, required, andaria-invalidvalues.
📍 Affects 3 files
packages/ui/src/mosaic/components/field/field.context.tsx#L4-L10(this comment)packages/ui/src/mosaic/components/field/field.tsx#L20-L37packages/ui/src/mosaic/components/input/input.tsx#L15-L39
🤖 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/components/field/field.context.tsx` around lines 4 -
10, Propagate disabled, required, and invalid state from Field.Root to composed
controls: in packages/ui/src/mosaic/components/field/field.context.tsx lines
4-10, add typed state fields to FieldContextValue; in
packages/ui/src/mosaic/components/field/field.tsx lines 20-37, add the states to
FieldRootProps and pass them through FieldProvider; in
packages/ui/src/mosaic/components/input/input.tsx lines 15-39, resolve explicit
control props before context values and apply the resulting disabled, required,
and aria-invalid attributes.
| useSafeLayoutEffect(() => { | ||
| if (!id || !setIds) { | ||
| return undefined; | ||
| } | ||
|
|
||
| setIds(ids => (ids.includes(id) ? ids : [...ids, id])); | ||
| return () => setIds(ids => ids.filter(value => value !== id)); | ||
| }, [id, setIds]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- candidate files ---'
fd -i 'field|input|mosaic|ssr|server' packages/ui/src packages/ui/test packages/ui/tests 2>/dev/null | head -200 || true
printf '%s\n' '--- relevant symbols ---'
rg -n -C 5 \
'useSafeLayoutEffect|messageIds|aria-describedby|aria-labelledby|Field\.Description|Field\.Error|renderToString' \
packages/ui/src packages/ui/test packages/ui/tests 2>/dev/null | head -500 || true
printf '%s\n' '--- field context outline ---'
if [ -f packages/ui/src/mosaic/components/field/field.context.tsx ]; then
ast-grep outline packages/ui/src/mosaic/components/field/field.context.tsx
cat -n packages/ui/src/mosaic/components/field/field.context.tsx | sed -n '1,180p'
fi
printf '%s\n' '--- input outline ---'
if [ -f packages/ui/src/mosaic/components/input/input.tsx ]; then
ast-grep outline packages/ui/src/mosaic/components/input/input.tsx
cat -n packages/ui/src/mosaic/components/input/input.tsx | sed -n '1,140p'
fiRepository: clerk/javascript
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- field context ---'
cat -n packages/ui/src/mosaic/components/field/field.context.tsx | sed -n '1,140p'
printf '%s\n' '--- field implementation ---'
cat -n packages/ui/src/mosaic/components/field/field.tsx | sed -n '1,280p'
printf '%s\n' '--- complete SSR test ---'
cat -n packages/ui/src/mosaic/components/field/field.ssr.test.tsx | sed -n '1,180p'
printf '%s\n' '--- relevant hydration and input tests ---'
cat -n packages/ui/src/mosaic/components/field/field.test.tsx | sed -n '1,230p'
cat -n packages/ui/src/mosaic/components/input/input.test.tsx | sed -n '1,220p'
printf '%s\n' '--- package scripts and diff summary ---'
if [ -f packages/ui/package.json ]; then
sed -n '1,180p' packages/ui/package.json
fi
git diff --stat
git status --shortRepository: clerk/javascript
Length of output: 31861
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from dataclasses import dataclass, field
`@dataclass`
class FieldContext:
message_ids: list[str] = field(default_factory=list)
def register_after_render(self, value: str) -> None:
if value and value not in self.message_ids:
self.message_ids.append(value)
def merge_ids(*values: str | None) -> str | None:
ids: list[str] = []
for value in values:
if value:
for item in value.split():
if item and item not in ids:
ids.append(item)
return " ".join(ids) if ids else None
# Model the relevant React SSR order:
# children render first, Input reads the provider's initial state, and effects do not run.
ctx = FieldContext()
description_id = "cl-field-description"
error_id = "cl-field-error"
server_aria = merge_ids("external-description", *ctx.message_ids)
assert server_aria == "external-description"
assert description_id not in server_aria
assert error_id not in server_aria
# Model the client effect phase after the same tree commits.
ctx.register_after_render(description_id)
ctx.register_after_render(error_id)
client_aria = merge_ids("external-description", *ctx.message_ids)
assert client_aria == "external-description cl-field-description cl-field-error"
print("SSR initial render omits generated message IDs; post-effect render includes both IDs.")
PYRepository: clerk/javascript
Length of output: 296
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from dataclasses import dataclass, field
from typing import Optional
`@dataclass`
class FieldContext:
message_ids: list = field(default_factory=list)
def register_after_render(self, value: str) -> None:
if value and value not in self.message_ids:
self.message_ids.append(value)
def merge_ids(*values: Optional[str]) -> Optional[str]:
ids = []
for value in values:
if value:
for item in value.split():
if item and item not in ids:
ids.append(item)
return " ".join(ids) if ids else None
# Initial server render: Input reads the provider state before effects run.
ctx = FieldContext()
description_id = "cl-field-description"
error_id = "cl-field-error"
server_aria = merge_ids("external-description", *ctx.message_ids)
assert server_aria == "external-description"
assert description_id not in server_aria
assert error_id not in server_aria
# Post-commit effect phase: message parts register their IDs.
ctx.register_after_render(description_id)
ctx.register_after_render(error_id)
client_aria = merge_ids("external-description", *ctx.message_ids)
assert client_aria == "external-description cl-field-description cl-field-error"
print("SSR initial render omits generated message IDs; post-effect render includes both IDs.")
PYRepository: clerk/javascript
Length of output: 240
Generate automatic ARIA relationships during server rendering.
messageIds is populated only in useSafeLayoutEffect, so SSR omits generated aria-describedby values for Field.Description and Field.Error. Derive these relationships during render and add an SSR assertion.
📍 Affects 2 files
packages/ui/src/mosaic/components/field/field.context.tsx#L67-L74(this comment)packages/ui/src/mosaic/components/input/input.tsx#L37-L39
🤖 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/components/field/field.context.tsx` around lines 67 -
74, Update the field context around the useSafeLayoutEffect registration and the
input relationship handling in
packages/ui/src/mosaic/components/field/field.context.tsx:67-74 and
packages/ui/src/mosaic/components/input/input.tsx:37-39 so generated Description
and Error IDs are derived during render rather than only populated by the
effect, while preserving client-side registration and cleanup. Add an SSR
assertion covering the generated aria-describedby relationships.
| it('defers an explicit control ID until hydration', () => { | ||
| const html = renderToString( | ||
| <Field.Root> | ||
| <Field.Label>Email</Field.Label> | ||
| <Input id='custom-control' /> | ||
| </Field.Root>, | ||
| ); | ||
|
|
||
| const labelControlId = html.match(/for="([^"]+)"/)?.[1]; | ||
| const inputControlId = html.match(/<input[^>]*\sid="([^"]+)"/)?.[1]; | ||
| expect(inputControlId).toBeDefined(); | ||
| expect(inputControlId).not.toBe('custom-control'); | ||
| expect(labelControlId).toBe(inputControlId); | ||
| expect(html).not.toContain('id="custom-control"'); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Preserve explicit control IDs in server markup.
The tests require Input id='custom-control' to be absent until hydration. This breaks external labels, ARIA references, URL fragments, and DOM integrations before client JavaScript runs. Keep the caller-provided ID in SSR output and make the associated Field label use that same ID.
packages/ui/src/mosaic/components/field/field.ssr.test.tsx#L48-L62: expectcustom-controlin the server HTML and on the labelforattribute.packages/ui/src/mosaic/components/field/field.test.tsx#L147-L176: verify hydration preserves the already-rendered explicit ID instead of adding it after hydration.
📍 Affects 2 files
packages/ui/src/mosaic/components/field/field.ssr.test.tsx#L48-L62(this comment)packages/ui/src/mosaic/components/field/field.test.tsx#L147-L176
🤖 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/components/field/field.ssr.test.tsx` around lines 48 -
62, Update Field.Root and its associated label/control ID handling so an
explicit Input id such as custom-control is preserved in SSR markup and used for
the label for attribute. Update
packages/ui/src/mosaic/components/field/field.ssr.test.tsx lines 48-62 to expect
custom-control in both locations, and update
packages/ui/src/mosaic/components/field/field.test.tsx lines 147-176 to verify
hydration preserves the server-rendered ID without adding it afterward.
Description
https://swingset-git-mosaic-text-field-component.clerkstage.dev/components/field
Adds the Mosaic Field component:
Field.Root,Field.Label,Field.Description, andField.ErrorAPI.aria-*attributes.alert-circleicon for validation errors.