David Dunn

Senior Software Developer


Back to Blog

Common React Hook Anti-Patterns (And What To Do Instead)

10 min read
react

Hooks make React code powerful and expressive — but also surprisingly easy to tangle. In this post we’ll look at three subtle anti-patterns with hooks: mega-useEffects, context overuse, and over-smart custom hooks.

In this post we’ll be taking a look at design-level anti-patterns that try to trick us. How? Well these anti-patterns work, but they make our code harder to change over time.

We’ll look at three common ones:

  1. Doing everything in one useEffect
  2. Overusing context instead of simple props
  3. Making hooks too “smart” to actually reuse

1. Doing everything in one useEffect

import { useEffect, useState } from "react";

function Dashboard() {
  const [data, setData] = useState<WidgetData[] | null>(null);

  useEffect(() => {
    let isCancelled = false;

    // Fetch data
    async function load() {
      const res = await fetch("/api/widgets");
      const json = await res.json();
      if (!isCancelled) {
        setData(json);
      }
    }

    load();

    // Subscribe to WebSocket updates
    const socket = new WebSocket("wss://example.com/widgets");
    socket.addEventListener("message", (event) => {
      if (!isCancelled) {
        const update = JSON.parse(event.data) as WidgetData;
        setData((prev) => (prev ? applyUpdate(prev, update) : prev));
      }
    });

    // Side effect: document title + analytics
    document.title = "Dashboard – Widgets";
    analytics.track("dashboard_viewed", { timestamp: Date.now() });

    // Cleanup
    return () => {
      isCancelled = true;
      socket.close();
      document.title = "My App";
    };
  }, []); // linter will not be happy here

  if (!data) return <p>Loading…</p>;

  return (
    <ul>
      {data.map((widget) => (
        <li key={widget.id}>{widget.value}</li>
      ))}
    </ul>
  );
}

Would the above work? It probably would.

But let us consider what happens if we need to change:

In every single one of the above cases we have to modify the effect that handles them all, which means by changing one we run the risk of breaking all the others. Obivously this can be mitigated with a comprhensive test suite… But we don’t always have that available.

Speaking of tests, how would we test just the Websockets subscription? Or just the widgets? We can’t easily test them in isolation.

It violates SOLID principles. S - Single Responsibility, this effect is juggling multiple responsibilities.

So what should we keep in mind when architecting our effects?

Each useEffect should address one concern

import { useEffect, useState } from "react";

function Dashboard() {
  const [data, setData] = useState<WidgetData[] | null>(null);

  // Fetch data
  useEffect(() => {
    let isCancelled = false;

    async function load() {
      const res = await fetch("/api/widgets");
      const json = (await res.json()) as WidgetData[];
      if (!isCancelled) setData(json);
    }

    load();

    return () => {
      isCancelled = true;
    };
  }, []);

  // Subscribe to WebSocket updates
  useEffect(() => {
    const socket = new WebSocket("wss://example.com/widgets");

    socket.addEventListener("message", (event) => {
      const update = JSON.parse(event.data) as WidgetData;
      setData((prev) => (prev ? applyUpdate(prev, update) : prev));
    });

    return () => {
      socket.close();
    };
  }, []);

  // 3. Document title + analytics
  useEffect(() => {
    document.title = "Dashboard – Widgets";
    analytics.track("dashboard_viewed", { timestamp: Date.now() });

    return () => {
      document.title = "My App";
    };
  }, []);

  if (!data) return <p>Loading…</p>;

  return (
    <ul>
      {data.map((widget) => (
        <li key={widget.id}>{widget.value}</li>
      ))}
    </ul>
  );
}

Same behaviour. But now:

So when it comes to designing our effects we should ask ourselves:

“Could we split this by concern or by external system (network, DOM, subscriptions, etc etc)?“

2. Overusing context instead of simple prop drilling

<ThemeProvider>
  <UserPreferencesProvider>
    <SidebarProvider>
      <Router>{/* ... */}</Router>
    </SidebarProvider>
  </UserPreferencesProvider>
</ThemeProvider>

At some point, many apps end up with many layers of wrapping context. Context is so powerful, and prop drilling often feels ‘wrong’ that we just immediately reach for context to avoid prop drilling a few levels.

However this way of thinking can result in us swimming in a see of Context, and we have:

While Context is a powerful feature, overusing context has a few hidden costs:

  1. Hidden dependencies. A component can look ‘pure’ (no props), but secretly depend on half the app via context.

  2. Heavier re-renders. When a context value changes, every consumer under that provider might re-render. With ‘state in context by default’, we can end up re-rendering large parts of the tree unnecessarily.

  3. Harder testing / reuse. A component that looks reusable actually isn’t, because it assumes it lives under a whole stack of providers.

So sometimes Context may not be the best answer, and we should be fine with leaning on prop drilling when it is the clearer option.

So what can we do? Well a common approach is to always start with props and migrate to context when it makes sense.

Let us take the following simple layout as an example:

function App() {
  const [sidebarOpen, setSidebarOpen] = useState(false);

  return (
    <Layout
      sidebarOpen={sidebarOpen}
      onSidebarToggle={() => setSidebarOpen((open) => !open)}
    />
  );
}

function Layout({
  sidebarOpen,
  onSidebarToggle,
}: {
  sidebarOpen: boolean;
  onSidebarToggle: () => void;
}) {
  return (
    <div className="layout">
      <Header onSidebarToggle={onSidebarToggle} />
      <Sidebar open={sidebarOpen} />
      <MainContent />
    </div>
  );
}

