React Anti-Patterns

📚 4 min read Tweet this post
Tires stacked in Pattern
Photo By Lane Smith

Introduction

  • Am I using the right pattern?
  • What will my coworkers think of this?
  • Is this readable?

Everyone asks themselves these questions at some point when learning something new. I’ve been working with React for 3 1/2 years now and these are some patterns that have come back to bite me.

You should watch out for code that you can’t understand easily, write readable code. Testable code is usually easier to read, so write tests! 🧪

Shoutout to react-testing-library

I first started writing React after reading a book on design patterns, and the first chapter was on Inheritance.

'Headfirst Design Patterns Book'

Great book, worth the read 👍🏼

The basics of inheritance are you have a class that can inherit the behavior of another class. Similar to how we inherit traits from our parents.

One of my first challenges was I had two components that needed the same data fetching behavior. And what came next was some of the worst code I have EVER written. I broke the number one React rule, I made a component base class 🙈

It looked something like this:

class Base extends React.Component {
  componentDidMount() {
    apiCall(this.props.type).then((data) => {
      this.setState({ data });
    });
  }

  render() {
    throw new Error("SHOULD NEVER GET HERE");
  }
}

class Widget extends Base {
  render() {
    if (!this.state.data) return null;

    return <div>{this.state.data.title}</div>;
  }
}

The idea was that Widget extends another component to save some code duplication. Don’t use the extends keyword to inherit other React components. The funny thing is, React has explicit warnings against this! Read the docs people.

I’m pretty sure everyone runs into this problem in React. I call it Prop Madness. Here is an example:

function Outer({ data, onHover, onClick, onSaveInner }) {
  return (
    <Inner
      title={data.title}
      date={data.date}
      isTesting={data.isTesting}
      text={data.text}
      actionButtonTitle={data.actionButtonTitle}
      onHover={onHover}
      onClick={onClick}
      onSaveInner={onSaveInner}
    />
  );
}

function Inner({ title, date, isTesting, onHover, onClick }) {
  // Do stuff ...
}

You can mask the problem like this:

function Outer({ data, onHover, onClick, onSaveInner }) {
  return <Inner {...data} onHover={onHover} onClick={onClick} onSaveInner={onSaveInner} />;
}

function Inner({ title, date, isTesting, onHover, onClick }) {
  // Do stuff ...
}

Problem solved! Right?

Nope.

We haven’t really solved it. Now any extra properties on the data object get passed to the <Inner /> component when they don’t need to! And what happens when some of those props become optional? When the <Inner /> component wants to rename one of those props?

Also notice how the <Outer /> component doesn’t actually use any of the props, it just passes them along!

This approach couples the two components together, you may as well inline the <Inner /> component inside the <Outer />

React created a built-in approach to solving this problem. It’s called Context.

Context can solve this problem quite nicely, and I recommend learning the API. However, if you use something like Redux, Apollo, Relay, MobX, or anything similar, keep in mind the next Anti-pattern.

Too often I see code that is over-abstracted and split up into pieces that are too small. Not just with React either. Have you ever seen a generic /utils folder that holds all the business logic and each “utility” is only used in one other place? It’s misdirection (not abstraction), and it hurts code readability.

Our Prophet, Dan Abramov, recently made an addendum to his post on Presentational vs Container Components, stating that separation like this can be harmful when taken too seriously.

I live by the following mantra:

A component is a component

Simple right? The idea is that we don’t need to separate concerns and make components focused on styles 💅 or data 📈. A component is a component, and that means it doesn’t matter if it has css, or makes api calls. Prop Madness can occur when trying to separate components by what they do instead of what they are.

Don’t over-abstract your components. Don’t focus on DRY code too much, and favor code readability over patterns and reuse.

My favorite quote about programming in general:

Make it work, Make it right, Make it fast” - Kent Beck

People, It’s O.K. to put more than one component in a single file. I’m not sure what started this trend, but I’ve seen projects where there is a file per component or even per function 😱.

Co-location is perfectly valid if you don’t need to reuse the code elsewhere.

Take the following NavBar component for example:

NavBar.js
import React from "react";
import axios from "axios";
import styled from "styled-components";

// Styled-components are my favorite 😊💅
const Title = styled.h1`
  background-color: #dc8b22;
  font-size: 1.5rem;
  text-decoration: underline;
`;

const Result = styled.div`
  border-bottom: 2px solid orange;
`;

// I also love Axios!
function apiCall(keyword) {
  return axios.get(`/api/search?query=${keyword}`);
}

export function NavBar() {
  const [keyword, setKeyword] = useState("");
  const [results, setResults] = useState(null);

  function handleChange(event) {
    setKeyword(event.target.value);
  }

  function search(event) {
    event.preventDefault();

    apiCall(keyword).then((data) => {
      setResults(data.results);
    });
  }

  return (
    <header>
      <Title>Find things!</Title>
      <form onSubmit={search}>
        <input placeholder="Search" onChange={handleChange} />
      </form>
      {results.map((result) => (
        <Result key={result.id}>{result.value}</Result>
      ))}
    </header>
  );
}

This code is completely arbitrary, and I’m leaving quite a bit out.

Notice how it combines styles, data loading, and regular markup? Especially notice how this file encapsulates all the logic around the NavBar search? We should optimize our code for readability. In most cases having 3 files open to understand how a nav bar gets rendered is not readable.

There are no anti-patterns!

Yeah, kinda boring conclusion here. I think the reason I’ve come to love React is the un-opinionated stance it takes. Do what makes sense and follow good principles of programming.

  • 🧪 Write tests
  • ♻️ Always be refactoring
  • 📚 Optimize for readability

React is doing something we have never seen before, it is outlasting the Javascript library lifecycle. I’m extremely Bullish on React and I can’t wait to see where we take it the next 5 years 👍🏼

react