18 Tips For a Better React Code Review (TS/JS)

Introduction

If you have some experience as a React developer, you are probably familiar with the React code review process. If not – it’s a process that helps keep good code in a project, eliminate potential bugs or just check from higher-skilled React developers. It also helps other teammates to be up to date as they see all code updates.

I will try to point out what you should be looking for during this process and how to write good comments instead of just “change A to B”.

But let’s start with the simple question. What is the goal of the React code review?

• Show other developers what changes have been made to the project.
• Share knowledge with teammates
• Less experienced developers coaching
• Discuss other solutions
• Catch issues or possible issues

Catching bugs seems to be the most desired goal BUT let’s be honest – it happens occasionally and should be threatened as a bonus. We should do a code review to share knowledge and make the other developer more confident about the code – if you will accept the PR – he will feel like he did a good job.

React Code Review Requirements

If you are a reviewer, and you do the React code review often – you should set up some rules first.

They will help you stay less angry, as the person that provides React development services and prepares the code review will have specific steps to follow before sending the code to you. Win-win.

There are a few things I really like in this process, and I consider as highly useful. Some of them are:

1. The code is linted.

Before submitting.

2. Developer (that requests the code) has actually checked the code himself.

On the platform, before sharing a link – that usually helps with some comments, console.logs, bad formatting and other leftovers.

3. Description of what has been done in this request.

It doesn’t have to be extra detailed, but something like “Add a new page for players listing with table, the table has a pagination but cannot be sortable or filterable. For now, we have to use a mock for data as the API is not ready yet.” will show the overall context.

4. A screenshot of the work that was done.

Sometimes it’s also good to send some screenshots, so the reviewer doesn’t have to run the project (unless he has to test it too).

Extra: Don’t create requests that contain a lot of files.

More files = fewer comments, as nobody is going to check that very precisely – it will take ages. If you have a big feature – you can create a branch for it and then create smaller sub-branches with smaller features.

These few things are just an example, and I would like to encourage you to set up your own rules the way you want.

General things to consider

Working in the same React team

If you are a team member, and you are working on the same project with the developer requesting code review – it’s much easier and you both will get some value from the code review process. As a reviewer, you will see what is changing in your project, and you can immediately catch it up from the code to help with it. This way, it’s much easier to find potential bugs, backwards compatibility issues or incorrect usage of methods, before the problem will cause more trouble.

Working outside the React team

On the other hand, if you are just responsible for the code review, but you don’t work with the project itself – don’t feel sorry for the things you are not aware of, and you won’t probably point out correctly. Nobody is going to blame you for functionality that is not working, and you didn’t notice it.

In general, it’s hard to find bugs during that process, and if you will find any – that’s great! But if you don’t, be ready to ask for more details or why some things are done in this or that way and not the other. Get truly curious.

Making comments visible

Try to make all your comments visible on the platform you use. It will help others to pick up the right context. If you will just comment on the code in private chat it can easily be lost, and only you can see it.

Indicating the time

If you don’t have time for a proper review – add it as a note.

Example:

“I had only 15 minutes, so I just quickly checked the most important things like A, B, C.”.

Remember – if someone asks you for a review, tell them when you will have time for it. Some people have the tendency to just wait until you will finish and send them the code back – but if you tell them, for example, you will do it the next day – they may find some other work to do in the meantime.

Don’t waste time on styling issues

Generally, most of the comments in React code reviews (I’ve seen) are about styling issues – and personally, I don’t like them.

If you have any styling issues, it means you have your linter set incorrectly, or you are not using it at all, and if anyone starts pointing out that kind of comment – I recommend stopping doing it.

In my opinion, that’s just a huge waste of time, and it can be fixed automatically by linter/prettier. If you see that there are styling issues all over the React code – point that out once – and suggest solutions, so they won’t appear in the future. If you don’t do that they will be visible on each request.

18 Tips for better React Code Review

Here’s the full list of our tips and what to check to do a better React Code Review:

• Are there any new npm packages added?
  
  
• Check if there is no functionality duplicates like date-fns + moment.
  
  
• Also check for imports, as sometimes tree shaking is not working as you wish, and you could bundle the whole library and use just a single method like the below:

import _ from 'lodash';
//should became more precise import like:
import uniq from 'lodash/uniq'; 

• If your app is using translations – check if all new areas have also translations. If not, point that out and the developer should be aware of that in the future.

const NewComponent = () => {
  return (
    <div>
      New static text inside new component should be translated!
    </div>
  )
}

