Today I Learned

React code-smells: Mapping UI having methods for items

Whenever mapped elements have their own functions, they should be extracted to their own components.

const UsersList = () => {
  const [users, setUsers] = useState([])
  const fetchUsers = () => { ...fetchingLogic }

  const getHeader = (item, index) =>
    item.isAdmin 
      ? `${index + 100} :: ${item.superName} :: ${item.rank}` 
      : `${index} :: ${item.name} :: ${item.age}`

  const getImage = (item) =>
   item.url ? `${item.url}?token=${item.authToken}` : 'http://default.img.com'

  return (
    <div>
      {list.map((user, index) => (
        <div key={user.id}>
          <h3>{getHeader(user, index)}</h3>
          <p>Some introduction</p>
          {getImage(user)}
        </div>
      ))}
    </div>
  )
}

That way:

  • Item methods (getImage & getHeader) do not pollute UsersList component having own methods (fetchUsers)
  • There is no need to pass params to item methods
const User = ({ user, index }) => {
    const getHeader = () =>
      user.isAdmin
        ? `${index + 100} :: ${item.superName} :: ${item.rank}`
        : `${index} :: ${user.name} :: ${user.age}`
  
    const getImage = () =>
      user.url ? `${user.url}?token=${user.authToken}` : 'http://default.img.com'
  
    return (
      <div>
        <h3>{getHeader()}</h3>
        <p>Some introduction</p>
        {getImage()}
      </div>
    )
  }
  
  const UsersList = ({ list }) => {
    const [users, setUsers] = useState([])
    const fetchUsers = () => { ...fetchingLogic }

    return (
      <div>
        {users.map((user, index) =>
          <User user={user} index={index} key={user.id}/>
        }
      </div>
    )
  }