React – keep JSX simple

Consider this code:

const Header = () => {
  const isAuthenticated = useIsAuthenticated()

  return (
    <div>
      <Logo />
      {isAuthenticated ? <p>Welcome Member</p> : <p>Welcome Guest</p>}
      <Menu />
    </div>
  )
}

export default Header

const Header = () => {

const isAuthenticated = useIsAuthenticated()

return (

<div>

{isAuthenticated ? <p>Welcome Member</p> : <p>Welcome Guest</p>}

</div>

)

}

export default Header

Nothing inherently wrong here, but I do think this this code could be a little bit cleaner, and to be honest what I am about to point out could be considered very subjective, but I think it is worth at least considering. I believe JSX should stick to a document (markup) style as much as possible.

So I would prefer this approach:

const HeaderTitle = () => {
  const isAuthenticated = useIsAuthenticated()

  if (isAuthenticated) {
    return <p>Welcome Member</p>
  }

  return <p>Welcome Guest</p>
}

const Header = () => (
  <div>
    <Logo />
    <HeaderTitle />
    <Menu />
  </div>
)

const HeaderTitle = () => {

const isAuthenticated = useIsAuthenticated()

if (isAuthenticated) {

return <p>Welcome Member</p>

}

return <p>Welcome Guest</p>

}

const Header = () => (

<div>

</div>

)

I feel that this is the most readable and provides ease when trying to quickly understand the hierarchy of components present, and specific concerns are more separated and contextualised.

This was a very basic example, and I might not even bother leaving a comment on a code review if the instance was no more complex than what we have here, but I have seen instances in which the structures were much more complex, the JSX became muddled with many branches of logic, or maybe a ternary, and even sometimes a nested ternary – in these cases I absolutely would leave a comment suggesting that we should consider abstracting these branches of logic out into separate components.