React Router Code Review

React Router Code Review

Quick Reference

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 acceptable during revalidation

Context-Sensitive Rules

Only flag these issues when the specific context applies:

Gates (before reporting any finding)

Run in order. Pass each gate with evidence (paths, line refs, or a one-line quote from code)—not intuition alone.

Gate 1 — Scope the route surface

Pass when: You have repo path(s) to the route module, routes config entry, or layout that owns the behavior under review (write them in your notes before flagging).

Gate 2 — Context-sensitive match

Pass when: For every issue that maps to Context-Sensitive Rules, the Flag ONLY IF condition is satisfied with a one-line rationale tied to the code; for other checklist items, you have a concrete code citation (path + line or short excerpt).

Gate 3 — Non-issue patterns

Pass when: The behavior is not covered by Valid Patterns (Do NOT Flag) for that category.

Gate 4 — Verification protocol

Load and follow review-verification-protocol. Pass when: Its pre-report checklist (and any issue-type subsection that applies) is complete for each finding you will output.

When to Load References

• Reviewing data fetching code → data-loading.md
• Reviewing forms or mutations → mutations.md
• Reviewing error handling → error-handling.md
• Reviewing navigation logic → navigation.md

Review Questions

1. Is data loaded in loaders instead of effects? 2. Are mutations using Form/action patterns? 3. Are there error boundaries at appropriate route levels? 4. Is navigation declarative with Link components? 5. Are pending states properly handled?

Data Loading

Critical Anti-Patterns

1. Using useEffect Instead of Loaders

Problem: Race conditions, loading states, unnecessary client-side fetching.

// BAD - Loading data in useEffect
function UserProfile() {
  const [user, setUser] = useState(null);
  const [loading, setLoading] = useState(true);
  const { userId } = useParams();

  useEffect(() => {
    setLoading(true);
    fetch(`/api/users/${userId}`)
      .then(r => r.json())
      .then(setUser)
      .finally(() => setLoading(false));
  }, [userId]);

  if (loading) return <div>Loading...</div>;
  return <div>{user.name}</div>;
}

// GOOD - Using loader
// Route definition
{
  path: "users/:userId",
  element: <UserProfile />,
  loader: async ({ params }) => {
    const response = await fetch(`/api/users/${params.userId}`);
    if (!response.ok) throw new Response("Not Found", { status: 404 });
    return response.json();
  }
}

// Component
function UserProfile() {
  const user = useLoaderData<User>();
  return <div>{user.name}</div>;
}

2. Unsafe Route Params Access

Problem: Runtime errors from missing or invalid params.

// BAD - No validation
const loader = async ({ params }) => {
  // params.userId could be undefined!
  return fetch(`/api/users/${params.userId}`);
};

// GOOD - Validate params
const loader = async ({ params }) => {
  const userId = params.userId;
  if (!userId) {
    throw new Response("User ID required", { status: 400 });
  }

  // Optional: validate format
  if (!/^\d+$/.test(userId)) {
    throw new Response("Invalid user ID", { status: 400 });
  }

  return fetch(`/api/users/${userId}`);
};

// BETTER - Type-safe with zod
import { z } from "zod";

const ParamsSchema = z.object({
  userId: z.string().regex(/^\d+$/)
});

const loader = async ({ params }) => {
  const { userId } = ParamsSchema.parse(params);
  return fetch(`/api/users/${userId}`);
};

3. Sequential Data Fetching

Problem: Slow page loads when data can be fetched in parallel.

// BAD - Sequential fetching
const loader = async ({ params }) => {
  const user = await fetchUser(params.userId);
  const posts = await fetchPosts(params.userId);
  const comments = await fetchComments(params.userId);

  return { user, posts, comments };
};

// GOOD - Parallel fetching
const loader = async ({ params }) => {
  const [user, posts, comments] = await Promise.all([
    fetchUser(params.userId),
    fetchPosts(params.userId),
    fetchComments(params.userId),
  ]);

  return { user, posts, comments };
};

// BETTER - Using defer for progressive loading
import { defer } from "react-router-dom";

const loader = async ({ params }) => {
  // Critical data - await it
  const user = await fetchUser(params.userId);

  // Non-critical data - defer it
  return defer({
    user,
    posts: fetchPosts(params.userId), // Don't await
    comments: fetchComments(params.userId), // Don't await
  });
};

// Component with Suspense
function UserProfile() {
  const { user, posts, comments } = useLoaderData();

  return (
    <div>
      <h1>{user.name}</h1>

      <Suspense fallback={<div>Loading posts...</div>}>
        <Await resolve={posts}>
          {(posts) => <PostList posts={posts} />}
        </Await>
      </Suspense>

      <Suspense fallback={<div>Loading comments...</div>}>
        <Await resolve={comments}>
          {(comments) => <CommentList comments={comments} />}
        </Await>
      </Suspense>
    </div>
  );
}

4. Not Revalidating After Mutations

Problem: Stale data after updates, manual cache invalidation.

