Progressive Linting

During my time at Bocoup I had the opportunity to help a wide variety of clients standardize large, unruly web applications. My colleagues there have been long-time advocates of the value of developer tooling, and we always endeavored to bring those benefits to our consulting projects. Linting and code style tools like JSHint and ESLint make it easier to catch typos before you commit (or even bundle), to painlessly switch between projects, and to allow large groups of contributors to work in a consistent manner.

Something I often hear from client teams is that these tools are all fine and well when you are starting fresh, but it is daunting to add them to existing code. ESLint in particular supports such a range of opinionated style rules that on a large codebase with multiple contributors you’re almost guaranteed to be looking at thousands of issues the first time you run the tool.

The story usually goes something like this: after enthusiastic debate, the team agrees to put ESLint in place. One of the developers goes back to their desk, creates a branch, and tries it out.

✖ 3128 problems (1277 errors, 1851 warnings)

Ok, we think, that’s more than we can take care of today. The branch is abandoned, and the team moves on to more tractable issues. In approximately three months we will repeat this charade.

I understand why teams de-prioritize or disable style checking when they don’t have the bandwidth to do a full audit, but I also believe that the time it takes to add these tools early pays off handsomely. Today I want to highlight an approach I’ve taken with several of my client projects to progressively lint large-scale JavaScript codebases without disrupting their normal sprint activity.

The hardest part of adding a linter is agreeing to the rules you want to enforce—but this post is not about how to do that! Let’s assume to start that your team has elected to use ESLint and has agreed on a set of rules. (The below technique will work with JSCS or JSHint as well.) At this point you may have an npm package script set up to run eslint:

    "scripts": {
        "lint": "eslint --ext .js,.jsx src"

Running npm run lint at this point yields a disheartening number of errors, because we’re checking every file in our project. However, we don’t generally modify every file in our project at once: we work on a single feature or bug, and only touch related files in a given patch or pull request. Git gives us some nice tools for seeing which files have changed between our base branch (“master” in the below snippet) and our working branch:

$ git diff --name-only --diff-filter=ACMRTUXB master

The default Mac OS or Linux command-line terminals both support nesting commands, so we can feed this list of files in to ESLint (this is trickier on Windows, but should work with the Windows Subsystem for Linux or Git Bash):

$ eslint $(git diff --name-only --diff-filter=ACMRTUXB master)

Now I’m seeing ✖ 35 problems (31 errors, 4 warnings), which is much better than the thousands we started with! I can easily fix these errors before I submit my patch, and our codebase will be that much more consistent. But there’s two weaknesses to this command we’ve created: it doesn’t filter out non-JS (or JSX) files, and it doesn’t handle situations where your working branch is behind master.

Let’s look at the latter problem first. Git provides the merge-base command, which will attempt to find a common ancestor for two references. Put another way, if you provide two branches, git merge-base branch1 branch2 will try to find the point at which those branches diverged. Since Git allows you to reference your current working state with the reference head, then you can usually find the commit at which your working branch diverged from your master by running git merge-base head master

$ git merge-base head master

Bash allows us to nest commands as deeply as we want, so let’s swap that merge-base in for our reference to master in the earlier script:

eslint $(git diff --name-only --diff-filter=ACMRTUXB $(git merge-base head master))

No matter what has happened to master since you created your branch, this command should pull out only those changed files and feed them into ESLint. We’re getting close! But ESLint will balk if you try to give is SCSS, HTML, or other non-JS file types that may have changed in your project.

Unix-like systems provide a utility grep, which can many things but in particular will find lines within a provided block of text that match a given pattern. Since our git diff --name-status output places each file on its own line, we can narrow that list down to only JS and JSX files by “piping” the entire list through grep:

eslint $(git diff --name-only --diff-filter=ACMRTUXB $(git merge-base head master) | grep '.jsx*$')

Non-JS files will now be filtered out, allowing ESLint to do its work without interruption.

I like to take this command and expose it with an NPM script “lint:branch”, like in this snippet from my project’s package.json manifest:

    "scripts": {
        "lint": "eslint --ext .js,.jsx src",
        "lint:branch": "eslint $(git diff --name-only --diff-filter=ACMRTUXB $(git merge-base head master) | grep '.jsx*$')"

Now anybody on this project can use npm run lint:branch to identify errors and inconsistencies in only those files they care about right now. If these problems are fixed in a final commit when submitting each new pull request, this approach allows a large team to rapidly fix a large number of errors throughout a codebase without significantly disrupting their regular momentum.

As a bonus, this command can be extended to look at only uncommitted files that you’re working on now:

    "scripts": {
        "lint:diff": "eslint $(git diff --name-only --diff-filter=ACMRTUXB | grep '.jsx*$')"

Set up npm run lint:diff as a pre-commit hook, and you’ll never mistakenly commit syntax errors again.

Leave a Reply

Your email address will not be published. Required fields are marked *

This site uses Akismet to reduce spam. Learn how your comment data is processed.