Tips and Strategies for Writing Clean and Maintainable JSX Code

Adding to #7: Since we use Tailwind for CSS and the className string can get quite long, I started abstracting it out to a classes object to keep the JSX code more concise. For example:

// utils.js
export const classes = {
  wrapper: 'flex flex-col justify-between bg-white text-grey-8 border border-grey-5 rounded'
}

// component.jsx
import { classes } from './utils';

export default function Component(){
  return (
    <section className={classes.wrapper}>
      {/* ... */}
    </section>
  )
}

In terms of readability

// Good
function WelcomeBanner({ user }) {
  const isLoggedIn = user != null;
  return (
    <div>
      {isLoggedIn ? <LoggedInBanner user={user} /> : <LoggedOutBanner />}
    </div>
  );
}

i would prefer this one:

// Even better readability
function WelcomeBanner({ user }) {
  const isLoggedIn = user != null;

  if(!isLoggedIn) return <LoggedOutBanner />;

  return <LoggedInBanner user={user} />;
}

2. Use Descriptive Component Names

I would argue that both of these are descriptive names and I would expect that InputField is a generic input component that will be used to create specific inputs:

// Good
function EmailInputField(props: InputFieldProps & EmailProps) {
  return <InputField {...props} />;
}

// Avoid
function InputField(props: InputFieldProps)) {
  // ...
}

3. Destructure Props for Clarity

I think this is definitely true for JavaScript users. But with TypeScript, I'd recommend the opposite a lot of the time. Also, you should just pass props if any of your input is stateful so you don't end up with a bunch of duplicated nomenclature:

// bad
function StatefulGreeting({ name, message }) {
  const [nameState, setNameState] = useState(name);
  const [messageState, setMessageState] = useState(message);

  return <SomeComponent name={nameState} message={messageState} />;
}

This is just adding noise and making it complicated to reason about:

// good
function StatefulGreeting(props) {
  const [name, setName] = useState(props.name);
  const [message, setMessage] = useState(props.message);

  return <SomeComponent {...{name, message}} />
}

5. Abstract Conditional Logic

This isn't is a very good example of complex conditional logic. In the current form, you are just doing a loose falsy comparison on user.

In this case, you should flip your examples:

// Bad
function WelcomeBanner({ user }) {
  const isLoggedIn = user != null;

  return (
    <div>
      {isLoggedIn ? <LoggedInBanner user={user} /> : <LoggedOutBanner />}
    </div>
  );
}

This first version just adds unnecessary noise for such a simple comparison. Something so simple should be expressed inline:

// Better
function WelcomeBanner({ user }) {
  return (
    <div>
      {user != null 
        ? <LoggedInBanner user={user} /> 
        : <LoggedOutBanner />
      }
    </div>
  );
}

This is better, but the loose comparison is unnecessary too. For simple conditions like this the most common pattern I see in the React community is like this:

// Best
function WelcomeBanner({ user }) {
  return (
    <div>
      {user && <LoggedInBanner user={user} /> || <LoggedOutBanner />}
    </div>
  );
}

Short, simple and to the point.

Now say you have an actually complex condition, then you should extract it from your component template:

// bad
const isLoggedInLocalAndOverage = user && user.locale === APP_LOCALE && user.birthday < dayjs().subtract(18, 'year')

return <div>
    {isLoggedInLocalAndOverage && <LoggedInBanner user={user} /> || <LoggedOutBanner />}
</div>

But that's still bad as you have to reason about it in your component, and this kind of logic may be needed elsewhere.

Better to abstract it out of the component completely:

return <div>
    {isLoggedInLocalAndOverage(user) && <LoggedInBanner user={user} /> || <LoggedOutBanner />}
</div>

I think this is one piece of advice that's always missing from these kinds of articles: express component logic in your component, express your application logic elsewhere.

6. Use Comments Wisely

I feel like all of your examples here are bad comments. There's no difference between:

• "Fetching data from the API and filtering for active users"
• "This is a div"

They both literally describe the code. If your API call looks something like this, then it should be pretty clear what's happening. And if it doesn't look like this, you should modify it to make it more explicit.

api.get('/users', { active: true })

And this is explicit from the wording. It would be very worrying if developers need a comment to understand what is happening here:

{activeUsers.length > 0 && (
  <UserList users={activeUsers} />
)}

I think in your example, the only comment that's needed is to document the component's purpose as its name is very generic and doesn't describe itself:

/**  
 * Renders a list of active users.
 * @constructor  
 */
function ComplexComponent() {
  //...
}

A better solution here might be renaming the component, so that the comment is redundant:

function ActiveUsersList() {
  //...
}

And if you find yourself needing to pepper your component with comments, you should probably go back to step one and break it up into smaller components.

7. Avoid Inline Styles

This is a big opinion. Your example is very basic and not really inline with how a lot of teams would use inline styles. At the very least you should update it to use some variables:

function Button({ label }) {
  return (
    <button style={{ 
      backgroundColor: theme.colors.blue, 
      color: theme.colors.white,
    }}>      
      {label}
    </button>
  );
}

But again, your advice here is personal opinion... On the one hand, extracting styles makes components cleaner. On the other hand, now you've increased the complexity of your application and maintenance burden.

Which of these is preferable is a matter for individual teams to determine.

8. Prop Types and Default Props

This is certainly true for JavaScript users. In this case the prop-types package is almost mandatory. But I've almost never used it on a TypeScript project. TypeScript is very good at catching a lot of these kinds of errors before runtime which just makes prop-types redundant.

If you need to check types and enforce defaults, just define a default props variable and merge them with passed props:

type UserProfileProps = {
  name: string;
  age?: number;
}

const DEFAULT_PROPS: Pick<UserProfileProps, 'age'> = { age: 30 };

function UserProfile(props: UserProfileProps): React.JSX.Element {
  const { name, age } = { ...DEFAULT_PROPS, ...props };
}

I would add point 1: use a proper state manager.
Without it, or relying on React build-in defaults like useState, useContext and useEffect to manage state - is pretty much a code smell. (With single exception in writing ui library code and using useContext where)

I recommend Jotai.

Resct number 1 biggest problem is unnecessary re-renders. Absolutely every react developer suffers because of it at least once.
This problem goes completely away using a proper state manager (and using it right).

Full Fragment is useful in for loops where you can group all entries with a react key prop without having to add it to every single component

"Clean and Maintainable JSX Code" feels very much like an oxymoron