// BAD - Manual refetch
function UserProfile() {
  const user = useLoaderData<User>();
  const [localUser, setLocalUser] = useState(user);

  const handleUpdate = async (data) => {
    await fetch(`/api/users/${user.id}`, {
      method: "PATCH",
      body: JSON.stringify(data),
    });

    // Manual refetch - easy to forget!
    const updated = await fetch(`/api/users/${user.id}`).then(r => r.json());
    setLocalUser(updated);
  };

  return <UserForm user={localUser} onSubmit={handleUpdate} />;
}

// GOOD - Automatic revalidation
// Action automatically triggers loader revalidation
const action = async ({ request, params }) => {
  const formData = await request.formData();
  const response = await fetch(`/api/users/${params.userId}`, {
    method: "PATCH",
    body: formData,
  });

  if (!response.ok) throw new Response("Update failed", { status: 400 });
  return redirect(`/users/${params.userId}`);
};

function UserProfile() {
  const user = useLoaderData<User>();
  // No useState needed - loader data auto-revalidates
  return <UserForm user={user} />;
}

5. Missing Error Handling in Loaders

Problem: Uncaught errors, poor user experience.

// BAD - No error handling
const loader = async ({ params }) => {
  const response = await fetch(`/api/users/${params.userId}`);
  return response.json(); // What if response is 404 or 500?
};

// GOOD - Proper error handling
const loader = async ({ params }) => {
  const response = await fetch(`/api/users/${params.userId}`);

  if (!response.ok) {
    throw new Response("User not found", {
      status: response.status,
      statusText: response.statusText
    });
  }

  return response.json();
};

// BETTER - Detailed error responses
const loader = async ({ params }) => {
  try {
    const response = await fetch(`/api/users/${params.userId}`);

    if (response.status === 404) {
      throw new Response("User not found", { status: 404 });
    }

    if (response.status === 403) {
      throw new Response("You don't have permission to view this user", {
        status: 403
      });
    }

    if (!response.ok) {
      throw new Response("Failed to load user", {
        status: response.status
      });
    }

    return response.json();
  } catch (error) {
    if (error instanceof Response) throw error;

    // Network error or other unexpected error
    throw new Response("Network error - please try again", {
      status: 503
    });
  }
};

6. Accessing Search Params Without URLSearchParams

Problem: Manual string parsing, inconsistent handling.

// BAD - Manual parsing
const loader = async ({ request }) => {
  const url = new URL(request.url);
  const search = url.search.slice(1); // Remove '?'
  const page = search.split('&').find(p => p.startsWith('page='))?.split('=')[1] || '1';

  return fetchUsers(parseInt(page));
};

// GOOD - Using URLSearchParams
const loader = async ({ request }) => {
  const url = new URL(request.url);
  const page = url.searchParams.get('page') || '1';

  return fetchUsers(parseInt(page, 10));
};

// BETTER - Type-safe search params
import { z } from "zod";

const SearchParamsSchema = z.object({
  page: z.coerce.number().min(1).default(1),
  sort: z.enum(['name', 'date', 'popular']).default('name'),
  filter: z.string().optional(),
});

const loader = async ({ request }) => {
  const url = new URL(request.url);
  const rawParams = Object.fromEntries(url.searchParams);
  const { page, sort, filter } = SearchParamsSchema.parse(rawParams);

  return fetchUsers({ page, sort, filter });
};

Review Questions

1. Is all route data loaded via loaders, not useEffect? 2. Are route params validated before use? 3. Are independent data fetches executed in parallel? 4. Is defer() used for non-critical data? 5. Do loaders throw proper Response objects on errors? 6. Are search params parsed with URLSearchParams?

Error Handling

Critical Anti-Patterns

1. Missing Error Boundaries

Problem: Entire app crashes on route errors, poor UX.

// BAD - No error handling
const router = createBrowserRouter([
  {
    path: "/",
    element: <Root />,
    children: [
      {
        path: "users/:userId",
        element: <UserProfile />,
        loader: async ({ params }) => {
          // If this fails, entire app shows error
          return fetch(`/api/users/${params.userId}`).then(r => r.json());
        }
      }
    ]
  }
]);

// GOOD - Error boundaries at route level
const router = createBrowserRouter([
  {
    path: "/",
    element: <Root />,
    errorElement: <RootErrorBoundary />, // Catch all errors
    children: [
      {
        path: "users/:userId",
        element: <UserProfile />,
        errorElement: <UserErrorBoundary />, // Scoped error handling
        loader: async ({ params }) => {
          const response = await fetch(`/api/users/${params.userId}`);
          if (!response.ok) {
            throw new Response("User not found", { status: 404 });
          }
          return response.json();
        }
      }
    ]
  }
]);

// Error boundary component
function UserErrorBoundary() {
  const error = useRouteError();

  if (isRouteErrorResponse(error)) {
    if (error.status === 404) {
      return <div>User not found</div>;
    }
    if (error.status === 403) {
      return <div>You don't have permission to view this user</div>;
    }
  }

  return <div>Something went wrong loading this user</div>;
}

