TABLE OF CONTENTS

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

Jakub Dakowicz
By Jakub Dakowicz

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 a check from a 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 in 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 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.

Steps to do before you send your Reac code to review

They will help you stay less angry, as the person that prepares the code review will have specific steps to follow before sending the code to you. Win – win.

There are 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, 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 a platform you use. It will help others to pick up the right context. If you will just comment 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 review (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 comments – I recommend stop 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 are 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 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 a 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 components 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 getgetUsers, 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 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, 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 backwards compatibility issues like changes in props from optional to required. Or changes in some methods parameters 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 receive 500 errors from API, will the user see a message with 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 we need all the data in a sequence? Check if we actually wait for this data if we need it, or we read from promise object.

  • Sometimes you may notice potential bugs. A big part of knowledge comes with the 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 working 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, 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 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 the 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 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 to each change.

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


Final word

Sometimes you are just wrong – deal with it.

Leave a Reply