Easier code reviews with ESLint and React

This is an extended write-up of a talk I delivered at the React London meetup — the video is here.

Photo by NESA by Makers on Unsplash

Who uses ESLint?

ESLint is a pretty common tool in the front end armoury. It’s the industry standard for static code analysis of JavaScript (AKA ECMAScript, hence ESLint). It gets about 7 million downloads a week, which is a lot (for comparison, React gets 5.5 million).

We use ESLint to avoid errors (e.g. no undefined variables), code in a consistent style (semicolons or not, indentation) and keep our code tidy (no unreachable code).

Most people use a slight modification of a typical ESLint configuration. ESLint comes with with a set of rules and a default configuration, as does the React ESLint plugin. The Airbnb config is also popular.

However, if you’re just using a standard ESLint config, you’re probably not using the full power of ESLint. You probably have team opinions that are passed from developer to developer, maybe by word of mouth or code reviews. A lot of the time, we could codify those unwritten rules and make them actual rules. ESLint rules.

That’s what I’m going to go through in the rest of this post — how to write an ESLint plugin and a rule, focusing on writing a rule to use with React code.

The problem

To illustrate the rule we’re going to write, I’ll first set out the problem. This example isn’t a major issue, but it was enough that when I spotted it in a code review I passed it back to my teammate. ESLint can do a lot more than this, but it’s a nice example to demonstrate it.

MyComponent with the Fragment being required

This bit of code isn’t the actual example, but it illustrates the problem. This sort of conditional logic became much easier when React 16.2 was released and introduced Fragments. They allow two (or more) components to be returned from the same component, or same expression, as siblings, without any extra DOM element.

This is all lovely until we refactor it, extracting ThatButton and ThatOtherThing into a new component, ThatButtonAndThatOtherThing:

Following a refactor, the Fragment is no longer required.

The extra Fragment wrapper isn’t doing anything here. It isn’t doing any actual harm to the component, but it’s just making it slightly untidy.

I spotted it and asked the developer to tidy it up. But then I remembered developers are supposed to be lazy. I don’t want to have to spot this in a review, I want a computer to catch it, whether in CI or in the original developer’s editor, or both. Sounds like a case for ESLint. But the rule didn’t already exist! (Or at least I couldn’t find it.) So I set about writing my own rule.

Automation — credit XKCD

A new ESLint plugin

Because this was the first rule we’d written internally, I had to start my own ESLint plugin from scratch. I could also have raised a PR to get it merged into someone else’s plugin, but I wanted to use it straight away!

Fortunately, there are tools to do most of the setup of an ESLint plugin for you. It’s just these four commands:

Commands to create an ESLint plugin

This is using a tool called Yeoman (“yo”), which you may have come across. It’s a tool for scaffolding new projects, based on generators which are elaborate templates. We’re using the ESLint generator, cleverly titled generator-eslint. We install these globally as they’re tools to set up our project, not part of the project itself.

We then run the generator with the “:plugin” suffix to tell it to create a plugin. It then asks a couple of questions, like the package name and description, and sets up an empty project.

Unfortunately the generator is a little out of date, so the first thing I did in the empty project was upgrade it to the version of ESLint we’re using in our project.

A new rule

Creating a rule is even easier. Again, this is using the yeoman generator, but this time for a rule.

Command to create a new ESLint rule

One of the questions it asks is for an example of code that will fail the rule, so I went for the simplest example I could think of:

Minimal invalid code example

How does ESLint work?

ESLint works by parsing our source code files and turning it into an Abstract Syntax Tree, or AST. This is basically a representation of the source code as a tree of objects. Here’s a very simple example:

Simple code example for AST

would correspond to

Corresponding AST (simplified slightly)