2. Not Using isRouteErrorResponse

Problem: Unsafe error access, runtime errors in error handlers.

// BAD - Unsafe error access
function ErrorBoundary() {
  const error = useRouteError();

  // error might not have these properties!
  return (
    <div>
      <h1>Error {error.status}</h1>
      <p>{error.statusText}</p>
      <p>{error.data}</p>
    </div>
  );
}

// GOOD - Type-safe error checking
import { isRouteErrorResponse } from 'react-router-dom';

function ErrorBoundary() {
  const error = useRouteError();

  if (isRouteErrorResponse(error)) {
    // Now we know error has status, statusText, data
    return (
      <div>
        <h1>Error {error.status}</h1>
        <p>{error.statusText}</p>
        {typeof error.data === 'string' && <p>{error.data}</p>}
      </div>
    );
  }

  if (error instanceof Error) {
    return (
      <div>
        <h1>Unexpected Error</h1>
        <p>{error.message}</p>
        {import.meta.env.DEV && <pre>{error.stack}</pre>}
      </div>
    );
  }

  return <div>An unknown error occurred</div>;
}

3. Throwing Raw Errors Instead of Responses

Problem: Missing status codes, inconsistent error format.

// BAD - Throwing raw errors
const loader = async ({ params }) => {
  const user = await db.user.findUnique({
    where: { id: params.userId }
  });

  if (!user) {
    throw new Error('User not found'); // No status code!
  }

  if (!user.isPublic && !currentUser) {
    throw new Error('Unauthorized'); // Should be 403, not 500!
  }

  return user;
};

// GOOD - Throwing Response objects
const loader = async ({ params }) => {
  const user = await db.user.findUnique({
    where: { id: params.userId }
  });

  if (!user) {
    throw new Response('User not found', { status: 404 });
  }

  if (!user.isPublic && !currentUser) {
    throw new Response('You must be logged in to view this profile', {
      status: 403
    });
  }

  return user;
};

// BETTER - Using json() helper for structured errors
import { json } from 'react-router-dom';

const loader = async ({ params }) => {
  const user = await db.user.findUnique({
    where: { id: params.userId }
  });

  if (!user) {
    throw json(
      { message: 'User not found', userId: params.userId },
      { status: 404 }
    );
  }

  if (!user.isPublic && !currentUser) {
    throw json(
      { message: 'Login required', redirectTo: `/login?return=/users/${params.userId}` },
      { status: 403 }
    );
  }

  return user;
};

// Error boundary using structured error
function ErrorBoundary() {
  const error = useRouteError();

  if (isRouteErrorResponse(error)) {
    if (error.status === 403 && error.data?.redirectTo) {
      return (
        <div>
          <p>{error.data.message}</p>
          <Link to={error.data.redirectTo}>Log in</Link>
        </div>
      );
    }

    if (error.status === 404) {
      return <div>{error.data.message}</div>;
    }
  }

  return <div>Something went wrong</div>;
}

4. Not Differentiating Error Types

Problem: Same handling for different errors, poor UX.

// BAD - Generic error handling
function ErrorBoundary() {
  const error = useRouteError();

  // Everything gets same treatment
  return <div>Error: {String(error)}</div>;
}

// GOOD - Specific handling per error type
function ErrorBoundary() {
  const error = useRouteError();

  // Network/fetch errors
  if (error instanceof TypeError && error.message.includes('fetch')) {
    return (
      <div className="error">
        <h1>Network Error</h1>
        <p>Unable to connect to the server. Please check your connection.</p>
        <button onClick={() => window.location.reload()}>Retry</button>
      </div>
    );
  }

  // Route errors
  if (isRouteErrorResponse(error)) {
    if (error.status === 404) {
      return (
        <div className="error">
          <h1>Page Not Found</h1>
          <p>The page you're looking for doesn't exist.</p>
          <Link to="/">Go home</Link>
        </div>
      );
    }

    if (error.status === 403) {
      return (
        <div className="error">
          <h1>Access Denied</h1>
          <p>You don't have permission to access this resource.</p>
          <Link to="/login">Log in</Link>
        </div>
      );
    }

    if (error.status === 500) {
      return (
        <div className="error">
          <h1>Server Error</h1>
          <p>Something went wrong on our end. Please try again later.</p>
        </div>
      );
    }

    // Generic HTTP error
    return (
      <div className="error">
        <h1>Error {error.status}</h1>
        <p>{error.statusText}</p>
      </div>
    );
  }

  // JavaScript errors
  if (error instanceof Error) {
    return (
      <div className="error">
        <h1>Unexpected Error</h1>
        <p>{error.message}</p>
        {import.meta.env.DEV && (
          <details>
            <summary>Stack trace</summary>
            <pre>{error.stack}</pre>
          </details>
        )}
      </div>
    );
  }

  // Unknown error
  return (
    <div className="error">
      <h1>Unknown Error</h1>
      <p>An unexpected error occurred.</p>
    </div>
  );
}

