What's wrong with this code?

function ContactList({contacts}) {
  return (
    <div>
      <ul>
        {contacts.length &&
          contacts.map(contact => (
            <li key={contact.id}>
              {contact.firstName} {contact.lastName}
            </li>
          ))}
      </ul>
    </div>
  )
}

Not sure? Let me ask you another question. What would happen with the above code if contacts was []? That's right! You'd render 0!

I shipped this mistake to production at PayPal once. No joke. It was on this page:

When a user came with no contacts, this is what they saw:

Yeah, that was not fun... Why is that? Because when JavaScript evaluates 0 && anything the result will always be 0 because 0 is falsy, so it doesn't evaluate the right side of the &&.

The solution? Use a ternary to be explicit about what you want rendered in the falsy case. In our case, it was nothing (so using null is perfect):

function ContactList({contacts}) {
  return (
    <div>
      <ul>
        {contacts.length
          ? contacts.map(contact => (
              <li key={contact.id}>
                {contact.firstName} {contact.lastName}
              </li>
            ))
          : null}
      </ul>
    </div>
  )
}

What about this code? What's wrong here?

function Error({error}) {
  return error && <div className="fancy-error">{error.message}</div>
}

Not sure? What if error is undefined? If that's the case, you'll get the following error:

Uncaught Error: Error(...): Nothing was returned from render. This usually means a return statement is missing. Or, to render nothing, return null.

Or, in production, you might get this:

react-dom.production.min.js:13 Uncaught Invariant Violation: Minified React error #152; visit https://reactjs.org/docs/error-decoder.html?invariant=152&args[]=f for the full message or use the non-minified dev environment for full errors and additional helpful warnings.

The problem here is that undefined && anything will always evaluate to undefined.

So what gives?

Really, the problem in both of these cases is we're using && to do conditional rendering. Said differently, we're using && to do conditional argument passing. Remember, JSX is a simple syntactic abstraction over calling React.createElement. So it'd be like trying to write this:

function throwTheCandy(candyNames) {
  for (const candyName of candyNames) {
    throwCandy(candyName)
  }
}

throwTheCandy(candies.length && candies.map(c => c.name))

That wouldn't make any sense right? We've got our types all messed up. The reason React allows you to do this though is because rendering 0 is valid.

Luckily, TypeScript can help you avoid the second example of accidentally returning undefined. But on top of that layer of protection, how about we be more explicit about our intentions. If you want to do something conditionally, don't abuse the logical AND operator (&&).

It actually seems counter-intuitive because we often use && in an if condition to say: if both of these values are truthy, then do this thing, otherwise don't. Unfortunately for our use-case, the && operator also has this "feature" where if both values aren't truthy, it returns the value of the falsy one:

0 && true // 0
true && 0 // 0
false && true // false
true && '' // ''

Use actual branching syntax

So I strongly suggest that rather than abusing && (or || for that matter) in your JSX, you use actual branching syntax features, like ternaries (condition ? trueConsequent : falseConsequent) or even if statements (and in the future, maybe even do expressions).

I'm personally a fan of ternaries, but if you'd prefer if statements, that's cool too. Here's how I'd do the contacts thing with if statements:

function ContactList({contacts}) {
  let contactsElements = null
  if (contacts.length) {
    contactsElements = contacts.map(contact => (
      <li key={contact.id}>
        {contact.firstName} {contact.lastName}
      </li>
    ))
  }

  return (
    <div>
      <ul>{contactsElements}</ul>
    </div>
  )
}

Personally, I think the ternary is more readable (if you disagree, remember that "readable" is subjective and the readability of something has much more to do with familiarity than anything else). Whatever you do, you do you, just don't do bugs.

Another benefit of using actual branching syntax is that test coverage tools can detect and indicate when your tests are not covering one of the branches.

Conclusion

I'm well aware that we could've solved the contact problem by using !!contacts.length && ... or contacts.length > 0 && ... or even Boolean(contacts.length) && ..., but I still prefer not abusing the logical AND operator for rendering. I prefer being explicit by using a ternary.

To finish strong, if I had to write both of these components today, here's how I'd write them:

function ContactList({contacts}) {
  return (
    <div>
      <ul>
        {contacts.length
          ? contacts.map(contact => (
              <li key={contact.id}>
                {contact.firstName} {contact.lastName}
              </li>
            ))
          : null}
      </ul>
    </div>
  )
}

function Error({error}) {
  return error ? <div className="fancy-error">{error.message}</div> : null
}

Good luck!