Component Quality
Purpose
Apply component design principles to maximize reusability and testability. Avoid both extremes: premature abstraction (one-off "reusable" components) and copy-paste blindness.
Universal — separating logic from presentation, effect cleanup, extraction criteria, typed variant systems, and composition over wrapper proliferation apply to any component-based framework. The default Procedure illustrates them with the React + shadcn/ui idiom (cva, cn(), Radix Slot, hooks); the Other stacks section maps each concept to Vue / Svelte / Angular equivalents.
Procedure
Separate logic from presentation
- Pull data fetching, state, and handlers out of the render path — the modern vehicle is a custom hook (step 4), not a wrapper component
- The classic Container/Presentational split earns its keep only when one presentational shell must serve multiple data sources; otherwise a hook is lighter
- Either way the goal is the same: the rendering piece is a pure function of props, trivially testable in Storybook
Place Error Boundaries strategically
- One at the page section level (not per-component, not just at root)
- Wrap third-party widgets that can fail without bringing down the page
- Fallback UI offers a recovery path (reload section, go home)
- This skill decides where boundaries sit in the component tree; the fallback's loading/error/empty UI states belong to
async-ux-states
Audit
useEffect— first "does it need to exist?", then cleanup- Find every effect and check each (search — see Implementation). Many effects shouldn't exist at all ("You Might Not Need an Effect"):
- Deriving / transforming data for render → compute during render, don't effect-and-store
- Responding to a user event → put the logic in the event handler, not an effect
- Fetching that belongs server-side → move to a Server Component / route loader / query library
- For the effects that do belong, three patterns REQUIRE cleanup (missing cleanup = memory leak):
- Timer (
setTimeout,setInterval) - Event listener (
addEventListener) - Subscription (Supabase realtime, WebSocket, custom event bus)
- Timer (
- Find every effect and check each (search — see Implementation). Many effects shouldn't exist at all ("You Might Not Need an Effect"):
Extract custom hooks
- Same
useState+useEffectpattern in 2+ components → extract touseX - Or: even one occurrence is worth extracting if it has test isolation value (e.g., complex state transitions)
- Same
Apply a typed variant system
- Use a typed variant system for components with 3+ visual variants (a conditional class chain > 2 is the signal to switch)
- Merge consumer overrides left-to-right instead of string-concatenating classes — concatenation silently breaks on duplicate utilities (cva +
cn()— see Implementation)
Prefer composition (children / slots) over deep prop chains
- Pass JSX as
children(or named slots) instead of threading props down many levels — cuts prop drilling, and a component that holds state but renderschildrenfrom above re-renders itself without re-rendering those children (same element reference). Composition is the first tool against drilling AND re-renders, before reaching for context or memo. - shadcn/Radix
asChild+<Slot>: when a wrapper only adds styles/behavior to one child, acceptasChildand forward className/refs/props to the consumer's child — avoids<ButtonLink>/<ButtonAnchor>/<ButtonDiv>proliferation. Pattern:<Button asChild><Link href="/x">Go</Link></Button>renders as<a>with button styles.
- Pass JSX as
Decide extraction with explicit criteria
Candidates (OR — any one warrants review):
- Same/similar JSX appears in 2+ places
- File > 500 LOC with multiple concerns
- Section marker comments like
{/* Modal */}{/* Divider */}
Execute extraction (AND — all must hold):
- ① Cleanly expressible as props (no state coupling)
- ② Readability improves after extraction
- → Otherwise: keep inline + add a comment explaining the decision
Write hook tests
- Test hooks' state transitions, not implementation details (
renderHook— see Implementation)
- Test hooks' state transitions, not implementation details (
Anti-patterns
| ❌ Anti-pattern | ✅ Correct |
|---|---|
useEffect(() => setInterval(fn, 1000), []) (no cleanup) |
useEffect(() => { const id = setInterval(...); return () => clearInterval(id) }, []) |
<div className={cn('base', isActive && 'active', size === 'sm' && 'small')} (3+ conditionals) |
Extract to cva({ variants: { state, size } }) |
<ButtonAnchor>, <ButtonLink>, <ButtonDiv> (wrapper proliferation) |
One <Button asChild> + Radix <Slot> |
Premature React.memo on every component |
Profile first; memo only where Profiler shows wasted renders |
800-LOC component with {/* Header */}, {/* Sidebar */}, {/* Modal */} markers |
Extract each marked section (if it passes the AND criteria) |
Severity tiers
| Tier | Examples | Action SLA |
|---|---|---|
| Critical | useEffect with missing cleanup on timer/listener/subscription (memory leak); file > 1000 LOC with no extraction decision |
Fix immediately |
| Major | File 500-1000 LOC mixing 3+ concerns; conditional class chain > 4 without cva; missing cn(internal, className) on shared components |
Fix this sprint |
| Minor | Premature memoization without Profiler evidence; minor naming inconsistencies in custom hooks | Schedule within 2 sprints |
Completion Criteria
- All
useEffectaudited for cleanup - Extraction candidates reviewed and decision recorded (extracted OR comment justifying inline)
- Custom Hooks have tests
-
cn(internal, className)used in shared components for consumer overrides - No file > 500 LOC without extraction-decision comment
- All Critical findings fixed; all Major findings scheduled
Output
- Improved component code: cleanup applied (useEffect cleanup, cn() merging, cva variants)
- Extracted units: hooks in
src/hooks/use<X>.ts, services insrc/lib/<domain>/, each with co-located test file - Inline comments:
// inline because [reason]for explicit non-extraction decisions - Commit format:
refactor(component): <description>for cleanup;feat(hook): add use<X>for new extractions - PR description block:
- Components touched: N - Hooks extracted: N (with tests) - useEffect cleanup added: N - Extraction decisions: extracted X / kept inline Y (with reasons)
Implementation
React + Next.js (default)
- Effect audit: find effects via
grep -rn 'useEffect' src/, then check each - Effect cleanup:
useEffect(() => { ... return cleanup }, [])— timer/listener/subscription - Custom hook extraction:
useXnaming, return value or destructured object - Variants:
class-variance-authority(cva) — minimal signature:const buttonVariants = cva('inline-flex items-center', { variants: { size: { sm: 'h-8 px-3', md: 'h-10 px-4', lg: 'h-12 px-6' }, intent: { primary: 'bg-primary text-primary-foreground', ghost: 'bg-transparent' }, }, defaultVariants: { size: 'md', intent: 'primary' }, }); - Class merging:
cn(internal, className)(clsx + tailwind-merge) - Composition: Radix
<Slot>+asChildprop - Memo:
React.memo,useCallback,useMemo(after Profiler proves cost) - Hook tests:
@testing-library/react'srenderHook
Other stacks
- Vue / Nuxt: composables (
use<Name>) replace custom hooks; cleanup viaonUnmounted(); variants viacva(works in Vue) ortailwind-variants; composition via<slot>+ scoped slots; memo viadefineComponent+<KeepAlive>or computed - SvelteKit: stores or
$state.rawfor shared logic; cleanup inonDestroy(); variants viatailwind-variants; composition via<slot>+ slot props; Svelte 5 fine-grained reactivity reduces memo need - Angular: services for shared logic; cleanup via
ngOnDestroy()ortakeUntilDestroyed(); variants via service or directive; composition via content projection (<ng-content>);OnPushchange detection + signals for performance - Universal: extraction criteria (OR for candidates → AND for execution), file LOC limits, effect cleanup as a categorical rule (timer/listener/subscription) apply identically across all component frameworks
Related skills
code-refactoring— for non-component code changesdesign-system-construction— when extraction targets the shared component librarytest-strategy— write tests for newly-extracted hooks/componentsasync-ux-states— owns the loading/error/empty fallback UI that error boundaries render
Reference
- Key insight encoded: Define variants with
cva(typed, declarative, compound-variant-aware) and always merge consumer overrides withcn(internal, className)left-to-right so consumers can override. Never string-concatenate Tailwind classes — that pattern silently breaks when the same utility appears in both internal and consumer values.