5. Missing Root Error Boundary

Problem: Uncaught errors bubble to browser, blank screen.

// BAD - No root error boundary
const router = createBrowserRouter([
  {
    path: "/",
    element: <Root />,
    children: [
      // children routes...
    ]
  }
]);

// GOOD - Root error boundary catches everything
const router = createBrowserRouter([
  {
    path: "/",
    element: <Root />,
    errorElement: <RootErrorBoundary />,
    children: [
      {
        path: "users",
        element: <Users />,
        errorElement: <UsersErrorBoundary />, // Scoped
      },
      // other routes...
    ]
  }
]);

// Root error boundary with full-page layout
function RootErrorBoundary() {
  const error = useRouteError();

  return (
    <html lang="en">
      <head>
        <title>Error - My App</title>
        <meta charSet="utf-8" />
        <meta name="viewport" content="width=device-width,initial-scale=1" />
      </head>
      <body>
        <div className="error-page">
          <header>
            <Link to="/">
              <img src="/logo.png" alt="My App" />
            </Link>
          </header>
          <main>
            {isRouteErrorResponse(error) ? (
              <>
                <h1>Error {error.status}</h1>
                <p>{error.statusText}</p>
              </>
            ) : error instanceof Error ? (
              <>
                <h1>Unexpected Error</h1>
                <p>{error.message}</p>
              </>
            ) : (
              <h1>Unknown Error</h1>
            )}
            <Link to="/">Go back home</Link>
          </main>
        </div>
      </body>
    </html>
  );
}

6. Not Logging Errors

Problem: No visibility into production errors, hard to debug.

// BAD - Silent errors
function ErrorBoundary() {
  const error = useRouteError();
  return <div>Error occurred</div>;
}

// GOOD - Errors logged to monitoring service
function ErrorBoundary() {
  const error = useRouteError();

  React.useEffect(() => {
    // Log to error tracking service
    if (isRouteErrorResponse(error)) {
      logError({
        type: 'RouteError',
        status: error.status,
        statusText: error.statusText,
        data: error.data,
      });
    } else if (error instanceof Error) {
      logError({
        type: 'JavaScriptError',
        message: error.message,
        stack: error.stack,
      });
    } else {
      logError({
        type: 'UnknownError',
        error: String(error),
      });
    }
  }, [error]);

  return <ErrorDisplay error={error} />;
}

// BETTER - Centralized error logging
function useErrorLogging(error: unknown) {
  React.useEffect(() => {
    // Don't log in development
    if (import.meta.env.DEV) return;

    // Send to monitoring service (Sentry, etc.)
    if (isRouteErrorResponse(error)) {
      window.analytics?.track('Route Error', {
        status: error.status,
        statusText: error.statusText,
        path: window.location.pathname,
      });
    } else if (error instanceof Error) {
      window.analytics?.track('JavaScript Error', {
        message: error.message,
        stack: error.stack,
        path: window.location.pathname,
      });
    }
  }, [error]);
}

function ErrorBoundary() {
  const error = useRouteError();
  useErrorLogging(error);

  return <ErrorDisplay error={error} />;
}

Review Questions

1. Does every route have an errorElement? 2. Is isRouteErrorResponse used to check error types? 3. Are loaders/actions throwing Response objects with status codes? 4. Are different error types handled differently? 5. Is there a root error boundary? 6. Are errors logged to a monitoring service?

Mutations

Critical Anti-Patterns

1. Manual Form Submission with fetch

Problem: Missing navigation state, manual revalidation, no progressive enhancement.

// BAD - Manual fetch in handler
function CreateUser() {
  const [loading, setLoading] = useState(false);
  const navigate = useNavigate();

  const handleSubmit = async (e) => {
    e.preventDefault();
    setLoading(true);

    const formData = new FormData(e.target);
    const response = await fetch('/api/users', {
      method: 'POST',
      body: JSON.stringify(Object.fromEntries(formData)),
    });

    if (response.ok) {
      navigate('/users');
    } else {
      alert('Error creating user');
    }
    setLoading(false);
  };

  return (
    <form onSubmit={handleSubmit}>
      <input name="name" />
      <button disabled={loading}>
        {loading ? 'Creating...' : 'Create'}
      </button>
    </form>
  );
}

// GOOD - Using Form and action
// Route definition
{
  path: "users/new",
  element: <CreateUser />,
  action: async ({ request }) => {
    const formData = await request.formData();
    const response = await fetch('/api/users', {
      method: 'POST',
      body: formData,
    });

    if (!response.ok) {
      return { error: 'Failed to create user' };
    }

    return redirect('/users');
  }
}

// Component
import { Form, useNavigation, useActionData } from 'react-router-dom';

function CreateUser() {
  const navigation = useNavigation();
  const actionData = useActionData();
  const isSubmitting = navigation.state === 'submitting';

  return (
    <Form method="post">
      <input name="name" />
      {actionData?.error && <div className="error">{actionData.error}</div>}
      <button disabled={isSubmitting}>
        {isSubmitting ? 'Creating...' : 'Create'}
      </button>
    </Form>
  );
}