• Check for missing or invalid types if you are using TypeScript. All “ANY” types should also be fixed unless you have a really, really good explanation for not doing so. Below we have missing props types and any in the method.

const NewComponent = ({items, data}) => {
  const getItemId = (data: any) => data.id
  return (
    <div>
      {items.map(item => (
        <span key={getItemId(item)}>
          <h1>{item.title}</h1>
          <p>{item.description}</p>
        </span>
      ))}
    </div>
  )
}

• Check for variables, functions, and component names. They should all declare what they are and what they do.
  
  
• For boolean values, it’s good to have a prefix is/are/should which declares their behaviour (visible => isVisible) and it will be harder to treat them as html properties.
  
  
• Functions should declare what they do, and if they return anything, they should have something like get – getUsers, if they are manipulating data, they should somehow tell what they are doing – updateUsers => addUniqId, parseData => parseToAPIFormat etc.
  
  
• Check for weird logic patterns (things you have never seen before). Sometimes when a developer takes too much time on a single task – they start to be really creative and create methods or flow that have no sense at all. You should help them here – to point that out and maybe help with a better solution.
  
  
• Check for too complicated code chunks. If someone is adding an ID into an array using 20 lines of code instead of 1, take some actions. Or when you are using some 3rd party packages like lodash, but the developer keeps writing all the methods by himself.
  
  
• If you can’t understand what a specific chunk of code is doing – we need to add a description comment there, or it’s just written incorrectly. In case the first option is viable – add a comment with a description. You can come back to this point in the future – and you still know what it does. If it’s incorrect – it needs fixing.
  
  
• Check for hardcoded names, paths, and values. Separate that kind of code, so you can easily change it in one place. Use paths instead. They are (in most cases) used in routing configuration and in every link and redirection. Also, separate types, date formats and everything that can be used in multiple places – to easily change them.
  
  
• Check for backward compatibility issues like changes in props from optional to required. Or changes in some methods’ parameter types. If you made such a change with TypeScript – it should throw a compilation error. If you are using just JavaScript – you need to track that manually.
  
  
• Check for code repetition. If you’ve seen the same/similar logic in multiple places – point that out. Code should be reusable and if you will need to update that logic, you will have to update it in a single place. Not 3 of them.
  
  
• Check for missing form validations or incorrect form validations. I’ve never done an app that has a form without field validation.
  
  
• Check for missing error handlers from API. If a user receives 500 errors from API, will the user see a message with the correct info? Mostly about try/catch, and what is happening in a catch body?
  
  
• Check for async methods – can they be done in parallel, or do we need all the data in a sequence? Check if we actually wait for this data if we need it, or if we read from the promise object.
  
  
• Sometimes you may notice potential bugs. A big part of knowledge comes with experience. If you see something you’ve done in the past, but it caused errors – don’t make it happen again. Explain that you’ve been there, and you know the way out as you’ve made it work before.

Comments in React Code Review

I think that a good way of segregating the comments is to categorize them.

For example, divide them into at least 3 groups:

• MAJOR – Comments that have a big impact on the code. They can break the app, create potential issues, don’t meet the criteria, have regression issues, etc. They are just comments that have to be fixed before merging.
• MINOR – In here we have some improvements – how we can make the code better and future-proof. Mostly about changing the implementation to a more readable code, more reusable or just better but won’t affect functionality (mostly) :). But if the developer has a good explanation about why it should stay this way – it’s fine to skip these.
• OPTIONAL – just syntax updates or something that won’t change the functionality at all. Like formatting issues or micro improvements.

Remember to communicate with your developer about the comments. That will speed up the process a lot.

Sometimes a simple “Hi, I left a few comments in your PR, please let me know if you have any questions.” is enough.

Summary

Remember – even if 10 people will review your code, it’s still your code, and you are responsible for it.

Setting up some rules will make cooperation much easier.

Don’t forget to point out good things too.

If you think that something is wrong, and you have an idea how to fix it – suggest that – that will speed the process up.

Don’t just add comments like “change A to B” – add a proper explanation of why it should be changed. For example: “Please change the name from “changeUserStatus” to “changeUserData” as we are changing multiple fields in user – not just status.”

And of course, be nice! There is no point in making the developer feel guilty, sad or worthless. Using correct language will change the sentence meaning like “A to B” – “Can you change the name of A to B as it will be more readable”. In other words, give a reason for each change.

Also, remember to communicate about the process status, whenever you want to discuss some solution, or you need some more answers.