feat(swingset): Scaffold component documentation package#8797
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Repository UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (19)
🚧 Files skipped from review as they are similar to previous changes (19)
📝 WalkthroughWalkthroughAdds the ChangesSwingset Component Explorer
Mosaic Button & CVA metadata
Reference docs fix
Sequence Diagram (high-level interaction) sequenceDiagram
participant Browser
participant NextApp
participant DocsViewer
participant Registry
participant MosaicProvider
Browser->>NextApp: navigate /components/:component or /components/:component/:story
NextApp->>DocsViewer: render MDX for component (slug)
NextApp->>Registry: findStory(componentSlug, storySlug)
Registry-->>NextApp: story module & export
NextApp->>MosaicProvider: render story with knob values / variables
MosaicProvider-->>Browser: rendered story UI
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 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. ✨ Finishing Touches📝 Generate docstrings
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 |
API Changes Report
Summary
No API Changes DetectedAll packages have stable APIs with no detected changes. Report generated by Break Check Last ran on |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
packages/swingset/src/components/ui/separator.tsx (1)
1-22: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd JSDoc for exported component.
This component is exported as a public API but lacks JSDoc documentation. Per coding guidelines, all public package APIs must be documented with JSDoc.
📝 Suggested JSDoc structure
+/** + * A visual separator component supporting horizontal and vertical orientations. + * + * `@param` {SeparatorPrimitive.Props} props - Separator props including orientation + * `@returns` {React.JSX.Element} The rendered separator element + */ function Separator({ className, orientation = 'horizontal', ...props }: SeparatorPrimitive.Props): React.JSX.Element {🤖 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/components/ui/separator.tsx` around lines 1 - 22, Add a JSDoc block above the exported Separator component describing the component and its public API: document the function name Separator, list the props (including className, orientation with default 'horizontal', and other spread props), describe the component behavior (renders a styled SeparatorPrimitive with data-slot 'separator'), and mark it as an exported public component; include types for the props (referencing SeparatorPrimitive.Props) and a short example or usage note as appropriate per package docs.Source: Coding guidelines
packages/swingset/src/components/ui/skeleton.tsx (2)
1-14:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winAdd missing React import.
The component uses
React.ComponentPropson line 3 but does not import React. This will cause a type error.🔧 Suggested fix
+import * as React from 'react'; + import { cn } from '`@/lib/utils`';🤖 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/components/ui/skeleton.tsx` around lines 1 - 14, The file uses React types (React.ComponentProps in the Skeleton component) but lacks a React import; add an import for React (e.g., import React from 'react') at the top of the file so the type reference for React.ComponentProps resolves and the Skeleton component compiles correctly.
1-14: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd JSDoc for exported component.
This component is exported as a public API but lacks JSDoc documentation. Per coding guidelines, all public package APIs must be documented with JSDoc.
📝 Suggested JSDoc structure
+/** + * A loading skeleton placeholder component with pulse animation. + * + * `@param` {React.ComponentProps<'div'>} props - Standard div element props + * `@returns` {React.JSX.Element} The rendered skeleton element + */ function Skeleton({ className, ...props }: React.ComponentProps<'div'>): React.JSX.Element {🤖 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/components/ui/skeleton.tsx` around lines 1 - 14, Add a JSDoc block above the exported Skeleton component describing its purpose and public API: document the Skeleton function (exported symbol Skeleton), its props parameter (React.ComponentProps<'div'>) including the className and spread props, mention that it renders a div with data-slot='skeleton' and the default classes ('bg-muted animate-pulse rounded-md') merged via cn, and include `@param` and `@returns` tags (and an optional `@example` showing basic usage). Ensure the JSDoc sits immediately above the function declaration so the component is documented as a public API.Source: Coding guidelines
packages/swingset/src/components/ui/label.tsx (1)
1-21: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd JSDoc for exported component.
This component is exported as a public API but lacks JSDoc documentation. Per coding guidelines, all public package APIs must be documented with JSDoc.
📝 Suggested JSDoc structure
+/** + * A form label component with accessibility and theming support. + * Automatically handles disabled states via peer/group selectors. + * + * `@param` {React.ComponentProps<'label'>} props - Standard label element props + * `@returns` {React.JSX.Element} The rendered label element + */ function Label({ className, ...props }: React.ComponentProps<'label'>): React.JSX.Element {🤖 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/components/ui/label.tsx` around lines 1 - 21, The exported React component Label lacks JSDoc; add a JSDoc block immediately above the Label function that documents the component purpose, its props (using the existing type React.ComponentProps<'label'> or `@param` {React.ComponentProps<'label'>} props), any important data- attributes (e.g., data-slot='label'), and what it returns (JSX.Element), and mark it as part of the public API; ensure the JSDoc includes a short description, `@param` for props, and `@returns` so the exported Label is documented per package guidelines.Source: Coding guidelines
packages/swingset/src/components/ui/input.tsx (1)
1-21: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd JSDoc for exported component.
This component is exported as a public API but lacks JSDoc documentation. Per coding guidelines, all public package APIs must be documented with JSDoc including purpose, parameters, and usage examples.
📝 Suggested JSDoc structure
+/** + * A styled input component wrapping Base UI's Input primitive. + * Provides consistent styling with theme-aware states (focus, disabled, invalid). + * + * `@param` {React.ComponentProps<'input'>} props - Standard input element props + * `@returns` {React.JSX.Element} The rendered input element + * + * `@example` + * ```tsx + * <Input type="email" placeholder="Enter email" /> + * ``` + */ function Input({ className, type, ...props }: React.ComponentProps<'input'>): React.JSX.Element {🤖 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/components/ui/input.tsx` around lines 1 - 21, The exported Input component lacks JSDoc; add a JSDoc block above the Input function explaining its purpose, parameters (props like className, type and other input props), return type, and a short usage example (e.g., <Input type="email" placeholder="Enter email" />); ensure the JSDoc references the Input component name and mentions it is exported, so the public API is properly documented for consumers and tooling.Source: Coding guidelines
packages/swingset/src/components/ui/sidebar.tsx (1)
1-690: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftSplit large sidebar file into smaller modules.
This file is 690 lines, significantly exceeding the 150-200 line guideline for component files. The sidebar system includes 25+ exported components and should be split into logical modules for better maintainability.
Consider this structure:
sidebar-context.tsx- Context, provider, and hooksidebar-core.tsx- Main Sidebar, Trigger, Rail, Insetsidebar-layout.tsx- Header, Footer, Content, Separator, Inputsidebar-group.tsx- Group components (Group, Label, Action, Content)sidebar-menu.tsx- Menu components (Menu, MenuItem, Button, Action, Badge, Skeleton)sidebar-submenu.tsx- Submenu components (Sub, SubItem, SubButton)sidebar-variants.ts- CVA variantsindex.ts- Re-export all 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 `@packages/swingset/src/components/ui/sidebar.tsx` around lines 1 - 690, The Sidebar file is too large and should be split into smaller modules: extract the context, provider, and hook (SidebarContext, SidebarProvider, useSidebar) into sidebar-context.tsx; move core components (Sidebar, SidebarTrigger, SidebarRail, SidebarInset) into sidebar-core.tsx; put layout pieces (SidebarHeader, SidebarFooter, SidebarContent, SidebarSeparator, SidebarInput) into sidebar-layout.tsx; group-related components (SidebarGroup, SidebarGroupLabel, SidebarGroupAction, SidebarGroupContent) into sidebar-group.tsx; menu components (SidebarMenu, SidebarMenuItem, SidebarMenuButton, SidebarMenuAction, SidebarMenuBadge, SidebarMenuSkeleton) into sidebar-menu.tsx; submenu components (SidebarMenuSub, SidebarMenuSubItem, SidebarMenuSubButton) into sidebar-submenu.tsx; isolate CVA variants (sidebarMenuButtonVariants) into sidebar-variants.ts; and create an index.ts that re-exports everything; ensure each new file imports shared utilities (useRender, mergeProps, cn, cva, useIsMobile, Button, Input, Tooltip, Sheet, etc.) and preserve original prop types and exports (export { Sidebar, SidebarProvider, useSidebar, ... }) so consumers are unchanged.Source: Coding guidelines
🟡 Minor comments (5)
packages/swingset/src/hooks/use-mobile.ts-5-5 (1)
5-5:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd explicit return type annotation for the exported hook.
The
useIsMobilehook is exported but lacks an explicit return type annotation. As per coding guidelines, public APIs should have explicit return types for clarity and type safety.📝 Proposed fix
-export function useIsMobile() { +export function useIsMobile(): boolean { const [isMobile, setIsMobile] = React.useState<boolean | undefined>(undefined);🤖 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/hooks/use-mobile.ts` at line 5, The exported hook useIsMobile lacks an explicit return type; update its signature to include the correct return type (e.g., boolean) so the public API is typed explicitly — change the declaration of useIsMobile to include the return type (useIsMobile(): boolean) and adjust any internal returns if necessary to satisfy the type.Source: Coding guidelines
packages/swingset/mdx-components.tsx-7-14 (1)
7-14:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd runtime validation for type assertion safety.
The type assertion on line 9 assumes the
codeelement hasclassNameandchildrenprops matching the expected shape, but there's no runtime validation. If MDX generates a code element with a different structure, this could fail silently or throw at runtime.🛡️ Proposed fix with type guard
+function hasCodeProps( + props: unknown +): props is { className?: string; children: string } { + return ( + typeof props === 'object' && + props !== null && + 'children' in props && + typeof props.children === 'string' + ); +} + function PreBlock({ children }: { children?: React.ReactNode }) { if (React.isValidElement(children) && (children as React.ReactElement).type === 'code') { - const { className, children: code } = (children as React.ReactElement<{ className?: string; children: string }>) - .props; + if (!hasCodeProps(children.props)) { + return <pre>{children}</pre>; + } + const { className, children: code } = children.props; return <CodeBlock className={className}>{code}</CodeBlock>; } return <pre>{children}</pre>; }As per coding guidelines: "Implement type guards for
unknowntypes using the patternfunction isType(value: unknown): value is Type."🤖 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/mdx-components.tsx` around lines 7 - 14, The PreBlock function assumes children is a ReactElement with props { className?: string; children: string }—add a runtime type guard to validate that before casting: implement an isCodeElement(value: unknown): value is React.ReactElement<{ className?: string; children: string }> that checks React.isValidElement(value), typeof value.props === 'object', 'children' in value.props and typeof value.props.children === 'string' (and optionally typeof value.props.className === 'string' || value.props.className === undefined); use this guard in PreBlock where you currently cast (children as React.ReactElement...) so you only extract className and code when isCodeElement(children) returns true, otherwise fall back to the <pre>{children}</pre> path.packages/swingset/src/components/CodeBlock.tsx-32-32 (1)
32-32:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd cleanup for timer on component unmount.
The
timerRefholds a timeout that may still be pending when the component unmounts. This can cause a warning about setting state on an unmounted component.🔧 Proposed fix
Add a cleanup effect after the timer ref declaration:
const timerRef = useRef<ReturnType<typeof setTimeout> | null>(null); +useEffect(() => { + return () => { + if (timerRef.current) { + clearTimeout(timerRef.current); + } + }; +}, []); + useEffect(() => {As per coding guidelines: "Implement proper cleanup in useEffect hooks."
🤖 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/components/CodeBlock.tsx` at line 32, The timerRef in CodeBlock.tsx may hold a pending timeout when the component unmounts; add a useEffect cleanup that clears the timeout stored in timerRef (use clearTimeout(timerRef.current)) and sets timerRef.current to null to prevent callbacks from running after unmount and avoid setState-on-unmounted warnings; update any places that set timerRef (via setTimeout) to be compatible with this cleanup and ensure the effect has an empty dependency array so it runs once on mount and cleans on unmount.packages/swingset/src/components/ui/breadcrumb.tsx-55-66 (1)
55-66:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove
role='link'from non-interactive current page element.Line 59 applies
role='link'to a<span>element representing the current page. This is an accessibility violation—the current page should not be presented as a link (interactive element). Remove therole='link'attribute and rely onaria-current='page'to indicate the current location.♿ Proposed fix
function BreadcrumbPage({ className, ...props }: React.ComponentProps<'span'>) { return ( <span data-slot='breadcrumb-page' - role='link' aria-disabled='true' aria-current='page' className={cn('text-foreground font-normal', className)} {...props} /> ); }Also add explicit return type:
🔧 Add return type
-function BreadcrumbPage({ className, ...props }: React.ComponentProps<'span'>) { +function BreadcrumbPage({ className, ...props }: React.ComponentProps<'span'>): React.JSX.Element {🤖 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/components/ui/breadcrumb.tsx` around lines 55 - 66, In BreadcrumbPage, remove the incorrect interactive semantics by deleting role='link' from the <span> used for the current page and keep aria-current='page' and aria-disabled='true' to convey non-interactive state; also add an explicit return type for the function (e.g., React.ReactElement or JSX.Element) on BreadcrumbPage to make its signature explicit.Source: Coding guidelines
packages/swingset/src/app/page.tsx-5-7 (1)
5-7:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winConsider a more resilient redirect target.
The hardcoded redirect to
/components/buttonassumes that the button component exists. If the button component is removed or renamed, this route will break.Consider either:
- Making this redirect dynamic based on the first available component from the registry
- Rendering a landing page instead of redirecting
🤖 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/app/page.tsx` around lines 5 - 7, HomePage currently calls redirect('/components/button') which is brittle; update the HomePage function to either (A) query the components registry (e.g., use the existing componentsRegistry, getComponentList, getAllComponents, or similar API) to pick the first available component slug and redirect to `/components/{slug}`, handling the empty-list case by falling back to a sensible default, or (B) stop redirecting and render a safe landing page UI instead. Locate the redirect call inside the HomePage component and replace it with logic that checks the registry for available components and redirects dynamically or returns a landing page when none exist.
🧹 Nitpick comments (20)
packages/swingset/README.md (3)
26-28: ⚡ Quick winAdd missing type import for clarity.
The example casts props to
MyComponentPropson line 27, but this type is never imported or defined. For a complete, runnable example, consider adding the import or noting that it should be imported from the component module.📝 Suggested addition
/** `@jsxImportSource` `@emotion/react` */ import type { StoryMeta } from '`@/lib/types`'; -import { MyComponent, myComponentStyles } from '`@clerk/ui/mosaic/components/my-component`'; +import { MyComponent, myComponentStyles, type MyComponentProps } from '`@clerk/ui/mosaic/components/my-component`';🤖 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/README.md` around lines 26 - 28, The example's Default function casts props to MyComponentProps but that type is not imported or defined; update the README example to import or declare MyComponentProps from the component module used by MyComponent so the example compiles — e.g., add an import statement for MyComponentProps (or a short local type declaration) and ensure MyComponent and MyComponentProps are referenced consistently in the Default(props: Record<string, unknown>) example.
7-9: ⚡ Quick winAdd language identifier to code block.
Per markdown linting, fenced code blocks should specify a language for proper syntax highlighting.
📝 Suggested fix
-``` +```bash pnpm dev --filter `@clerk/swingset`</details> <details> <summary>🤖 Prompt for AI Agents</summary>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/README.mdaround lines 7 - 9, The fenced code block
containing the command "pnpm dev --filter@clerk/swingset" in
packages/swingset/README.md needs a language identifier for markdown linting;
update the triple-backtick fence to include "bash" (i.e., ```bash) so the block
becomes a bash-highlighted code block and satisfies the linter.</details> <!-- cr-comment:v1:6ca9168e45c7875681860910 --> _Source: Linters/SAST tools_ --- `87-87`: _💤 Low value_ **Add language identifier to code block.** Per markdown linting, fenced code blocks should specify a language. For directory tree structures, you can use `text` or `plaintext`. <details> <summary>📝 Suggested fix</summary> ```diff -``` +```text src/ app/ Next.js App Router ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>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/README.mdat line 87, The fenced code block is missing a
language identifier; update the opening fence fromtotext (orplain text. Locate the block that starts with ``` and replace it with ```text while keeping the block content unchanged (e.g., the snippet showing the src/ app/ Next.js tree).Source: Linters/SAST tools
packages/ui/src/mosaic/Button.tsx (1)
54-54: 💤 Low valueRemove redundant
disabled || falsecoercion.The
disabled || falsecoercion is redundant sincebuttonStylesalready specifiesdefaultVariants: { disabled: false }. The variant system will handle the undefined case automatically.♻️ Simplification
- disabled={disabled || false} + disabled={disabled}🤖 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/Button.tsx` at line 54, Remove the redundant boolean coercion in the Button component: the prop usage disabled={disabled || false} should be simplified to just pass disabled (or omit it) because buttonStyles has defaultVariants: { disabled: false } and the variant system will handle undefined; update the Button component where disabled is forwarded (prop name disabled in the component and any JSX usage in Button.tsx) to stop forcing || false so the variant system controls the default.packages/shared/src/types/clerk.ts (1)
703-717: ⚡ Quick winDocument the new experimental public API surface more explicitly.
Line 703 and Line 2305 introduce new public API entries, but the exported props type is undocumented and the new methods are not explicitly tagged as
@experimental. Please add concise JSDoc so generated reference docs stay clear and consistent.Suggested patch
/** * Mounts the experimental MosaicOrganizationProfile component at the target element. + * `@experimental` This API is experimental and may change without notice. * `@param` targetNode Target node to mount the MosaicOrganizationProfile component. * `@param` props configuration parameters. */ __experimental_mountMosaicOrganizationProfile: ( @@ /** * Unmounts the experimental MosaicOrganizationProfile component from the target element. + * `@experimental` This API is experimental and may change without notice. * `@param` targetNode Target node to unmount the MosaicOrganizationProfile component from. */ __experimental_unmountMosaicOrganizationProfile: (targetNode: HTMLDivElement) => void; @@ +/** + * Props for mounting the experimental MosaicOrganizationProfile component. + * + * `@experimental` This API is experimental and may change without notice. + */ export type __experimental_MosaicOrganizationProfileProps = { appearance?: ClerkAppearanceTheme; };As per coding guidelines, public APIs in
packages/**/src/**/*.{ts,tsx}should have comprehensive JSDoc, and public/reference-facing JSDoc changes may need Docs-team review.Also applies to: 2305-2307
🤖 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/shared/src/types/clerk.ts` around lines 703 - 717, The new experimental API lacks descriptive JSDoc: add concise JSDoc comments for __experimental_mountMosaicOrganizationProfile and __experimental_unmountMosaicOrganizationProfile that include a short description, mark them with `@experimental`, document parameters (targetNode: HTMLDivElement and props?: __experimental_MosaicOrganizationProfileProps) and return type, and add JSDoc for the exported type __experimental_MosaicOrganizationProfileProps describing each prop and its purpose; ensure wording is reference-doc friendly and note that public/reference-facing JSDoc changes may require Docs-team review.Source: Coding guidelines
packages/ui/src/contexts/components/__experimental_MosaicOrganizationProfile.ts (1)
17-22: 💤 Low valueRemove redundant destructuring and reconstruction.
Lines 17-22 destructure
componentNamefromcontext, then immediately reconstruct it in the return object. Since the validation on line 11 already narrows the type and confirmscomponentNameis present, you can return the validated context directly.Proposed simplification
- const { componentName, ...ctx } = context; - - return { - ...ctx, - componentName, - }; + return context;🤖 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/contexts/components/__experimental_MosaicOrganizationProfile.ts` around lines 17 - 22, The current code needlessly destructures componentName from context into ctx then reassembles it; instead, stop the redundant destructuring and return the already-validated context directly. In the function that currently does "const { componentName, ...ctx } = context; return { ...ctx, componentName }", replace that logic with a direct return of the validated context variable (context) to preserve type narrowing and avoid creating an unnecessary ctx copy.packages/swingset/src/components/StoryCanvas.tsx (1)
28-33: ⚡ Quick winConsider including
foundin the dependency array or computing it inside the effect.The
useEffectusesfoundfrom the outer scope but doesn't list it in the dependency array. While this works correctly now (sincefoundis deterministic based oncomponentSlug/storySlug), it violates React's exhaustive-deps rule and could cause stale-closure bugs if the registry lookup logic becomes dynamic in the future.🔧 Suggested fix
Option 1: Include
foundin the dependency array (may cause unnecessary re-runs if object identity changes):useEffect(() => { if (!found) return; const nextKnobs = generateKnobs(found.mod.meta); setKnobs(nextKnobs); setKnobValues(initKnobValues(nextKnobs)); - }, [componentSlug, storySlug]); + }, [componentSlug, storySlug, found]);Option 2 (preferred): Compute
foundinside the effect to avoid external dependency:useEffect(() => { + const found = findStory(componentSlug, storySlug); if (!found) return; const nextKnobs = generateKnobs(found.mod.meta); setKnobs(nextKnobs); setKnobValues(initKnobValues(nextKnobs)); }, [componentSlug, storySlug]);🤖 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/components/StoryCanvas.tsx` around lines 28 - 33, The effect currently closes over found but doesn't declare it as a dependency; to fix, move the computation of found into the useEffect body (so useEffect computes found from componentSlug/storySlug locally) and keep the dependency array as [componentSlug, storySlug]; update the effect that calls generateKnobs(found.mod.meta), setKnobs, and setKnobValues(initKnobValues(...)) to use the locally computed found and early-return if it's falsy, referencing symbols useEffect, found, generateKnobs, setKnobs, initKnobValues, componentSlug and storySlug.packages/swingset/src/components/StoryEmbed.tsx (1)
15-15: 💤 Low valueConsider more specific typing for story component.
The type assertion
as React.ComponentType<Record<string, unknown>>is very permissive. Since stories are typed viaStoryModule, consider using a more specific type that reflects the actual story function signature or letting TypeScript infer from the module definition.💡 Alternative approach
-const StoryComp = storyModule[name] as React.ComponentType<Record<string, unknown>>; +const StoryComp = storyModule[name] as React.ComponentType<any>;Or define a more specific type in
types.tsif all stories share a common prop shape.🤖 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/components/StoryEmbed.tsx` at line 15, The current assertion "as React.ComponentType<Record<string, unknown>>" for StoryComp is too broad; update the typing to use the actual story module types instead of a generic record: change the cast to derive the component type from storyModule/StoryModule (e.g., use the exported StoryModule generic or the specific story function type) so TypeScript can infer props, or create a shared story-props type in types.ts and use that (e.g., React.ComponentType<StoryProps>) and apply it to StoryComp to reflect the real signature.packages/swingset/src/components/CodeBlock.tsx (1)
12-20: ⚡ Quick winAdd explicit return type annotation.
Helper functions should have explicit return types for clarity.
✨ Proposed fix
-function getHighlighter(): Promise<Highlighter> { +function getHighlighter(): Promise<Highlighter> {Wait, this already has a return type. Let me re-check... yes, line 12 already has
: Promise<Highlighter>. Skip this comment.🤖 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/components/CodeBlock.tsx` around lines 12 - 20, The helper function getHighlighter already declares an explicit return type Promise<Highlighter>, so no change is necessary; leave the function signature (getHighlighter) and its return type as-is and proceed without modification.Source: Coding guidelines
packages/swingset/mdx-components.tsx (1)
7-14: ⚡ Quick winAdd explicit return type annotation.
Internal helper functions should have explicit return types.
✨ Proposed fix
-function PreBlock({ children }: { children?: React.ReactNode }) { +function PreBlock({ children }: { children?: React.ReactNode }): React.JSX.Element { if (React.isValidElement(children) && (children as React.ReactElement).type === 'code') {As per coding guidelines: "Always define explicit return types for functions, especially public APIs."
🤖 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/mdx-components.tsx` around lines 7 - 14, The PreBlock component lacks an explicit return type; update its signature (function PreBlock) to declare a return type, e.g. : JSX.Element (or React.ReactElement | null if you prefer nullability), so the component's return type is explicit and consistent with project guidelines; keep the existing props typing for children and do not change the implementation inside the function.Source: Coding guidelines
packages/swingset/src/components/DocsViewer.tsx (1)
5-7: 💤 Low valueReconsider the type assertion on
dynamic().
next/dynamicalready returns a properly typed component. Theas React.ComponentTypeassertion may be unnecessary and could mask type issues. Consider letting TypeScript infer the type or using a more specific type that matches the dynamic import return value.💡 Suggested improvement
-const docModules: Record<string, React.ComponentType> = { - button: dynamic(() => import('../stories/button.mdx')) as React.ComponentType, +const docModules: Record<string, React.ComponentType> = { + button: dynamic(() => import('../stories/button.mdx')), };🤖 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/components/DocsViewer.tsx` around lines 5 - 7, The explicit "as React.ComponentType" cast on the dynamic import is unnecessary and can hide type errors; remove the assertion from the dynamic(...) call and let TypeScript infer the type, or make the container type align with dynamic's return type (e.g. change docModules to Record<string, ReturnType<typeof dynamic>> or Record<string, React.ComponentType<any>>). Update the docModules declaration and the entry using dynamic(() => import('../stories/button.mdx')) accordingly so you no longer assert the component with "as React.ComponentType".packages/swingset/src/components/app-sidebar.tsx (2)
26-90: ⚡ Quick winAdd explicit return type annotation.
The component function should declare an explicit return type per TypeScript guidelines for functions.
♻️ Suggested addition
-export function AppSidebar({ ...props }: React.ComponentProps<typeof Sidebar>) { +export function AppSidebar({ ...props }: React.ComponentProps<typeof Sidebar>): JSX.Element { const pathname = usePathname();🤖 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/components/app-sidebar.tsx` around lines 26 - 90, The AppSidebar component is missing an explicit return type; update the function signature for AppSidebar to include a JSX return type (e.g., : JSX.Element or : React.ReactElement) so TypeScript enforces the component's return contract (update the signature on the exported function AppSidebar that currently deconstructs props and returns the Sidebar JSX).Source: Coding guidelines
24-24: ⚖️ Poor tradeoffModule-level
getSidebarGroups()eagerly loads all story modules.The module-level call to
getSidebarGroups()at line 24 causes all story modules to be eagerly imported when this client component loads. This is noted inpage.tsxas the reason for using a redirect instead of rendering sidebar groups from a Server Component.For the current scope of a component explorer, this is likely acceptable. However, if the number of components grows significantly, consider lazy-loading story modules on-demand or splitting the sidebar into smaller client 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 `@packages/swingset/src/components/app-sidebar.tsx` at line 24, The module-level call to getSidebarGroups() (assigned to groups) eagerly imports all story modules; move this call into the client component lifecycle so story modules are loaded on demand—e.g., replace the top-level const groups = getSidebarGroups() with a lazy load inside the AppSidebar component using useState/useEffect (or React.lazy/dynamic import) to call getSidebarGroups() at mount or when needed, or split the sidebar into smaller client components that each import their portions; update references to groups within the component to use the state value returned by the lazy load.packages/swingset/src/app/components/[component]/page.tsx (1)
7-10: ⚡ Quick winAdd explicit return type annotation.
The async page function should declare an explicit return type per TypeScript guidelines for functions.
♻️ Suggested addition
-export default async function ComponentPage({ params }: Props) { +export default async function ComponentPage({ params }: Props): Promise<JSX.Element> { const { component } = await params; return <DocsViewer slug={component} />; }🤖 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/app/components/`[component]/page.tsx around lines 7 - 10, Add an explicit return type to the async page function ComponentPage so TypeScript knows the component's return shape; change the signature for ComponentPage({ params }: Props) to declare a return type of Promise<JSX.Element> (or Promise<React.ReactNode> if you prefer), ensuring the exported async function ComponentPage is annotated and still returns the DocsViewer JSX node with slug={component}.Source: Coding guidelines
packages/swingset/src/app/components/[component]/[story]/page.tsx (1)
7-15: ⚡ Quick winAdd explicit return type annotation.
The async page function should declare an explicit return type per TypeScript guidelines for functions.
♻️ Suggested addition
-export default async function StoryPage({ params }: Props) { +export default async function StoryPage({ params }: Props): Promise<JSX.Element> { const { component, story } = await params; return ( <StoryCanvas🤖 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/app/components/`[component]/[story]/page.tsx around lines 7 - 15, Add an explicit return type to the async page function StoryPage: update the function signature for StoryPage({ params }: Props) to declare its return type (e.g., Promise<JSX.Element> or Promise<React.ReactNode>) so the compiler knows the resolved JSX type returned by StoryCanvas; ensure the exported async function signature includes that Promise-returning type while leaving the body and usage of componentSlug/storySlug and StoryCanvas unchanged.Source: Coding guidelines
packages/swingset/src/components/ThemeToggle.tsx (1)
9-34: ⚡ Quick winAdd explicit return type annotation.
The component function should declare an explicit return type per TypeScript guidelines for functions.
♻️ Suggested addition
-export function ThemeToggle() { +export function ThemeToggle(): JSX.Element { const { resolvedTheme, setTheme } = useTheme();🤖 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/components/ThemeToggle.tsx` around lines 9 - 34, Add an explicit return type to the ThemeToggle component by annotating its function signature (e.g., change export function ThemeToggle() to export function ThemeToggle(): JSX.Element or React.ReactElement) so the component's return type is declared; if your project requires it, ensure the appropriate React types are available/imported (or use React.FC) and update the function signature for ThemeToggle accordingly.Source: Coding guidelines
packages/swingset/src/components/ClientRoot.tsx (1)
20-29: ⚡ Quick winAdd explicit return type annotations.
Both
useBreadcrumbandClientRootshould declare explicit return types per TypeScript guidelines for functions.♻️ Suggested additions
-function useBreadcrumb() { +function useBreadcrumb(): string[] { const pathname = usePathname(); // ... } -export function ClientRoot({ children }: { children: React.ReactNode }) { +export function ClientRoot({ children }: { children: React.ReactNode }): JSX.Element { const crumbs = useBreadcrumb(); // ... }Also applies to: 31-70
🤖 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/components/ClientRoot.tsx` around lines 20 - 29, Both functions lack explicit TypeScript return types; add them to satisfy guidelines by annotating useBreadcrumb to return string[] (function useBreadcrumb(): string[]) and annotate ClientRoot to return a JSX element (e.g., function ClientRoot(): JSX.Element or React.ReactElement). Update the function signatures for useBreadcrumb and ClientRoot accordingly and ensure any necessary React types are imported if not already present.Source: Coding guidelines
packages/swingset/src/app/layout.tsx (1)
17-31: ⚡ Quick winAdd explicit return type to exported function.
The
RootLayoutfunction is exported but lacks an explicit return type annotation. Per coding guidelines, exported functions should have explicit return types for clarity and type safety.♻️ Suggested fix
-export default function RootLayout({ children }: { children: React.ReactNode }) { +export default function RootLayout({ children }: { children: React.ReactNode }): React.JSX.Element { return (🤖 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/app/layout.tsx` around lines 17 - 31, The exported function RootLayout currently lacks an explicit return type; update its signature to include a React return type (e.g., React.ReactElement or React.JSX.Element) so the declaration becomes: export default function RootLayout({ children }: { children: React.ReactNode }): React.JSX.Element { ... } ensuring the return type covers the JSX returned by RootLayout and satisfies the project's typing rules.Source: Coding guidelines
packages/swingset/next.config.mjs (1)
30-33: 💤 Low valueConsider making the webpack alias more maintainable.
The hardcoded relative path
'../ui/src/mosaic'couples this config to the current monorepo structure. If packages are reorganized, this alias will silently break.♻️ Alternative approaches
Option 1: Use TypeScript path mapping instead (more portable):
In
tsconfig.json:{ "compilerOptions": { "paths": { "`@clerk/ui/mosaic`": ["../ui/src/mosaic"] } } }Option 2: Add a runtime validation:
webpack(config) { + const mosaicPath = resolve(__dirname, '../ui/src/mosaic'); + if (!require('fs').existsSync(mosaicPath)) { + throw new Error(`Mosaic path not found: ${mosaicPath}`); + } - config.resolve.alias['`@clerk/ui/mosaic`'] = resolve(__dirname, '../ui/src/mosaic'); + config.resolve.alias['`@clerk/ui/mosaic`'] = mosaicPath; return config; },🤖 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/next.config.mjs` around lines 30 - 33, The webpack alias currently hardcodes a relative path ('../ui/src/mosaic') which is brittle; update the alias in next.config.mjs (config.resolve.alias['`@clerk/ui/mosaic`']) to a more maintainable resolution strategy — either rely on TypeScript path mapping (add an entry for "`@clerk/ui/mosaic`" in tsconfig.json paths and remove the hardcoded alias) or compute the path at runtime using a robust lookup (e.g., use require.resolve or a package-based resolution strategy to locate the `@clerk/ui/mosaic` module) and add a runtime existence check to log/throw a clear error if resolution fails.packages/swingset/src/components/ThemeProvider.tsx (1)
6-17: ⚡ Quick winAdd explicit return type to exported function.
The
ThemeProviderfunction is exported but lacks an explicit return type annotation. Per coding guidelines, exported functions should have explicit return types for type safety and API clarity.♻️ Suggested fix
-export function ThemeProvider({ children }: { children: React.ReactNode }) { +export function ThemeProvider({ children }: { children: React.ReactNode }): React.JSX.Element { return (🤖 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/components/ThemeProvider.tsx` around lines 6 - 17, The exported function ThemeProvider lacks an explicit return type; update its signature to include a JSX return type (e.g., React.ReactElement or JSX.Element) so the exported API is explicitly typed—locate the ThemeProvider declaration and add the return type annotation after the parameter list while leaving the internals (NextThemesProvider, attribute/defaultTheme/enableSystem/disableTransitionOnChange and {children}) unchanged.Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Repository UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 11c9a89a-2617-4b42-ad92-b2651287775f
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (62)
packages/clerk-js/sandbox/app.tspackages/clerk-js/sandbox/template.htmlpackages/clerk-js/src/core/clerk.tspackages/shared/src/types/clerk.tspackages/swingset/README.mdpackages/swingset/components.jsonpackages/swingset/mdx-components.tsxpackages/swingset/next-env.d.tspackages/swingset/next.config.mjspackages/swingset/package.jsonpackages/swingset/postcss.config.mjspackages/swingset/src/app/components/[component]/[story]/page.tsxpackages/swingset/src/app/components/[component]/page.tsxpackages/swingset/src/app/globals.csspackages/swingset/src/app/layout.tsxpackages/swingset/src/app/page.tsxpackages/swingset/src/components/ClientRoot.tsxpackages/swingset/src/components/CodeBlock.tsxpackages/swingset/src/components/DocsViewer.tsxpackages/swingset/src/components/KnobPanel.tsxpackages/swingset/src/components/PropTable.tsxpackages/swingset/src/components/StoryCanvas.tsxpackages/swingset/src/components/StoryEmbed.tsxpackages/swingset/src/components/ThemeProvider.tsxpackages/swingset/src/components/ThemeToggle.tsxpackages/swingset/src/components/VariablesPanel.tsxpackages/swingset/src/components/app-sidebar.tsxpackages/swingset/src/components/ui/breadcrumb.tsxpackages/swingset/src/components/ui/button.tsxpackages/swingset/src/components/ui/collapsible.tsxpackages/swingset/src/components/ui/input.tsxpackages/swingset/src/components/ui/label.tsxpackages/swingset/src/components/ui/select.tsxpackages/swingset/src/components/ui/separator.tsxpackages/swingset/src/components/ui/sheet.tsxpackages/swingset/src/components/ui/sidebar.tsxpackages/swingset/src/components/ui/skeleton.tsxpackages/swingset/src/components/ui/switch.tsxpackages/swingset/src/components/ui/tabs.tsxpackages/swingset/src/components/ui/tooltip.tsxpackages/swingset/src/hooks/use-mobile.tspackages/swingset/src/lib/generateKnobs.tspackages/swingset/src/lib/registry.tspackages/swingset/src/lib/slug.tspackages/swingset/src/lib/types.tspackages/swingset/src/lib/utils.tspackages/swingset/src/stories/button.mdxpackages/swingset/src/stories/button.stories.tsxpackages/swingset/src/types/global.d.tspackages/swingset/tsconfig.jsonpackages/ui/src/components/MosaicOrganizationProfile/MosaicOrganizationProfile.tsxpackages/ui/src/components/MosaicOrganizationProfile/index.tspackages/ui/src/contexts/ClerkUIComponentsContext.tsxpackages/ui/src/contexts/components/__experimental_MosaicOrganizationProfile.tspackages/ui/src/contexts/components/index.tspackages/ui/src/customizables/parseAppearance.tspackages/ui/src/lazyModules/components.tspackages/ui/src/mosaic/Button.tsxpackages/ui/src/mosaic/__tests__/cva.test.tspackages/ui/src/mosaic/cva.tspackages/ui/src/types.tsreferences/mosaic-architecture.md
…rp/swingset PR Strip out the experimental mountMosaicOrganizationProfile method and all related wiring that was unintentionally added to this PR. The PR should only contain the new swingset component explorer and mosaic UI foundation (cva, Button, tests). Reverted: - __experimental_mountMosaicOrganizationProfile from clerk-js/core/clerk.ts - Related sandbox wiring in clerk-js - New types in shared/src/types/clerk.ts Removed from packages/ui: - MosaicOrganizationProfile component files - Context and lazy module wiring - Type definitions Resolved merge conflict in cva.test.ts (kept HEAD version).
🦋 Changeset detectedLatest commit: 142b54e 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 |
- Fix VariablesPanel to use 'rounded' instead of 'radius' - Fix control flow statements to use explicit curly braces - Fix function naming consistency (setRounded instead of setRadius) - Add dependency to useEffect hook - Add eslint-disable comment for label-has-associated-control - Reorganize imports to match linting requirements
- Auto-fix import sorting with ESLint - Fix floating promises with void operator - Add button type attributes - Fix PropTable type casting for default values - Fix label a11y comment placement - Add explicit button type in CodeBlock and SidebarRail
@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: |
2 participants
Description
Scaffolds new component documentation package that is used to document and iterate on mosaic design system components.
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
Documentation
Chores