2. Using Form When useFetcher is Appropriate

Problem: Unnecessary navigation, losing current page state.

// BAD - Form causes navigation away from current page
function TodoList() {
  const todos = useLoaderData<Todo[]>();

  return (
    <div>
      {todos.map(todo => (
        <div key={todo.id}>
          <span>{todo.text}</span>
          {/* This will navigate away! */}
          <Form method="post" action={`/todos/${todo.id}/toggle`}>
            <button>Toggle</button>
          </Form>
        </div>
      ))}
    </div>
  );
}

// GOOD - useFetcher stays on current page
import { useFetcher } from 'react-router-dom';

function TodoList() {
  const todos = useLoaderData<Todo[]>();

  return (
    <div>
      {todos.map(todo => (
        <TodoItem key={todo.id} todo={todo} />
      ))}
    </div>
  );
}

function TodoItem({ todo }) {
  const fetcher = useFetcher();

  // Optimistic UI - show state immediately
  const isComplete = fetcher.formData
    ? fetcher.formData.get('complete') === 'true'
    : todo.complete;

  return (
    <div>
      <span style={{ textDecoration: isComplete ? 'line-through' : 'none' }}>
        {todo.text}
      </span>
      <fetcher.Form method="post" action={`/todos/${todo.id}/toggle`}>
        <input type="hidden" name="complete" value={String(!isComplete)} />
        <button disabled={fetcher.state !== 'idle'}>
          {fetcher.state !== 'idle' ? 'Toggling...' : 'Toggle'}
        </button>
      </fetcher.Form>
    </div>
  );
}

3. Not Validating Action Data

Problem: Runtime errors, poor error messages.

// BAD - No validation
const action = async ({ request }) => {
  const formData = await request.formData();

  // What if name is missing or invalid?
  const name = formData.get('name');
  const email = formData.get('email');

  await createUser({ name, email });
  return redirect('/users');
};

// GOOD - Validation with helpful errors
const action = async ({ request }) => {
  const formData = await request.formData();
  const name = formData.get('name');
  const email = formData.get('email');

  const errors = {};

  if (!name || typeof name !== 'string' || name.trim().length === 0) {
    errors.name = 'Name is required';
  }

  if (!email || typeof email !== 'string') {
    errors.email = 'Email is required';
  } else if (!/^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email)) {
    errors.email = 'Invalid email format';
  }

  if (Object.keys(errors).length > 0) {
    return { errors };
  }

  await createUser({ name, email });
  return redirect('/users');
};

// BETTER - Schema validation
import { z } from 'zod';

const CreateUserSchema = z.object({
  name: z.string().min(1, 'Name is required').max(100),
  email: z.string().email('Invalid email format'),
});

const action = async ({ request }) => {
  const formData = await request.formData();
  const data = Object.fromEntries(formData);

  try {
    const validated = CreateUserSchema.parse(data);
    await createUser(validated);
    return redirect('/users');
  } catch (error) {
    if (error instanceof z.ZodError) {
      return {
        errors: error.flatten().fieldErrors
      };
    }
    throw error;
  }
};

// Component using validation errors
function CreateUser() {
  const actionData = useActionData<{ errors?: Record<string, string[]> }>();

  return (
    <Form method="post">
      <div>
        <input name="name" />
        {actionData?.errors?.name && (
          <span className="error">{actionData.errors.name[0]}</span>
        )}
      </div>
      <div>
        <input name="email" type="email" />
        {actionData?.errors?.email && (
          <span className="error">{actionData.errors.email[0]}</span>
        )}
      </div>
      <button>Create User</button>
    </Form>
  );
}

4. Missing Optimistic UI

Problem: Slow perceived performance, no immediate feedback.

// BAD - No optimistic update
function LikeButton({ postId, liked }: { postId: string; liked: boolean }) {
  const fetcher = useFetcher();

  return (
    <fetcher.Form method="post" action={`/posts/${postId}/like`}>
      <button>
        {/* Only updates after server responds */}
        {liked ? '❤️' : '🤍'}
      </button>
    </fetcher.Form>
  );
}

// GOOD - Optimistic UI
function LikeButton({ postId, liked }: { postId: string; liked: boolean }) {
  const fetcher = useFetcher();

  // Show optimistic state immediately
  const optimisticLiked = fetcher.formData
    ? fetcher.formData.get('liked') === 'true'
    : liked;

  return (
    <fetcher.Form method="post" action={`/posts/${postId}/like`}>
      <input type="hidden" name="liked" value={String(!optimisticLiked)} />
      <button disabled={fetcher.state !== 'idle'}>
        {optimisticLiked ? '❤️' : '🤍'}
      </button>
    </fetcher.Form>
  );
}

