React JS Code Review Checklist

This post provides a checklist to review a React JS application code. For many issues, especially related to personal taste like semicolon or arrow vs traditional function, and things like propTypes, we can simply configure linters such as ESlint and Prettier to automatically detect and force us to resolve the warnings. We can furthermore make these linting fixes mandatory on commits (such as with Husky hooks) or PR merges (with CI/CD tools such as Github Actions).

Below, I’ve tried to cover things that are not necessarily detected by linting tools:

CONST Variables for Repeated String Literals

If a string literal is being used constantly throughout the file, it’s a candidate for a const variable at the top. For example:

if(user.instituteType === "school"){

}

// ...

const schoolGoing = users.map(u=>u.instituteType === "school");

// ...

{
  user.instituteType === "school"? <div>School</div>: null
}

The above could be changed to:

const SCHOOL = "school";

if(user.instituteType === SCHOOL){

}

// ...

const schoolGoing = users.map(u=>u.instituteType === SCHOOL);

// ...

{
    user.instituteType === SCHOOL? <div>School</div>: null
}

Variable or Helper Method for Complicated Expression

If there are too many && or || in a condition and, even worse, repeated multiple times, it’s a good idea to assign it to a variable. If it requires more calculation with context, convert it to a helper method. This immensely improves the readability of the code. Examples:

1. Variable

if (user.active && (user.role === "admin")){
// ...
}

const activeAdmin = user.active && (user.role === "admin");

if (activeAdmin){
// ...
}

2. Helper Method

{
  user.active && 
  user.noOfAttempts > 3 
  && (Date.now() - user.lastLoginAttempt >= 60000) 
  ? <div>You can log in</div> : <div>You are not allowed to login. Please try again after some time.</div>
}

// somewhere in the helper methods file or at the top if only used in one file
const canAttemptLogin = ({active, noOfAttempts, lastLoginAttempt}) => {
  return active && noOfAttempts > 3 && (Date.now() - lastLoginAttempt >= 60000;
}


// inside your component
{
  canAttemptLogin(user) ? <div>You can log in</div> : <div>You are not allowed to login. Please try again after some time.</div>
}

Outdated or Pointless Comments

Look for comments that no longer add to the context. They tend to remain in the code even when things are considerably changed, and are actually more confusing than helpful.

Commented Code

Many of us have a habit of commenting out some piece of code instead of removing it. If we need to reference something use git and remove the commented code for the sake of keeping the codebase clean.

Spelling and Grammar Mistakes

Although not an English writing contest, for the sake of better readability and searchability, there should be minimal spelling errors in your variable, file, and directory names, and also in your static HTML text. Some examples:

// misspelled variable names
const lable, univresity, schoolGongChildren, multPlayer, caculation;

// misspelled directory and file names
// /resorces /configration helpre.js

// grammatically incorrect HTML static text
<span>Your welcome</span>

One of the benefits of writing spelling and grammar mistake-free code is the ability to find something in the codebase with ease. You won’t have to waste time searching for the file “helper.js” and getting back “no results found”.

If you’re using visual studio code, you can install Code Spell Checker extension.

Variable, File, and Directory Names Are Appropriate and Meaningful

Depending on what kind of naming convention you follow, any new addition of variable, file or directory should adhere to that. You should also ensure that the name is making sense. For example:

const firstName;

is a better variable than:

const fN;

Take special note of naming parameters inside of methods such as each, map, filter etc.

{
  users.map(user => <span>{user.firstName}</span>)
}

Here user is a way more informative name than:

{
  users.map(item => <span>{item.firstName}</span>)
  // OR
  users.map(i => <span>{i.firstName}</span>)
}

Similarly, an informative and to-the-point directory and file name are always self-documenting and easier to reason about for others. E.g.:

// /configJsonFiles

is more informative than

// /jsonFiles

// OR worse

// /files

Destructuring

Whether you’re importing different methods of a package or using props values, try to destructure your es6 code for cleanliness:

import _ from "lodash";

// ....

_.forEach(
  //...
)
_.filter(
  //...
)

import {forEach, filter} from "lodash";

// ....

forEach(
  //...
)
filter(
  //...
)

Similarly:

const UserInfo = (props) =>
(<div>
  <div>
    First Name: {props.firstName}
  </div>
  <div>
    Second Name: {props.secondName}
  </div>
</div>)

const UserInfo = ({
  firstName,
  lastName
}) =>
(<div>
  <div>
    First Name: {firstName}
  </div>
  <div>
    Second Name: {secondName}
  </div>
</div>)

System-Wide Additions and Removals

Be extra careful about addition to or removal from any file (usually configuration) that affects the whole system.

One example would be tailwind configuration file. If you remove any existing configuration or add a new one, it might mess up the styling of the application somewhere. So you should be really confident about these changes.

Unit Tests

For a new addition of code, see if some unit tests are worth adding. Unit tests should never be for the sake of coverage, but they should really be to improve the confidence in code. No tests are better than a suite of useless tests that are broken and nobody cares about. So make sure to identify and add quality tests.

Don’t Repeat Yourself (Or Don’t Reinvent the Wheel)

This decision is usually not straightforward, but we should strive to write DRY code whenever possible. In React code, keep the following questions in mind:

Can this component better be written as a common component to prevent a future repetition?
Can this logic be performed with one of the methods of the library we already have (such as lodash or underscore)?
Is this logic better placed in a helper method and used at multiple places?
Can we wrap this combination of tailwind utility classes in one CSS class? (because the same combination is being used at so many places)

In short, identify a repetition and use your judgment to decide if moving that code to DRY is better or not.

Improve the Algorithm

Usually, the backend is concerned with complex algorithms, and the frontend only cares about fetching and displaying the data to the UI, with minimal processing. Any algorithm that the frontend does need, such as sorting, is covered by famous third party libraries such as lodash.

Regardless, there might still be some piece of code you can optimize to shave a few milliseconds off the browser’s processing time. When you come across something like this in code review, you need to decide if it’s worth optimizing. If it is, go for it.

Addition of New Package(s)

React applications, like others, are built on top of so many other useful packages/libraries. In an active development, you still need to add new packages. While code reviewing, pay special attention to the changes in package.json file. Consider the following points:

Is the new package absolutely necessary?
Is the new package rightly listed under “dependencies” or “devDependencies”?
The npm profile page for the library shows healthy downloads per week and frequent releases of new versions (a package with no new release for over a year is a red sign).
The license of the package is compatible with your needs. If the license is ISC or MIT, you’re probably good to go.

reactjs