The simple variable declaration snippet gives this AST (this is a slight simplification in that I removed some noise, but it illustrates the concept. We have an object with type “VariableDeclaration”, with kind “const”. This has an identifier (id of type “Identifier”) with the name “val” and an initial value, which is a “BinaryExpression” made up of two literals and an operation.

ASTs beyond this simple example can get quite large quite quickly, but there are some awesome tools to explore them. One of these is AST Explorer, which can be set up to use a range of parsers (including espree, which ESLint uses). We can type whatever code we want, then click on it to highlight the corresponding elements in the AST on the right.

To write our rule, we just need to look for the particular pattern in the AST and report an issue if we find it.

The Awesome AST Explorer

This example, which corresponds to our problem, shows that the Fragment is a JSXElement with an openingElement of type JSXOpeningElement.

Test-Driven Development

Writing ESLint rules is a great fit for TDD. We can write an example of code to check, then iterate on the rule until it gives the right outcome, and repeat. Fortunately, the yeoman generator has already set up a test for us.

ESLint comes with a great test framework using an import called RuleTester. The generator added mocha to run these tests, but they work fine with any JS unit test library that supports the “describe” and “it” keywords. Because I’m used to it, and because we’ve already got it installed in the monorepo this sits in, I switched this to use Jest. I also set up Babel so that ESLint can understand our JSX.

Setting up Jest, because the team is more familiar with it

This is the unit test that the generator set up (with me having added parser: ‘babel-eslint’ to make it run):

The initial rule that the yeoman generator set up (less imports)

This is using that snippet I provided when I set up the rule. The rule tester will run my rule with this code and check that I get the right set of errors. The “type” is used to check that it is assigning the error to the right element, which is useful when you have ESLint integration in your text editor. We can change the message to the sensible error we want (“Remove redundant Fragment”).

Now we can just put Jest in watch mode and it will keep rerunning our test as we edit the rule.

Writing the rule

ESLint rules are based on the visitor pattern. In practice, this means we return an object where the keys are node types (from the AST) and the values are functions to run to check that node.

The AST from AST Explorer in more detail

We saw in AST explorer that our Fragment in JSX was a node with type JSXElement. So to begin with, let’s look for JSXElement nodes:

Initial implementation of the rule, reporting errors against all JSXElement nodes

This is all a rule is — some metadata and a function to create our rule. Context is what we use to report the error. For now, we’re reporting every JSXElement as invalid.

If we look at the test, it’s not passing because the node type is incorrect. We’re reporting against the JSXElement, so we can just change our test expect an error against the JSXElement (just pass this as a string), and we’re good!

Refining the rule

Our test is passing, but we know that our rule is a bit overzealous — it’s currently reporting any use of Fragments. We can now add a new test which our current rule will fail, with a simple piece of valid code.

Adding a test case for valid code

For the valid case, we don’t have to provide an expected error obviously. This test is now failing. The logic we need is “the Fragment is valid if we have two or more children”. Back to the AST to work out how to do that:

We have an array of children, which makes this logic really simple — if that array has length >= 2, the Fragment is fine. Let’s put that in code:

The rule updated to only report errors against Fragments with 1 or fewer children

This is basically the cycle of refining the rule. Identify a case that the rule doesn’t handle properly, write a test for it, then tweak the rule to handle it.

In our version, we’ve added a few more tweaks:

• Rather than reporting against the JSXElement, we can report against the JSXOpeningElement. This looks nicer in your text editor.
• When the code is properly indented, the whitespace is included in the array of children. We want to ignore that, so we filter the list of children.
• Fragments can be called as React.Fragment or as the <></> shorthand in newer babel versions. We should handle that.

If we apply the necessary tweaks for this and split it up into sensible functions for readability, we get something like this:

The rule updated to handle the additional cases listed above

Fix the code

Another handy feature of ESLint is that in some cases it can automatically fix the code. The general guidance for this is to do as little as possible to fix your rule, and leave other bits to other plugins. We don’t try to reformat the code or remove an unused import, we just do what we need to to fix our error.

In our case, fixing the code is just deleting the Fragment. When we report an error, we can provide a fixing function to do this. This function receives a “fixer” as a parameter, which we use to interact with the code. To fix our error, we just use the fixer to remove the opening and closing elements. This will leave the code badly formatted, but we can leave that to something else (Prettier) to sort out.

Updating reportError from the previous example to add a fixing function

Use the rule

We need to add the rule to our ESLint config to use it.

We can publish our plugin package to NPM, or to an internal registry, or just install it as a local filepath or git repo URL. We then need to tell ESLint to load up our plugin and treat our rule as an error.

Example ESLint config for create-react-app users

First we load up the plugin, using the shorthand where we leave our “eslint-plugin-” from the package name. Then we configure our rule (prefixed with the plugin name) to be treated as an error — 0 is accept, 1 is warn, 2 is error.

It works!

And finally, here is a screenshot of the error in practice in my text editor!

Error appearing nicely in my editor

Now all the developers on my team can quickly see when they need to tidy up a Fragment like this and we don’t depend on spotting this in a code review. Once you’ve written one rule, writing more is easy, and the accumulation of these rules will make the entire codebase follow a more standardised pattern.