// BETTER - Optimistic UI with count
function LikeButton({
  postId,
  liked,
  likeCount
}: {
  postId: string;
  liked: boolean;
  likeCount: number;
}) {
  const fetcher = useFetcher();

  const optimisticLiked = fetcher.formData
    ? fetcher.formData.get('liked') === 'true'
    : liked;

  const optimisticCount = fetcher.formData
    ? optimisticLiked
      ? likeCount + 1
      : likeCount - 1
    : likeCount;

  return (
    <fetcher.Form method="post" action={`/posts/${postId}/like`}>
      <input type="hidden" name="liked" value={String(!optimisticLiked)} />
      <button disabled={fetcher.state !== 'idle'}>
        {optimisticLiked ? '❤️' : '🤍'} {optimisticCount}
      </button>
    </fetcher.Form>
  );
}

5. Not Handling Action Errors

Problem: Silent failures, poor error UX.

// BAD - No error handling
const action = async ({ request }) => {
  const formData = await request.formData();
  // If this throws, user sees error boundary
  await createUser(Object.fromEntries(formData));
  return redirect('/users');
};

// GOOD - Graceful error handling
const action = async ({ request }) => {
  const formData = await request.formData();

  try {
    await createUser(Object.fromEntries(formData));
    return redirect('/users');
  } catch (error) {
    // Return error to show in form, not error boundary
    if (error instanceof Error) {
      return { error: error.message };
    }
    return { error: 'An unexpected error occurred' };
  }
};

// BETTER - Typed errors with status
const action = async ({ request }) => {
  const formData = await request.formData();

  try {
    await createUser(Object.fromEntries(formData));
    return redirect('/users');
  } catch (error) {
    if (error instanceof Response) {
      // API returned error response
      const body = await error.json();
      return { error: body.message, status: error.status };
    }

    if (error instanceof Error) {
      return { error: error.message };
    }

    return { error: 'An unexpected error occurred' };
  }
};

// Component showing errors
function CreateUser() {
  const actionData = useActionData<{ error?: string; status?: number }>();

  return (
    <div>
      {actionData?.error && (
        <div className={actionData.status === 400 ? 'warning' : 'error'}>
          {actionData.error}
        </div>
      )}
      <Form method="post">
        {/* form fields */}
      </Form>
    </div>
  );
}

6. Action Without Intent

Problem: Multiple actions in one endpoint, unclear intent.

// BAD - Multiple actions in one action function
const action = async ({ request }) => {
  const formData = await request.formData();
  const action = formData.get('_action');

  if (action === 'create') {
    // create logic
  } else if (action === 'update') {
    // update logic
  } else if (action === 'delete') {
    // delete logic
  }

  return redirect('/users');
};

// GOOD - Separate action routes
// Route definition
{
  path: "users",
  children: [
    {
      path: "new",
      element: <CreateUser />,
      action: createUserAction,
    },
    {
      path: ":userId/edit",
      element: <EditUser />,
      action: updateUserAction,
    },
    {
      path: ":userId/delete",
      action: deleteUserAction,
    }
  ]
}

// ACCEPTABLE - Multiple intents with clear intent field
const action = async ({ request }) => {
  const formData = await request.formData();
  const intent = formData.get('intent');

  switch (intent) {
    case 'archive':
      return handleArchive(formData);
    case 'unarchive':
      return handleUnarchive(formData);
    default:
      throw new Response('Invalid intent', { status: 400 });
  }
};

// Component making intent clear
<fetcher.Form method="post">
  <input type="hidden" name="intent" value="archive" />
  <button>Archive</button>
</fetcher.Form>

Review Questions

1. Are mutations using Form/fetcher.Form instead of manual fetch? 2. Is useFetcher used for actions that shouldn't navigate? 3. Are action inputs validated before processing? 4. Are optimistic UI updates shown for immediate feedback? 5. Do actions handle and return errors gracefully? 6. Is action intent clear and single-purpose?

Navigation

Critical Anti-Patterns

1. Using navigate() Instead of Link

Problem: Missing accessibility, no progressive enhancement, can't open in new tab.

// BAD - navigate() for user-initiated navigation
function UserCard({ userId }: { userId: string }) {
  const navigate = useNavigate();

  return (
    <div onClick={() => navigate(`/users/${userId}`)}>
      <h3>User {userId}</h3>
    </div>
  );
}

// Problems:
// - Can't right-click to open in new tab
// - Can't Cmd+Click to open in new tab
// - Screen readers don't know it's a link
// - No keyboard navigation

// GOOD - Use Link for navigation
function UserCard({ userId }: { userId: string }) {
  return (
    <Link to={`/users/${userId}`} className="user-card">
      <h3>User {userId}</h3>
    </Link>
  );
}

// Benefits:
// - Right-click works
// - Cmd/Ctrl+Click works
// - Accessible to screen readers
// - Tab navigation works
// - Shows URL on hover

2. Imperative Navigation in Render

Problem: Navigation happens during render, causes infinite loops.

