On my project, I’ve been noticing code like this:

class Header extends React.Component {
  static propTypes = {
    items: PropTypes.array,
    counters: PropTypes.object,
  };
  
  // ...
}

This is how it should look like instead:

class Header extends React.Component {
  static propTypes = {
    items: PropTypes.arrayOf(PropTypes.shape({
      id: PropTypes.string.isRequired,
      name: PropTypes.string.isRequired,
      link: PropTypes.string.isRequired,
    })).isRequired,
    counters: PropTypes.objectOf(PropTypes.number).isRequired,
  };

  // ...
}

The difference is that in the latter component, propTypes are much more detailed. It’s better for two reasons:

• React validates your props better. In the former component, you won’t get any warnings if you pass a wrong array into items or if you forget to pass it at all. Instead, you’ll have a wrong rendering result or a runtime error and will have to debug it.

• You understand the interface of the component easier. This is even more important.

  With the latter component, to understand the structure of items, you just look at its propTypes. With the former one, you have to dive into its code. It’s not a problem when the component has been created just 10 minutes before, and you remember what it accepts, but it makes further support way easier.

There’s only one case when I find it acceptable to skip some propTypes definitions. It’s when your component just passes a prop to a child component and doesn’t care about its structure. In this case, the child component should validate the prop:

class Header extends React.Component {
  static propTypes = {
    items: PropTypes.arrayOf(PropTypes.shape({
      id: PropTypes.string.isRequired,
    })).isRequired,
    counters: PropTypes.objectOf(PropTypes.any).isRequired,
  };

  render() {
    return <div>
      {this.props.items.map(item =>
        <HeaderItem item={item} counter={this.props.counters[item.id]} />
      }
    </div>;
  }
}

class HeaderItem extends React.Component {
  static propTypes = {
    item: PropTypes.shape({
      name: PropTypes.string.isRequired,
      link: PropTypes.string.isRequired,
    }).isRequired,
    counter: PropTypes.number.isRequired,
  };

  // ...
}

Here’s the rule:

Noticed a React anti-pattern while mentoring a younger front-end developer on our project.

Imagine you have a component that represents a list of articles. At one point, you realize that each article in the list is too complex:

const ArticleList = (props) => {
  return <div class="articles">
    <ul class="article-list">
      { props.articles.map(article => 
        <li class="article">
          <h2 class="article__title">{ article.title }</h2>
          { /* 25 other tags */ }
        </li>
      ) }
    </ul>
    { /* ... */ }
  </div>;
}

So you decide to move the item to a separate component. You take that code inside map(), extract it into <Article> and get something like this:

const Article = (props) => {
  return <li class="article">
    <h2 class="article__title">{ props.title }</h2>
    { /* 25 other tags */ }
  </li>;
}

const ArticleList = (props) => {
  return <div class="articles">
    <ul class="article-list">
      { props.articles.map(article => 
        <Article title={article.title} { /* other props */ } />
      ) }
    </ul>
    { /* ... */ }
  </div>;
}

Don’t do it this way. This approach is wrong. The problem is that by taking a <li> and making it a root of the component, you’ve just made your component non-reusable. This means that if you’d like to reuse <Article> in another place, you’ll only be able to apply it somewhere inside a list – because of this <li>. If you decide to render <Article> into e.g. a <div>, not only will this be non-semantic (<li> can only appear inside of <ul>s), but will also add unnecessary list item styling which is super weird.

Solution#

The solution is simple: move the <li> back into the <ArticleList> component and make the <Article>’s root element a <div> or something else. This will probably require some refactoring in your styles, but will make the component reusable. Look how cool:

const Article = (props) => {
  // Notice: <li>s are gone
  return <div class="article">
    <h2 class="article__title">{ props.title }</h2>
    { /* ... */ }
  </div>;
}

const ArticleList = (props) => {
  return <div class="articles">
    <ul class="article-list">
      { props.articles.map(article => 
        <li class="article-list__item">
          <Article title={article.title} { /* other props */ } />
        </li>
      ) }
    </ul>
    { /* ... */ }
  </div>;
}

// And now you can easily render an article inside of a sidebar – have no idea why though
const Sidebar = (props) => {
  return <div class="sidebar">
    <div class="sidebar__article">
      <Article title="Look ma" { /* other props */ } />
    </div>
    { /* ... */ }
  </div>;
}