Designing APIs: use arrays instead of rest parameters

Or why _.get(object, ['deep', 'prop']) is better than _.get(object, 'deep', 'prop').

Imagine you’re designing a function that accepts a variable number of arguments. Like getDeepProperty():

function getDeepProperty(<object> and <chain of nested fields>) {
  // Retrieve the deep property and return it
}

And you’re facing a choice: should you pass the chain of fields directly:

function getDeepProperty(object, ...fields) { /* ... */ }

getDeepProperty(someData, 'i', 'love', 'designing', 'apis');

or as an array:

function getDeepProperty(object, fields) { /* ... */ }

getDeepProperty(someData, ['i', 'love', 'designing', 'apis']);

Earlier, I preferred the former way because it seemed “cleaner”. Now, I use the latter one – because it makes the function forward-compatible.

At some moment in the future, you might want to pass an additional parameter specifying the default return value – i.e., what the function should return if the deep property is absent. The only backwards-compatible way to do this is to pass this parameter as the last argument to the function, like this:

function getDeepProperty(
  <object>,
  <chain of fields>,
  <default return value>
) {
  // Retrieve the deep property and return it,
  // or return the default value
}

And here comes the difference. If your function accepts an array, you just append a new argument, do a switch (arguments.length) inside your function body, and the old code keeps working:

function getDeepProperty(...args) {
  if (args.length === 3) {
    const [object, fields, defaultValue] = args;
    // Execute the new behavior
  } else {
    const [object, fields] = args;
    // Execute the old behavior
  }
}

// The new API calls work great: a user passes three arguments,
// the function understands this and returns 'no'
getDeepProperty({}, ['i', 'love', 'designing', 'apis'], 'no');

// The old API calls keep working: a user passes two arguments,
// the function works as usual and e.g. throws an exception
getDeepProperty({}, ['i', 'love', 'designing', 'apis']);

But if your function accepts a variable number of arguments, you become unable to add an argument without breaking the old API users. The function can’t understand whether the last parameter is the default return value or a value in the chain of fields:

function getDeepProperty(object, ...fieldsAndDefaultValue) {
  // So, is fieldsAndDefaultValue[fieldsAndDefaultValue.length - 1]
  // a default value or just a field in a chain of fields?
  // You can’t know

  // Execute the new behavior
}

// The new API works great: the function returns 'no'
getDeepProperty({}, 'i', 'love', 'designing', 'apis', 'no');

// But the old API breaks: the function returns 'apis'
getDeepProperty({}, 'i', 'love', 'designing', 'apis');

So, here’s the rule:

When defining a function, prefer arrays over variable number of arguments

Describe your React propTypes as deeply as possible

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:

Note how the items
propType in Header cares only about the id field it uses, and counters doesn’t care about its items type at all.

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:

Describe your propTypes as deeply as possible

React anti-pattern: don’t make <li> your component’s root

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>;
}