// BAD - navigate() during render
function ProtectedRoute({ children }: { children: React.ReactNode }) {
  const user = useLoaderData<User | null>();
  const navigate = useNavigate();

  if (!user) {
    navigate('/login'); // BAD: navigate during render!
    return null;
  }

  return <>{children}</>;
}

// GOOD - Navigate in effect
function ProtectedRoute({ children }: { children: React.ReactNode }) {
  const user = useLoaderData<User | null>();
  const navigate = useNavigate();

  React.useEffect(() => {
    if (!user) {
      navigate('/login');
    }
  }, [user, navigate]);

  if (!user) {
    return <div>Redirecting...</div>;
  }

  return <>{children}</>;
}

// BETTER - Handle in loader
const loader = async ({ request }) => {
  const user = await getUser(request);

  if (!user) {
    // Redirect before component renders
    throw redirect('/login');
  }

  return user;
};

3. Missing Pending UI States

Problem: No feedback during navigation, feels broken.

// BAD - No loading state
function UserList() {
  const users = useLoaderData<User[]>();

  return (
    <div>
      <h1>Users</h1>
      <ul>
        {users.map(user => (
          <li key={user.id}>
            <Link to={`/users/${user.id}`}>{user.name}</Link>
          </li>
        ))}
      </ul>
    </div>
  );
}

// User clicks link, nothing happens for 2 seconds, then page changes
// Bad UX!

// GOOD - Show loading state
import { useNavigation } from 'react-router-dom';

function UserList() {
  const users = useLoaderData<User[]>();
  const navigation = useNavigation();

  return (
    <div>
      <h1>Users</h1>
      {navigation.state === 'loading' && (
        <div className="loading-bar" />
      )}
      <ul className={navigation.state === 'loading' ? 'opacity-50' : ''}>
        {users.map(user => (
          <li key={user.id}>
            <Link to={`/users/${user.id}`}>{user.name}</Link>
          </li>
        ))}
      </ul>
    </div>
  );
}

// BETTER - Global loading indicator
function Root() {
  const navigation = useNavigation();

  return (
    <div>
      {navigation.state !== 'idle' && (
        <div className="global-loading-bar">
          Loading...
        </div>
      )}

      <nav>
        <Link to="/">Home</Link>
        <Link to="/users">Users</Link>
      </nav>

      <main className={navigation.state === 'loading' ? 'loading' : ''}>
        <Outlet />
      </main>
    </div>
  );
}

4. Not Using NavLink for Active Styles

Problem: Manual active state management, inconsistent UI.

// BAD - Manual active state
function Navigation() {
  const location = useLocation();

  return (
    <nav>
      <Link
        to="/"
        className={location.pathname === '/' ? 'active' : ''}
      >
        Home
      </Link>
      <Link
        to="/users"
        className={location.pathname.startsWith('/users') ? 'active' : ''}
      >
        Users
      </Link>
      <Link
        to="/settings"
        className={location.pathname === '/settings' ? 'active' : ''}
      >
        Settings
      </Link>
    </nav>
  );
}

// GOOD - NavLink with className function
import { NavLink } from 'react-router-dom';

function Navigation() {
  return (
    <nav>
      <NavLink
        to="/"
        end // Only match exact path
        className={({ isActive }) => isActive ? 'active' : ''}
      >
        Home
      </NavLink>
      <NavLink
        to="/users"
        className={({ isActive }) => isActive ? 'active' : ''}
      >
        Users
      </NavLink>
      <NavLink
        to="/settings"
        className={({ isActive }) => isActive ? 'active' : ''}
      >
        Settings
      </NavLink>
    </nav>
  );
}

// BETTER - NavLink with style function
function Navigation() {
  const activeStyle = {
    fontWeight: 'bold',
    color: 'var(--primary)',
    borderBottom: '2px solid var(--primary)',
  };

  return (
    <nav>
      <NavLink
        to="/"
        end
        style={({ isActive }) => isActive ? activeStyle : undefined}
      >
        Home
      </NavLink>
      <NavLink
        to="/users"
        style={({ isActive }) => isActive ? activeStyle : undefined}
      >
        Users
      </NavLink>
    </nav>
  );
}

5. Not Preserving Search Params on Navigation

Problem: Lost state, broken URLs, poor UX.

// BAD - Navigation loses search params
function UserFilters() {
  return (
    <div>
      {/* Current URL: /users?sort=name&filter=active */}
      {/* After clicking, URL becomes: /users?sort=date (filter lost!) */}
      <Link to="/users?sort=date">Sort by date</Link>
    </div>
  );
}

// GOOD - Preserve existing search params
function UserFilters() {
  const [searchParams] = useSearchParams();

  const getSortLink = (sort: string) => {
    const params = new URLSearchParams(searchParams);
    params.set('sort', sort);
    return `/users?${params.toString()}`;
  };

  return (
    <div>
      <Link to={getSortLink('date')}>Sort by date</Link>
      <Link to={getSortLink('name')}>Sort by name</Link>
    </div>
  );
}

