No matter what kind of developer you are, reviewing is part of your daily responsibilities when working in an engineering team. Being a React developer is no exception to this rule. There are a lot of resources out there that will teach you how to write better React code, but there are barely any articles, videos, or tutorials out there that will help you improve at reviewing React code.
Although reviewing your colleagues’ code is a crucial part of our responsibilities as a developer, it’s not a responsibility that a lot of developers look forward to. They feel like it’s boring to read code instead of writing it, it’s not meaningful to you as a reviewer, and the only thing you’re doing is gatekeeping colleagues.
Personally, I used to share the same opinions and wasn’t too fond of reviewing code. But after spending a lot of time on this topic and diving into different aspects as a React developer, I noticed that the issue boiled down to the fact that I didn’t know how to properly review React code.
My routine would be opening the Merge Request (MR), just reading through all the code top to bottom without any strategy, and adding review comments on anything that I noticed. Not surprising to say that I didn’t enjoy it and the reviews I provided weren’t high quality.
Nowadays, I always go into reviewing React code with a certain plan and topics in mind that I’ll focus on. Since I started doing this, reviewing became less boring and the quality of my reviews increased as time went on, which I received as feedback from my colleagues.
I would even go as far as saying that I now really enjoy reviewing the code of my colleagues. Reviewing their code provides me with a different way of learning about their style of coding, understanding their code, asking questions about code, learning new things, and most importantly providing feedback on their work to enhance the quality of the codebase altogether.
This article shares all the questions that I ask myself while reviewing React code. If you’re not sure how to approach reviewing React code or don’t know what to focus on, then these questions will help you get started. Using these as a foundation will allow you to create your reviewing checklist as I did, start making reviewing a conscious process, provide more meaningful reviews to your team, and maybe even start enjoying it.
The most important aspect to reviewing any is making sure that the code works. This is not necessarily something that you can easily verify in the code itself. Most of the time you’ll be relying on continuous integration (CI) or checking out the code for yourself locally. It’s also expected that the code works when provided to you in the form of a MR.
But despite all of that, it doesn’t hurt to question whether the code will work when reading through it. In the worse case, you’ll understand the code better. In the best case, you’ll notice some details that the code proposer missed and can help to improve the quality.
Oftentimes, developers will review a MR like they’re a linter or some other static analysis tool. They will only pay attention to the implementation of the code and check whether everything was implemented properly. The problem is that a respective static analysis tool can do the same thing quicker, more efficient, and more reliable. While it’s fine to pay attention to details, it shouldn’t be your main focus without understanding what the code actually does.
Whether it’s to provide more meaningful feedback or to anticipate working on it in the future, you’ll first have to understand what’s going on. in the MR. when thinking along with the code proposer and doing things a static analysis tool can’t do. But to do so, you first need to understand what’s going on in the MR. This involves everything, like the context, the purpose, and the implementation.
We shouldn’t only be content with code that works. It should be a minimal requirement, but not the deciding factor. In the end, code that is merged into the codebase and shipped to users will have to be maintained. But unlike the code itself, the developer who created it will not be around forever to work with it. Even the creator itself will most likely struggle with understanding their own code several months later.
For this reason, it’s important to have readable code. Having more readable code means other developers will be able to understand the code and perform maintenance related tasks more easily in the future. Specific examples are content states, inline conditional rendering, and custom hooks, but also where to place the code, organization, and the naming of variables and functions.
But readability is not only limited to specific code patterns or topics. It’s about all the code as a whole and how easy developers will be able to get through the code to understand it. In the end, readability is a long-term investment for the engineering team where the payments are done during reviews.
A classic anti-pattern in software development is the so-called God object. This refers to any programming instance, like an object, class, or function, that hoards all the responsibilities. It will know too much, do too much, and contain too many intertwined flows. The results are an instance that is difficult to maintain, understand, refactor, work in, and will be very fragile.
Similarly, we should avoid this in React development. Especially with React, where development is focused on reusability, extensibility, and single-responsibility, it’s important to consider this. Pay attention to React components and hooks, try to understand their purpose, and verify whether they’re doing too much. If so, then it’s wise to suggest addressing this by abstracting it into a component or hook to prevent the quality of the codebase from decreasing.
On the other side of not abstracting enough, thus potentially creating a God component or hook, there’s also the other side of abstracting too much. Making everything a component or a hook can ultimately also lower the quality of the React codebase.
Having an additional component in between layers can introduce prop drilling, a React specific anti-pattern, or prevent the recommended composition pattern. While creating a new custom hook for every piece of logic can result in too many abstractions, making the codebase unorganized and cluttered.
When reviewing, you’re the first reader of the code and thus the prime candidate for experiencing these consequences. Keeping this in mind and constantly asking yourself whether the chosen construction is necessary can allow you to contribute towards the code architecture without actually touching the code.
Whether it’s function parameters, React component’s props, or the custom hook parameters, implementing them is not an easy task. Ultimately, it’s a form of API design. In particularly with React props, it’s easy to create an API that is convoluted and redundant.
If you have two boolean props that target a similar part of the UI, maybe it’s better to consider an enumeration prop. If you have two props that are always used together, maybe it’s better to combine them into one. If certain props are only relevant for certain branches based on an enumeration or boolean prop, maybe it’s worth splitting the component into multiple components. If you’re passing an array and a render function for the individual items, maybe it’s better to consider the render props pattern. If you’re prop drilling a lot, maybe it’s worth considering the composition pattern.
All of these are concrete examples of suggestions you can make when considering the API design of React props. But it’s not only limited to these specific examples or props in general and could also apply to utility functions and custom hooks. The most important thing is to keep it in mind, try to understand the proposer’s API design and provide feedback accordingly.
This question sounds a bit weird as normally you would expect there to be tests. What’s the point of considering this when I’m reviewing? But in reality, you’ll be disappointed at how often tests are forgotten and neglected. Making sure new features, fixes, or code in general is tested will help out tremendously long-term for the engineering team.
This doesn’t only cover writing news tests for new code, but also updating the old tests in the relevant cases. In general, having at least one test that documents the purpose of the MR and the code that is added should be a minimum requirement. If that’s not the case, it could warrant a request for changes depending on how the team deals with this.
Some people argue that having any test is better than none, but I don’t quite agree with that opinion. While having tests in general can be the first step in ensuring the code quality of the codebase, it can also be a very deceiving step. Bad tests can lead to a false sense of security, shipping code that is not actually working, and wasted time and effort.
When the code proposer has included tests, make sure to check whether they are doing what they should do. The setup, the trigger, and the verification. Are they dependent on implementation details? Is the setup correct? Are we reaching a proper state? All of the different parts can be implemented very differently but even the smallest change will affect the trustworthiness of a test significantly.
An integral part of web development is that our application should be accessible for every type of user. Unfortunately, not every web application is optimized for screen readers, control using keyboard, or other groups.
No developer will know how to implement proper accessibility for all the features that exist. As a reviewer, be the person that remembers this topic, remind the proposer if they’ve forgotten to implement it, provide resources, or make suggestions if necessary.
The child that is most often forgotten about when refactoring or changing existing code is documentation. Whether it’s code comments, documentation pages, or READMEs, it’s always a dangerous act to assume that they’re up-to-date.
If you know that there are official docs, documentation pages, or comments that need to be updated, then definitely mention them in reviews. Especially when the code proposer is not normally working in a certain part of the codebase, it’s almost impossible to know about every single piece of existing documentation. But keeping them up-to-date is a team effort, including you.
One of the most important aspects of reviewing your colleagues’ code is knowing what to pay attention to and making it a conscious process. Unfortunately, a lot of developers don’t do this, which makes reviewing feel boring, tedious, and meaningless.
But in reality, reviewing can be entirely the opposite and an extremely effective way of ensuring code quality. To help with this, this article presents 10 questions that you should ask yourself when reviewing React code. These will help you realize topics to focus on, create a reviewing routine for yourself, and enhance the quality of your review.