diff --git a/.claude/skills/component/references/anti-patterns.md b/.claude/skills/component/references/anti-patterns.md index 5878d13e..8cce8ec6 100644 --- a/.claude/skills/component/references/anti-patterns.md +++ b/.claude/skills/component/references/anti-patterns.md @@ -65,6 +65,42 @@ import { Button } from '@lib/button'; --- +## Unnecessary Render Function Wrappers + +```tsx +// BAD: Function wrapper just passes props through + ( + + )} +/> + +// BAD: Trivial passthrough + } /> + +// BAD: Passing component reference directly — breaks hooks reconciliation + + +// GOOD: Element form — className and children on the headless component +}> + + + +// GOOD: Element form for passthrough +} /> + +// GOOD: Element form with extra props on render target +}> + + +``` + +**Why it fails:** Function wrappers add indirection, noise, and implicit `any` on `props` parameter. Component references (`render={Component}`) bypass React's reconciliation — components are called as plain functions, breaking hooks. Use element form (`render={}`) or function form only when rendering a different element type or accessing component state. + +--- + ## The `as` Prop ```tsx @@ -273,6 +309,7 @@ const Button = forwardRef(({ children, ...props - [ ] State styled via data attributes, not inline - [ ] No CSS shipped in component package - [ ] Polymorphism via `render` or `asChild`, not `as` +- [ ] No unnecessary render function wrappers (`render={}` over `render={(props) => }`) - [ ] Both controlled and uncontrolled modes - [ ] Nested instances don't interfere - [ ] Exit animations possible diff --git a/.claude/skills/component/references/polymorphism.md b/.claude/skills/component/references/polymorphism.md index 00c4908e..3465ed12 100644 --- a/.claude/skills/component/references/polymorphism.md +++ b/.claude/skills/component/references/polymorphism.md @@ -17,22 +17,53 @@ Polymorphism allows users to customize which element a component renders as. Two ## `render` Pattern (Preferred) +Two forms: element and function. + ### Element Form — Simple Cases ```tsx -// Renders MyButton with Dialog.Trigger behavior -}>Open dialog +// Renders Button with PlayButton behavior — clones element, merges props +}> + + + +// Pass props to the render target directly +}> + + ``` -### Function Form — State Access +The headless component clones the element and merges its own props onto it. + +### Function Form — State Access or Different Element ```tsx // Access internal state for conditional rendering {state.checked ? : }} /> + +// Render a fundamentally different element type + ( +
+ +
+ )} +/> ``` +### When to Use Which + +| Scenario | Form | +| --- | --- | +| Simple element swap | `render={}` | +| Render target needs its own props | `render={}` | +| Need internal state access | `render={(props, state) => ...}` | +| Rendering a different element type | `render={(props) =>
...}` | + +**Do not** pass component references directly (`render={Component}`). React calls render functions as plain functions, which breaks hooks reconciliation. Always use element form (`render={}`) or function form. + --- ## `asChild` Pattern @@ -51,7 +82,7 @@ Polymorphism allows users to customize which element a component renders as. Two | Concern | `render` | `asChild` | | ------------------ | ------------------------------------- | ----------------------------------------- | -| **Prop flow** | Explicit — you spread props visibly | Hidden — `cloneElement` merges implicitly | +| **Prop flow** | Explicit — element or function forms | Hidden — `cloneElement` merges implicitly | | **State access** | Function form exposes component state | No state access | | **TypeScript** | Predictable inference | Can slow IDE autocomplete | | **Debugging** | Traceable prop flow | Magic makes tracing difficult | diff --git a/.claude/skills/component/references/react.md b/.claude/skills/component/references/react.md index 79a6769a..83409612 100644 --- a/.claude/skills/component/references/react.md +++ b/.claude/skills/component/references/react.md @@ -157,6 +157,7 @@ useImperativeHandle(actionsRef, () => ({ open, close })); - Accept `ReactElement` or `(props, state) => ReactElement` - Use `cloneElement` for element form - Use `mergeProps` to combine internal + external props +- **Do not** accept component references (`render={Component}`) — calling components as plain functions breaks hooks reconciliation **`mergeProps` behavior:** diff --git a/packages/react/src/presets/audio/minimal-skin.tailwind.tsx b/packages/react/src/presets/audio/minimal-skin.tailwind.tsx index eaa77156..9dfb04a6 100644 --- a/packages/react/src/presets/audio/minimal-skin.tailwind.tsx +++ b/packages/react/src/presets/audio/minimal-skin.tailwind.tsx @@ -78,6 +78,10 @@ const SliderFill = forwardRef & { type?: ' ); }); +const SliderBuffer = forwardRef>(function SliderBuffer(props, ref) { + return ; +}); + const SliderThumb = forwardRef & { persistent?: boolean }>(function SliderThumb( { persistent, className, ...props }, ref @@ -102,15 +106,11 @@ function VolumePopover(): ReactNode { const volumeUnsupported = usePlayer((s) => s.volumeAvailability === 'unsupported'); const muteButton = ( - ( - - )} - /> + }> + + + + ); if (volumeUnsupported) return muteButton; @@ -119,9 +119,9 @@ function VolumePopover(): ReactNode { - }> - }> - } /> + }> + }> + } /> } /> @@ -145,15 +145,11 @@ export function MinimalAudioSkinTailwind(props: MinimalAudioSkinProps): ReactNod ( - - )} - /> + }> + + + + } /> @@ -164,17 +160,12 @@ export function MinimalAudioSkinTailwind(props: MinimalAudioSkinProps): ReactNod ( - - )} - /> + }> + + + {SEEK_TIME} + + } /> Seek backward {SEEK_TIME} seconds @@ -183,17 +174,12 @@ export function MinimalAudioSkinTailwind(props: MinimalAudioSkinProps): ReactNod ( - - )} - /> + }> + + + {SEEK_TIME} + + } /> Seek forward {SEEK_TIME} seconds @@ -207,23 +193,19 @@ export function MinimalAudioSkinTailwind(props: MinimalAudioSkinProps): ReactNod - }> - }> - } /> - } /> + }> + }> + } /> + } /> - } /> + } />
- )} - /> + }> + + + + ); if (volumeUnsupported) return muteButton; @@ -81,15 +77,11 @@ export function MinimalAudioSkin(props: MinimalAudioSkinProps): ReactNode { ( - - )} - /> + }> + + + + } /> @@ -100,17 +92,12 @@ export function MinimalAudioSkin(props: MinimalAudioSkinProps): ReactNode { ( - - )} - /> + }> + + + {SEEK_TIME} + + } /> Seek backward {SEEK_TIME} seconds @@ -119,17 +106,12 @@ export function MinimalAudioSkin(props: MinimalAudioSkinProps): ReactNode { ( - - )} - /> + }> + + + {SEEK_TIME} + + } /> Seek forward {SEEK_TIME} seconds @@ -155,11 +137,7 @@ export function MinimalAudioSkin(props: MinimalAudioSkinProps): ReactNode {
- )} - /> + }> + + + + ); if (volumeUnsupported) return muteButton; @@ -118,9 +118,9 @@ function VolumePopover(): ReactNode { - }> - }> - } /> + }> + }> + } /> } /> @@ -143,15 +143,11 @@ export function AudioSkinTailwind(props: AudioSkinProps): ReactNode { ( - - )} - /> + }> + + + + } /> @@ -162,17 +158,12 @@ export function AudioSkinTailwind(props: AudioSkinProps): ReactNode { ( - - )} - /> + }> + + + {SEEK_TIME} + + } /> Seek backward {SEEK_TIME} seconds @@ -181,17 +172,12 @@ export function AudioSkinTailwind(props: AudioSkinProps): ReactNode { ( - - )} - /> + }> + + + {SEEK_TIME} + + } /> Seek forward {SEEK_TIME} seconds @@ -199,23 +185,19 @@ export function AudioSkinTailwind(props: AudioSkinProps): ReactNode { - }> - }> - } /> - } /> + }> + }> + } /> + } /> - } /> + } /> - )} - /> + }> + + + + ); if (volumeUnsupported) return muteButton; @@ -80,15 +76,11 @@ export function AudioSkin(props: AudioSkinProps): ReactNode { ( - - )} - /> + }> + + + + } /> @@ -99,17 +91,12 @@ export function AudioSkin(props: AudioSkinProps): ReactNode { ( - - )} - /> + }> + + + {SEEK_TIME} + + } /> Seek backward {SEEK_TIME} seconds @@ -118,17 +105,12 @@ export function AudioSkin(props: AudioSkinProps): ReactNode { ( - - )} - /> + }> + + + {SEEK_TIME} + + } /> Seek forward {SEEK_TIME} seconds @@ -148,11 +130,7 @@ export function AudioSkin(props: AudioSkinProps): ReactNode { - )} - /> + }> + + + + ); if (volumeUnsupported) return muteButton; @@ -165,9 +165,9 @@ function VolumePopover(): ReactNode { - }> - }> - } /> + }> + }> + } /> } /> @@ -212,15 +212,11 @@ export function MinimalVideoSkinTailwind(props: MinimalVideoSkinProps): ReactNod ( - - )} - /> + }> + + + + } /> @@ -231,17 +227,12 @@ export function MinimalVideoSkinTailwind(props: MinimalVideoSkinProps): ReactNod ( - - )} - /> + }> + + + {SEEK_TIME} + + } /> Seek backward {SEEK_TIME} seconds @@ -250,17 +241,12 @@ export function MinimalVideoSkinTailwind(props: MinimalVideoSkinProps): ReactNod ( - - )} - /> + }> + + + {SEEK_TIME} + + } /> Seek forward {SEEK_TIME} seconds @@ -274,12 +260,12 @@ export function MinimalVideoSkinTailwind(props: MinimalVideoSkinProps): ReactNod - }> - }> - } /> - } /> + }> + }> + } /> + } /> - } /> + } />
@@ -293,11 +279,7 @@ export function MinimalVideoSkinTailwind(props: MinimalVideoSkinProps): ReactNod
- )} - /> + }> + + + } /> @@ -325,14 +303,10 @@ export function MinimalVideoSkinTailwind(props: MinimalVideoSkinProps): ReactNod ( - - )} - /> + }> + + + } /> @@ -343,14 +317,10 @@ export function MinimalVideoSkinTailwind(props: MinimalVideoSkinProps): ReactNod ( - - )} - /> + }> + + + } /> diff --git a/packages/react/src/presets/video/minimal-skin.tsx b/packages/react/src/presets/video/minimal-skin.tsx index 7310ec64..56ce4de5 100644 --- a/packages/react/src/presets/video/minimal-skin.tsx +++ b/packages/react/src/presets/video/minimal-skin.tsx @@ -43,7 +43,7 @@ const SEEK_TIME = 10; export type MinimalVideoSkinProps = BaseVideoSkinProps; const Button = forwardRef>(function Button({ className, ...props }, ref) { - return - )} - /> + }> + + + + ); if (volumeUnsupported) return muteButton; @@ -137,15 +133,11 @@ export function MinimalVideoSkin(props: MinimalVideoSkinProps): ReactNode { ( - - )} - /> + }> + + + + } /> @@ -156,17 +148,12 @@ export function MinimalVideoSkin(props: MinimalVideoSkinProps): ReactNode { ( - - )} - /> + }> + + + {SEEK_TIME} + + } /> Seek backward {SEEK_TIME} seconds @@ -175,17 +162,12 @@ export function MinimalVideoSkin(props: MinimalVideoSkinProps): ReactNode { ( - - )} - /> + }> + + + {SEEK_TIME} + + } /> Seek forward {SEEK_TIME} seconds @@ -219,11 +201,7 @@ export function MinimalVideoSkin(props: MinimalVideoSkinProps): ReactNode {
- )} - /> + }> + + + } /> @@ -251,14 +225,10 @@ export function MinimalVideoSkin(props: MinimalVideoSkinProps): ReactNode { ( - - )} - /> + }> + + + } /> @@ -269,14 +239,10 @@ export function MinimalVideoSkin(props: MinimalVideoSkinProps): ReactNode { ( - - )} - /> + }> + + + } /> diff --git a/packages/react/src/presets/video/skin.tailwind.tsx b/packages/react/src/presets/video/skin.tailwind.tsx index dc124f5f..4c3fe7bf 100644 --- a/packages/react/src/presets/video/skin.tailwind.tsx +++ b/packages/react/src/presets/video/skin.tailwind.tsx @@ -99,6 +99,10 @@ const SliderFill = forwardRef & { type?: ' ); }); +const SliderBuffer = forwardRef>(function SliderBuffer(props, ref) { + return ; +}); + const SliderThumb = forwardRef & { persistent?: boolean }>(function SliderThumb( { persistent, className, ...props }, ref @@ -148,15 +152,11 @@ function VolumePopover(): ReactNode { const volumeUnsupported = usePlayer((s) => s.volumeAvailability === 'unsupported'); const muteButton = ( - ( - - )} - /> + }> + + + + ); if (volumeUnsupported) return muteButton; @@ -165,9 +165,9 @@ function VolumePopover(): ReactNode { - }> - }> - } /> + }> + }> + } /> } /> @@ -213,15 +213,11 @@ export function VideoSkinTailwind(props: VideoSkinProps): ReactNode { ( - - )} - /> + }> + + + + } /> @@ -232,17 +228,12 @@ export function VideoSkinTailwind(props: VideoSkinProps): ReactNode { ( - - )} - /> + }> + + + {SEEK_TIME} + + } /> Seek backward {SEEK_TIME} seconds @@ -251,17 +242,12 @@ export function VideoSkinTailwind(props: VideoSkinProps): ReactNode { ( - - )} - /> + }> + + + {SEEK_TIME} + + } /> Seek forward {SEEK_TIME} seconds @@ -269,12 +255,12 @@ export function VideoSkinTailwind(props: VideoSkinProps): ReactNode { - }> - }> - } /> - } /> + }> + }> + } /> + } /> - } /> + } />
@@ -286,11 +272,7 @@ export function VideoSkinTailwind(props: VideoSkinProps): ReactNode { - )} - /> + }> + + + } /> @@ -318,14 +296,10 @@ export function VideoSkinTailwind(props: VideoSkinProps): ReactNode { ( - - )} - /> + }> + + + } /> @@ -336,14 +310,10 @@ export function VideoSkinTailwind(props: VideoSkinProps): ReactNode { ( - - )} - /> + }> + + + } /> diff --git a/packages/react/src/presets/video/skin.tsx b/packages/react/src/presets/video/skin.tsx index 96ae04bd..6f3cd966 100644 --- a/packages/react/src/presets/video/skin.tsx +++ b/packages/react/src/presets/video/skin.tsx @@ -43,7 +43,7 @@ const SEEK_TIME = 10; export type VideoSkinProps = BaseVideoSkinProps; const Button = forwardRef>(function Button({ className, ...props }, ref) { - return - )} - /> + }> + + + + ); if (volumeUnsupported) return muteButton; @@ -138,15 +134,11 @@ export function VideoSkin(props: VideoSkinProps): ReactNode { ( - - )} - /> + }> + + + + } /> @@ -157,17 +149,12 @@ export function VideoSkin(props: VideoSkinProps): ReactNode { ( - - )} - /> + }> + + + {SEEK_TIME} + + } /> Seek backward {SEEK_TIME} seconds @@ -176,17 +163,12 @@ export function VideoSkin(props: VideoSkinProps): ReactNode { ( - - )} - /> + }> + + + {SEEK_TIME} + + } /> Seek forward {SEEK_TIME} seconds @@ -212,11 +194,7 @@ export function VideoSkin(props: VideoSkinProps): ReactNode { - )} - /> + }> + + + } /> @@ -244,14 +218,10 @@ export function VideoSkin(props: VideoSkinProps): ReactNode { ( - - )} - /> + }> + + + } /> @@ -262,14 +232,10 @@ export function VideoSkin(props: VideoSkinProps): ReactNode { ( - - )} - /> + }> + + + } />