// BETTER - Reusable hook
function useSearchParamsWithPreserve() {
  const [searchParams, setSearchParams] = useSearchParams();

  const updateSearchParam = React.useCallback(
    (key: string, value: string | null) => {
      setSearchParams(prev => {
        const params = new URLSearchParams(prev);
        if (value === null) {
          params.delete(key);
        } else {
          params.set(key, value);
        }
        return params;
      });
    },
    [setSearchParams]
  );

  return [searchParams, updateSearchParam] as const;
}

function UserFilters() {
  const [searchParams, updateSearchParam] = useSearchParamsWithPreserve();

  return (
    <div>
      <button onClick={() => updateSearchParam('sort', 'date')}>
        Sort by date
      </button>
      <button onClick={() => updateSearchParam('sort', 'name')}>
        Sort by name
      </button>
    </div>
  );
}

6. Blocking Navigation Without Confirmation

Problem: Lost unsaved changes, data loss.

// BAD - No confirmation on navigation
function EditUser() {
  const [formData, setFormData] = useState({});
  const [isDirty, setIsDirty] = useState(false);

  // User can navigate away and lose changes!
  return (
    <form>
      <input
        onChange={(e) => {
          setFormData({ ...formData, name: e.target.value });
          setIsDirty(true);
        }}
      />
    </form>
  );
}

// GOOD - Block navigation with confirmation
import { useBlocker } from 'react-router-dom';

function EditUser() {
  const [formData, setFormData] = useState({});
  const [isDirty, setIsDirty] = useState(false);

  // Block navigation if form is dirty
  const blocker = useBlocker(
    ({ currentLocation, nextLocation }) =>
      isDirty && currentLocation.pathname !== nextLocation.pathname
  );

  return (
    <>
      {blocker.state === 'blocked' && (
        <div className="modal">
          <p>You have unsaved changes. Are you sure you want to leave?</p>
          <button onClick={() => blocker.proceed()}>Leave</button>
          <button onClick={() => blocker.reset()}>Stay</button>
        </div>
      )}

      <form>
        <input
          onChange={(e) => {
            setFormData({ ...formData, name: e.target.value });
            setIsDirty(true);
          }}
        />
      </form>
    </>
  );
}

// BETTER - Also handle browser navigation
function EditUser() {
  const [formData, setFormData] = useState({});
  const [isDirty, setIsDirty] = useState(false);

  const blocker = useBlocker(
    ({ currentLocation, nextLocation }) =>
      isDirty && currentLocation.pathname !== nextLocation.pathname
  );

  // Handle browser back/forward, refresh, close
  React.useEffect(() => {
    const handleBeforeUnload = (e: BeforeUnloadEvent) => {
      if (isDirty) {
        e.preventDefault();
        e.returnValue = ''; // Required for Chrome
      }
    };

    window.addEventListener('beforeunload', handleBeforeUnload);
    return () => window.removeEventListener('beforeunload', handleBeforeUnload);
  }, [isDirty]);

  return (
    <>
      {blocker.state === 'blocked' && (
        <ConfirmationModal
          onConfirm={() => blocker.proceed()}
          onCancel={() => blocker.reset()}
        />
      )}
      <form>{/* form fields */}</form>
    </>
  );
}

7. Not Using Relative Paths

Problem: Brittle routes, hard to refactor.

// BAD - Absolute paths everywhere
// Route: /projects/:projectId/tasks/:taskId

function TaskDetail() {
  const { projectId, taskId } = useParams();

  return (
    <div>
      <Link to={`/projects/${projectId}/tasks`}>Back to tasks</Link>
      <Link to={`/projects/${projectId}/tasks/${taskId}/edit`}>Edit</Link>
      <Link to={`/projects/${projectId}`}>Back to project</Link>
    </div>
  );
}

// If you change the route structure, all these links break!

// GOOD - Relative paths
function TaskDetail() {
  return (
    <div>
      {/* Go up one level */}
      <Link to="..">Back to tasks</Link>

      {/* Stay at current level, append /edit */}
      <Link to="edit">Edit</Link>

      {/* Go up two levels */}
      <Link to="../..">Back to project</Link>
    </div>
  );
}

// BETTER - Mix relative and absolute as appropriate
function TaskDetail() {
  const { projectId } = useParams();

  return (
    <div>
      {/* Relative for sibling/parent routes */}
      <Link to="..">Back to tasks</Link>
      <Link to="edit">Edit</Link>

      {/* Absolute for cross-section navigation */}
      <Link to="/">Home</Link>
      <Link to="/settings">Settings</Link>

      {/* Template when you need params */}
      <Link to={`/projects/${projectId}/settings`}>Project Settings</Link>
    </div>
  );
}

Review Questions

1. Are Links used for navigation instead of navigate()? 2. Is navigate() only called in effects or handlers, not render? 3. Are pending states shown during navigation? 4. Is NavLink used for navigation with active states? 5. Are search params preserved when updating URLs? 6. Are unsaved changes protected with useBlocker? 7. Are relative paths used within route hierarchies?