diff --git a/.changeset/mosaic-render-props.md b/.changeset/mosaic-render-props.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/mosaic-render-props.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/packages/headless/src/utils/index.ts b/packages/headless/src/utils/index.ts index f2a2d4c17c4..cafe09fe0a2 100644 --- a/packages/headless/src/utils/index.ts +++ b/packages/headless/src/utils/index.ts @@ -6,5 +6,6 @@ export { mergeProps, type RenderProp, type RenderPropOrElement, + type RenderProps, useRender, } from './use-render'; diff --git a/packages/headless/src/utils/use-render.test-d.ts b/packages/headless/src/utils/use-render.test-d.ts index 91701264962..051fe2eaad4 100644 --- a/packages/headless/src/utils/use-render.test-d.ts +++ b/packages/headless/src/utils/use-render.test-d.ts @@ -1,14 +1,36 @@ import type React from 'react'; import { describe, expectTypeOf, test } from 'vitest'; -import type { ComponentProps } from './use-render'; +import type { ComponentProps, RenderProps } from './use-render'; + +type RenderFn = Extract< + NonNullable['render']>, + (...args: never[]) => unknown +>; +type RenderArg = + RenderFn extends (props: infer P) => React.ReactElement ? P : never; +type HasColor

= 'color' extends keyof P ? true : false; describe('use-render', () => { - test('render prop arg is narrowed to the element tag props, not the generic HTMLAttributes', () => { - type Props = ComponentProps<'button'>; - type RenderFn = Extract, (...args: never[]) => unknown>; - type RenderArg = RenderFn extends (props: infer P) => React.ReactElement ? P : never; - expectTypeOf().toEqualTypeOf>(); + test('a part keeps its own tag props', () => { + expectTypeOf>().toExtend<{ type?: 'button' | 'submit' | 'reset' }>(); + }); + + test('the legacy `color` attribute is dropped, so it cannot widen a `color` variant', () => { + expectTypeOf>>().toEqualTypeOf(); + expectTypeOf>>().toEqualTypeOf(); + }); + + test('the render arg is the tag-agnostic RenderProps, not the default tag props', () => { + expectTypeOf>().toEqualTypeOf(); + expectTypeOf>().toEqualTypeOf(); + }); + + test('render props spread onto an element other than the default tag', () => { + // The point of `render`: a `div` part rendering an ``. A tag-pinned `ref` + // would make this fail, which is what every call site used to work around. + expectTypeOf().toExtend>(); + expectTypeOf().toExtend>(); }); test('render also accepts an element to clone', () => { diff --git a/packages/headless/src/utils/use-render.tsx b/packages/headless/src/utils/use-render.tsx index 8a58fd224d1..b314ea49396 100644 --- a/packages/headless/src/utils/use-render.tsx +++ b/packages/headless/src/utils/use-render.tsx @@ -5,27 +5,48 @@ import * as React from 'react'; // Types // --------------------------------------------------------------------------- +/** + * The props a `render` callback receives, deliberately *not* the default tag's props. + * + * `render` exists to swap the rendered element, so the element's type is unknown at + * the point this is declared. Two things follow, and both were previously worked + * around at each call site instead of here: + * + * - `ref` is not pinned to the default tag. A `div` part rendering an `` could + * not spread its props, because `Ref` is not a `Ref`. + * - `color` is dropped. It is a non-standard HTML attribute typed `string`, so it + * collides with the `color` variant a styled component spreads these props into. + */ +export type RenderProps = Omit, 'color'> & { + // SAFETY: the rendered element is chosen by the callback, after this type is fixed, so + // no concrete element type is correct here. `Ref` does not work: `RefObject` + // is not a `RefObject`. `any` is what makes the ref spreadable onto + // whatever the callback returns, which is the whole point of `render`. Base UI's + // `HTMLProps` resolves this the same way. + ref?: React.Ref; +}; + /** * A render prop: a function that receives computed HTML props and returns a JSX element. */ -export type RenderProp> = (props: Props) => React.ReactElement; +export type RenderProp = (props: Props) => React.ReactElement; /** * A `render` prop: a render function receiving the part's computed props, or a - * React element to clone with them (`render={}`). The element form lets a - * part render a component whose own props diverge from the tag's, which a render - * function cannot express — it is typed to receive the tag's props verbatim. + * React element to clone with them (`render={}`). */ -export type RenderPropOrElement = - | RenderProp> - | React.ReactElement; +export type RenderPropOrElement = RenderProp | React.ReactElement; /** - * Props accepted by any primitive part. Extends the native props for `Tag` - * and adds the optional `render` escape hatch, narrowed to that tag's props. + * Props accepted by any primitive part: the native props for `Tag` plus the + * optional `render` escape hatch. `color` is dropped for the reason given on + * `RenderProps` — a part and its render callback expose the same contract. */ -export type ComponentProps = React.ComponentPropsWithRef & { - render?: RenderPropOrElement; +export type ComponentProps = Omit< + React.ComponentPropsWithRef, + 'color' +> & { + render?: RenderPropOrElement; }; /** @@ -125,7 +146,7 @@ interface UseRenderParamsBase< /** Fallback HTML tag when `render` is not provided. */ defaultTagName: Tag; /** Render prop or element from the consumer. */ - render?: RenderPropOrElement; + render?: RenderPropOrElement; /** Ref(s) to merge onto the rendered element. Merged with the element's own ref. */ ref?: React.Ref | Array | undefined>; /** State object. Keys are mapped to data attributes via `stateAttributesMapping`. */ @@ -197,9 +218,7 @@ export function useRender< const computedProps = { ...props, ...dataAttrs }; if (typeof render === 'function') { - // SAFETY: computedProps is the tag's props widened with data-* attrs; the render - // function is declared to receive this tag's props. - return render({ ...computedProps, ref: mergedRef } as React.ComponentPropsWithRef); + return render({ ...computedProps, ref: mergedRef }); } if (React.isValidElement(render)) { diff --git a/packages/swingset/src/stories/dialog.component.stories.tsx b/packages/swingset/src/stories/dialog.component.stories.tsx index afe4b77b335..051d810540c 100644 --- a/packages/swingset/src/stories/dialog.component.stories.tsx +++ b/packages/swingset/src/stories/dialog.component.stories.tsx @@ -1,4 +1,5 @@ /** @jsxImportSource @emotion/react */ +import type { RenderProps } from '@clerk/headless/utils'; import { Button } from '@clerk/ui/mosaic/components/button'; import { Dialog, dialogRecipe } from '@clerk/ui/mosaic/components/dialog'; @@ -15,9 +16,7 @@ export const meta: StoryMeta = { styles: dialogRecipe, }; -const dialogTrigger = ({ color: _nativeColor, ...props }: React.HTMLAttributes) => ( - -); +const dialogTrigger = (props: RenderProps) => ; export function Default(args: Record) { const { size } = args as { size?: 'md' | 'lg' }; diff --git a/packages/ui/src/mosaic/components/button/button.tsx b/packages/ui/src/mosaic/components/button/button.tsx index df42713e57d..7906e9554b7 100644 --- a/packages/ui/src/mosaic/components/button/button.tsx +++ b/packages/ui/src/mosaic/components/button/button.tsx @@ -1,12 +1,12 @@ import * as stylex from '@stylexjs/stylex'; import React from 'react'; -import type { MosaicComponentProps } from '../../props'; +import type { MosaicElementProps } from '../../props'; import { mergeStyleProps, themeProps } from '../../props'; import { truncationStyles } from '../typography.styles'; import { iconSizes, sizes, styles, variants } from './button.styles'; -export interface ButtonProps extends Omit, 'render'> { +export interface ButtonProps extends MosaicElementProps<'button'> { color?: 'primary' | 'neutral' | 'negative'; variant?: 'filled' | 'outline' | 'ghost' | 'link'; size?: 'sm' | 'md' | 'lg'; diff --git a/packages/ui/src/mosaic/components/dialog.tsx b/packages/ui/src/mosaic/components/dialog.tsx index 82867213791..45927daf298 100644 --- a/packages/ui/src/mosaic/components/dialog.tsx +++ b/packages/ui/src/mosaic/components/dialog.tsx @@ -4,6 +4,7 @@ import type { ReactNode } from 'react'; import React from 'react'; import { Dialog as Primitive } from '../primitives/dialog'; +import type { MosaicComponentProps } from '../props'; import type { RecipeVariantProps } from '../slot-recipe'; import { defineSlotRecipe, useRecipe } from '../slot-recipe'; @@ -81,48 +82,46 @@ declare module '../registry' { } } -const Backdrop = React.forwardRef>( - function DialogBackdrop(props, ref) { - const { backdrop } = useRecipe(dialogRecipe); - return ( - - ); - }, -); +export type DialogBackdropProps = React.ComponentPropsWithoutRef; +export type DialogViewportProps = React.ComponentPropsWithoutRef; +export type DialogPopupProps = React.ComponentPropsWithoutRef; -const Viewport = React.forwardRef>( - function DialogViewport(props, ref) { - const { viewport } = useRecipe(dialogRecipe); - return ( - - ); - }, -); +const Backdrop = React.forwardRef(function DialogBackdrop(props, ref) { + const { backdrop } = useRecipe(dialogRecipe); + return ( + + ); +}); -const Popup = React.forwardRef>( - function DialogPopup(props, ref) { - const variantProps = React.useContext(DialogVariantContext); - const { popup } = useRecipe(dialogRecipe, { variants: variantProps }); - return ( - - ); - }, -); +const Viewport = React.forwardRef(function DialogViewport(props, ref) { + const { viewport } = useRecipe(dialogRecipe); + return ( + + ); +}); + +const Popup = React.forwardRef(function DialogPopup(props, ref) { + const variantProps = React.useContext(DialogVariantContext); + const { popup } = useRecipe(dialogRecipe, { variants: variantProps }); + return ( + + ); +}); interface DialogProps extends Pick { - trigger: React.ComponentProps['render']; + trigger: MosaicComponentProps<'button'>['render']; children: ReactNode | ((ctx: { close: () => void }) => ReactNode); size?: DialogVariantProps['size']; } diff --git a/packages/ui/src/mosaic/components/item/item.tsx b/packages/ui/src/mosaic/components/item/item.tsx index d79cc885538..2b93ccc66e7 100644 --- a/packages/ui/src/mosaic/components/item/item.tsx +++ b/packages/ui/src/mosaic/components/item/item.tsx @@ -1,4 +1,4 @@ -import { type RenderProp, useRender } from '@clerk/headless/utils'; +import { useRender } from '@clerk/headless/utils'; import * as stylex from '@stylexjs/stylex'; import React from 'react'; @@ -16,7 +16,7 @@ const DEFAULT_SIZE: Size = 'md'; /** Carries `Item.Root`'s size down to the parts it scales (`Item.Media`). */ const ItemContext = React.createContext(DEFAULT_SIZE); -export type ItemProps = Omit, 'render'> & { +export type ItemProps = MosaicComponentProps<'div'> & { /** * Row height and gap. Also sizes a nested `Item.Media`, which reads this from * context rather than taking its own prop, so a row scales as one unit. @@ -24,8 +24,6 @@ export type ItemProps = Omit, 'render'> & { * @default 'md' */ size?: Size; - /** Render a custom element (e.g. a link or button) in place of the default `div`. */ - render?: RenderProp>; }; /** diff --git a/packages/ui/src/mosaic/components/menu/menu.tsx b/packages/ui/src/mosaic/components/menu/menu.tsx index 5fdca0ea9de..a8aa0abbc80 100644 --- a/packages/ui/src/mosaic/components/menu/menu.tsx +++ b/packages/ui/src/mosaic/components/menu/menu.tsx @@ -4,18 +4,20 @@ import type { MenuPortalProps, MenuProps, MenuSeparatorProps, - MenuTriggerProps, } from '@clerk/headless/menu'; import { Menu as Primitive } from '@clerk/headless/menu'; import * as stylex from '@stylexjs/stylex'; import React from 'react'; +import type { MosaicComponentProps } from '../../props'; import { mergeStyleProps, themeProps } from '../../props'; import { Button } from '../button'; import { Icon } from '../icon'; import { styles } from './menu.styles'; -export type { MenuProps, MenuSeparatorProps, MenuTriggerProps }; +export type { MenuProps, MenuSeparatorProps }; + +export type MenuTriggerProps = MosaicComponentProps<'button'>; /** * Opens the menu. Renders a ghost `Button` holding an ellipsis glyph by default; @@ -25,20 +27,21 @@ export const MenuTrigger = React.forwardRef { render, className, style, children, ...rest }, ref, ) { + const trigger: MenuTriggerProps['render'] = + render ?? + (props => ( +