react-router-code-review

# React Router Code Review

## Quick Reference

| Issue Type | Reference |
|------------|-----------|
| useEffect for data, missing loaders, params | [references/data-loading.md](references/data-loading.md) |
| Form vs useFetcher, action patterns | [references/mutations.md](references/mutations.md) |
| Missing error boundaries, errorElement | [references/error-handling.md](references/error-handling.md) |
| navigate() vs Link, pending states | [references/navigation.md](references/navigation.md) |

## Review Checklist

- [ ] Data loaded via `loader` not `useEffect`
- [ ] Route params accessed type-safely with validation
- [ ] Using `defer()` for parallel data fetching when appropriate
- [ ] Mutations use `<Form>` or `useFetcher` not manual fetch
- [ ] Actions handle both success and error cases
- [ ] Error boundaries with `errorElement` on routes
- [ ] Using `isRouteErrorResponse()` to check error types
- [ ] Navigation uses `<Link>` over `navigate()` where possible
- [ ] Pending states shown via `useNavigation()` or `fetcher.state`
- [ ] No navigation in render (only in effects or handlers)

## Valid Patterns (Do NOT Flag)

These patterns are correct React Router usage - do not report as issues:

- **useEffect for client-only data** - Loaders run server-side; localStorage, window dimensions, and browser APIs must use useEffect
- **navigate() in event handlers** - Link is for declarative navigation; navigate() is correct for imperative navigation in callbacks/handlers
- **Type annotation on loader data** - `useLoaderData<typeof loader>()` is a type annotation, not a type assertion
- **Empty errorElement at route level** - Route may intentionally rely on parent error boundary
- **Form without action prop** - Posts to current URL by convention; explicit action is optional
- **loader returning null** - Valid when data may not exist; null is a legitimate loader return value
- **Using fetcher.data without checking fetcher.state** - May be intentional when stale data is ac