jaykim88/component-quality

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

1. 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

2. 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

3. 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)

4. Extract custom hooks

   • Same useState + useEffect pattern in 2+ components → extract to useX
   • Or: even one occurrence is worth extracting if it has test isolation value (e.g., complex state transitions)

5. 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)

6. 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 renders children from 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, accept asChild and 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.

7. 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

8. Write hook tests

   • Test hooks' state transitions, not implementation details (renderHook — see Implementation)

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 useEffect audited 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 in src/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: useX naming, 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> + asChild prop
• Memo: React.memo, useCallback, useMemo (after Profiler proves cost)
• Hook tests: @testing-library/react's renderHook

Other stacks

• Vue / Nuxt: composables (use<Name>) replace custom hooks; cleanup via onUnmounted(); variants via cva (works in Vue) or tailwind-variants; composition via <slot> + scoped slots; memo via defineComponent + <KeepAlive> or computed
• SvelteKit: stores or $state.raw for shared logic; cleanup in onDestroy(); variants via tailwind-variants; composition via <slot> + slot props; Svelte 5 fine-grained reactivity reduces memo need
• Angular: services for shared logic; cleanup via ngOnDestroy() or takeUntilDestroyed(); variants via service or directive; composition via content projection (<ng-content>); OnPush change 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 changes
• design-system-construction — when extraction targets the shared component library
• test-strategy — write tests for newly-extracted hooks/components
• async-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 with cn(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.