Refactoring React - Cleaning up return()

Refactoring React is a series documenting some of the things I've learned about how to write React and make the lives of future developers (or you a week from now) a little easier.

The first thing I look at in an unfamiliar React component is the return. I want to understand what the expected output is before I start reasoning about the rest. Ideally, the return is a nice block of JSX with some JS variables here and there. It's easy to read, and easy to track down variables or methods to figure out how the component works.

What I hope not to see is nested blocks of JS mapping, filtering, and anonymous-function-passing all over the place. Sights like this are not uncommon:

const Component = () => {
  return (
    <div className='stuff' >
      {someBoolean && <ItemList>
        {bigNestedMess.property.map(item => {
          helperMethod(item).filter(value => value === 'test')
          .map(filteredData => <Item>{ filteredData }</Item>)
        })}
      </ItemList>}
      {!someBoolean && <ItemList>
        {bigNestedMess.property.map(item => {
          helperMethod(item)
          .filter(value => value === 'differentTest')
          .map(filteredData => <Item>{ filteredData }</Item>)
        })}
      </ItemList>}
      <AnotherComponent onClick={() => {
        handleThis()
        handleThat()
      }}/>
    </div>
  )
}

The thought process of reading that return goes something like:

Ok, it returns a div, with... an ItemList component... that has some... wait no it's conditionally returning the ItemList, with Item children... that it's getting from a helper method? I'll dig into that later. Oh wait, it's filtering the children, too...

It's not pretty. All of the component's logic is mixed up with all of the component's JSX, and it's hard to reason about either.

Here's the same component with a basic refactor:

const RefactoredComponent = () => {
  const items = bigNestedMess.property.map(item => {
    const someData = helperMethod(item)
    someData.filter(value => {
      const testValue = someBoolean ? 'test' : 'differentTest'
      return value === testValue
    })
    .map(filteredData => <Item>{ filteredData }</Item>)
  })
  
  const handleClick = () => {
    handleThis()
    handleThat()
  }

  return (
    <div className='stuff' >
      <ItemList>
        { items }
      </ItemList>
      <AnotherComponent onClick={handleClick} />
    </div>
  )
}

Nice! It returns a div, with an ItemList and some children, along with AnotherComponent, which we pass a click handler to. It's easy to look up the variable and method above the return, and we can reason about the business logic and the output without getting dizzy.

Here are some of my rules of thumb for cleaning up return():

  • Construct arrays before the return. Even a simple map() makes the return harder to read
  • Conditional with caution. Having a focused ternary or logical && operator is great when you're toggling a component on or off. Once you start getting multiple conditionals, or conditionals with long blocks of JSX, consider pulling them out of the return by constructing them in a variable. Complex conditional logic can also be a sign it's time to create a new component.
  • Avoid passing anonymous functions. JSX isn't the first place anyone goes looking for function definitions, and they're impossible to reuse, hard to test, hard to debug, and can cause unnecessary renders.