At the moment there is no real concern here, we pass the props down from App to the Layout and then to both Header and Sidebar. It is a clear, perfectly normal, implementation of a component.

Now if later on in the project lifecyle we end up passing sidebarOpen and onSidebarToggle down more layers then we can migrate to context:

const SidebarContext = createContext<{
  open: boolean;
  toggle: () => void;
} | null>(null);

function SidebarProvider({ children }: { children: React.ReactNode }) {
  const [open, setOpen] = useState(false);

  const value = useMemo(
    () => ({
      open,
      toggle: () => setOpen((prev) => !prev),
    }),
    [open]
  );

  return (
    <SidebarContext.Provider value={value}>{children}</SidebarContext.Provider>
  );
}

function useSidebar() {
  const ctx = useContext(SidebarContext);
  if (!ctx) {
    throw new Error("useSidebar must be used within <SidebarProvider>");
  }
  return ctx;
}

Then the Header and Sidebar components can just get what they need:

function Header() {
  const { toggle } = useSidebar();
  // ...
}

function Sidebar() {
  const { open } = useSidebar();
  // ...
}

So we can follow these steps to decide whether Context or prop drilling is needed:

Otherwise we can consider Context, but if we’re ever in doubt then we should always start with props and migrate to Context as needed. This is often much simpler then starting with Context and trying to migrate to props.

3. Making hooks too smart to reuse

When creating custom hooks, the ideal goal is to create something that can be easily shared and reused throughout the codebase. However it is very easy for us to fall for the trap of making our hooks too ‘smart’ which feels great but then locks them to a single instance of being used. Then to work around this we end up adding loads of flags to the hook so we can disable, enable features as needed. It can create something that is a nightmare for teams to maintain.

Take the following as an example:

function useProfileForm() {
  const [values, setValues] = useState<ProfileFormValues>({
    /* ... */
  });
  const [errors, setErrors] = useState<Partial<ProfileFormValues>>({});
  const [isSubmitting, setIsSubmitting] = useState(false);
  const navigate = useNavigate();

  const handleChange = (field: keyof ProfileFormValues, value: string) => {
    setValues((prev) => ({ ...prev, [field]: value }));
    // validate on change
    const newErrors = validate({ ...values, [field]: value });
    setErrors(newErrors);
  };

  const handleSubmit = async () => {
    setIsSubmitting(true);
    try {
      const res = await fetch("/api/profile", {
        method: "POST",
        body: JSON.stringify(values),
      });

      if (!res.ok) throw new Error("Oops");

      toast.success("Profile updated");
      navigate("/dashboard");
    } catch (err) {
      toast.error("Something went wrong");
    } finally {
      setIsSubmitting(false);
    }
  };

  return {
    values,
    errors,
    isSubmitting,
    handleChange,
    handleSubmit,
  };
}

This would work great for the page it was designed for but maybe now we want to use this hook elsewhere, for example:

The only way around this would be to add those flags we discussed:

useProfileForm({
  onSuccessNavigateTo: "/dashboard",
  showToasts: true,
  validateOnChange: true,
  // ...
});

Now our hook, that was supposed to be reuable, is:

Like in the first section, we’re violating good design principles. Each hook should have a single responsibility:

function useFormState<TValues>(initial: TValues) {
  const [values, setValues] = useState<TValues>(initial);

  const updateField = <K extends keyof TValues>(
    field: K,
    value: TValues[K]
  ) => {
    setValues((prev) => ({ ...prev, [field]: value }));
  };

  return { values, setValues, updateField };
}

function useValidation<TValues>(
  values: TValues,
  validate: (values: TValues) => Partial<TValues>
) {
  const [errors, setErrors] = useState<Partial<TValues>>({});

  useEffect(() => {
    setErrors(validate(values));
  }, [values, validate]);

  return errors;
}

function useSubmit<TValues>(
  submit: (values: TValues) => Promise<unknown>,
  { onSuccess }: { onSuccess?: () => void } = {}
) {
  const [isSubmitting, setIsSubmitting] = useState(false);

  const handleSubmit = async (values: TValues) => {
    setIsSubmitting(true);
    try {
      await submit(values);
      onSuccess?.();
    } finally {
      setIsSubmitting(false);
    }
  };

  return { isSubmitting, handleSubmit };
}

Now we can build our useProfileForm hook using the above building blocks:

function useProfileForm() {
  const navigate = useNavigate();

  const form = useFormState<ProfileFormValues>({
    name: "",
    email: "",
    // ...
  });

  const errors = useValidation(form.values, validateProfile);
  const { isSubmitting, handleSubmit } = useSubmit(
    (values) =>
      fetch("/api/profile", {
        method: "POST",
        body: JSON.stringify(values),
      }),
    {
      onSuccess: () => {
        toast.success("Profile updated");
        navigate("/dashboard");
      },
    }
  );

  return {
    ...form,
    errors,
    isSubmitting,
    handleSubmit: () => handleSubmit(form.values),
  };
}

We’ve managed to keep the useProfileForm() API for the component but the underlying building blocks (useFormState, useValidation, useSubmit) are reusable elsewhere.

A good rule of thumb when creating custom hooks is if the hooks config options are getting than the return value then we might need to start splitting things out.

Conclusion

When reviewing our React code we should ask ourselves:

Hooks are a powerful feature of React but it is easy to fall for some of the traps but by following good software design principles we can turn hooks into a powerhouse that work for us, not against us.