Steven Sacks

2024-07-26

Level up your workflow with ESLint fix on save

Prettier, ESLint, and Stylelint are amazing tools. When used together and fully configured, they dramatically improve the speed and efficiency that you and everyone on your team write code, improve the overall quality of code, reduce bugs, make code reviews quicker, and empower your entire team to focus on what really matters.

Some teams only have ESLint setup to run on commit or worse, only in a CI action. But doing it that way misses out on the incredibly helpful automation that ESLint fix-on-save provides.

Embracing Prettier’s philosophy

If you haven’t read Prettier’s philosophy, I strongly suggest doing so.

Prettier is fix on save. Its formatting rules are applied automatically when you save a file.

A side benefit of using Prettier is that it also becomes a way for you to know if you have a syntax issue somewhere in your code. When you save, if the expected formatting does not happen, it means there’s a syntax error such as an extra character or a missing closing brace or tag. Or, if the code gets formatted in a way you did not expect, you might have a misplaced closing brace or tag.

What is ESLint fix on save?

Many ESLint rules accept the --fix flag, which will automatically refactor code to adhere to the rule, when possible.

Just like Prettier, your code will be fixed automatically in real-time. Anything that isn’t fixable, you’ll be notified about immediately.

JetBrains IDE (WebStorm, etc.)

Open Settings (⌘ + ,) or Ctrl + Alt + S on Windows
Select Languages & Frameworks > Code Quality Tools > ESLint
Check the box next to “Run eslint —fix on save”

VS Code

If you are using ESLint extension for VS Code open your settings.json file:

Press ⌘ + Shift + P or Ctrl + Shift + P on Windows.
Type “Open User Settings (JSON)“
Add the following code to your settings.json:

{
  "editor.codeActionsOnSave": {
    "source.fixAll": true
  }
}

Vim

If you are using ALE Vim plugin add the following code to your .vimrc:

let g:ale_fix_on_save = 1

What is Stylelint?

Stylelint does what Prettier and ESLint do, but for CSS.

Benefits of ESLint fix on save

Like many things in life, the benefits of ESLint fix on save become quickly apparent by using it rather than describing them. However, here are some of the best ones:

Prevent issues immediately
Automatic enforcement of high quality code
Improve focus by reducing unnecessary decisions while coding
Reduce or eliminate discussions around code style during code reviews
Consistent code makes it easier for everyone to work together on projects
Write code without worrying about formatting or rules
Saving a file becomes a powerful tool to “organize” what you just wrote
Over time, everyone will naturally start writing high quality code that adheres to the rules without needing to be fixed by ESLint
Junior developers get a built-in teacher to learn how to write better code

Choosing rules

As stated in Prettier’s philosophy:

People get very emotional around particular ways of writing code … Even if Prettier does not format all code 100% the way you’d like, it’s worth the “sacrifice”

Prettier’s customization is intentionally restricted. However, with ESLint, there are many rules, and along with that, many opinions.

While you have the ability to customize rules to match the needs of your project and preferences of your team, there are some standard rules that form a strong foundation.

The most important thing is that your ruleset helps avoid common (and not-so-common) issues, and has as many fix-on-save rules as possible to make it easy on developers.

After you take care of the foundation, there are some rules which can be configured one way or another. Some rules might come to down to preference, and it’s up to your company culture how to decide, but do not skip these rules and leave it up to everyone to decide on their own. That will lead to a mess. It’s worth the “sacrifice” to decide, especially if the rule supports --fix.

Recommendations

I will go into detail about recommended rules at the end of the article, including foundational rules and opinionated rules.

Do not use “warning”

Your ESLint rules should either be “error” or “off”.

A rule is not a rule unless it’s enforced. Be decisive.

Run on commit

In addition to fix-on-save, you should also run ESLint automatically on commit. This will catch issues such as those caused by a refactor which made changes in files that you did not explicitly open.

If you’re using TypeScript, you should make sure you’re running tsc on commit, as well, since this will catch TypeScript issues that were not caught by ESLint.

It is crucial that commits get rejected if there are any ESLint or TypeScript errors. This will prevent careless mistakes and bugs, and reduce cruft. We’re all human, we all forget things, even during a single coding session. It only takes a moment for a developer to fix their ESLint and TypeScript violations when they commit.

If you don’t stop issues from getting committed, the burden falls on other team members to find and fix them, or worse, they get missed entirely and end up on production.

The --max-warnings=0 flag in ESLint takes care of this automatically.

The easiest and most common way to run on commit is with lint-staged via husky.

lint-staged

This will save time by only running linters on files that have changed.

Here’s the .lintstagedrc file that we use:

{
    "{src,.storybook}/**/*.{ts,tsx}": [
        "eslint --fix --max-warnings=0 --cache --cache-location ./node_modules/.cache/eslint .",
        "prettier --write"
    ],
    "src/**/*.css": [
        "stylelint --fix",
        "prettier --write"
    ],
    "src/**/*.json": [
        "eslint --cache --fix",
        "prettier --write"
    ]
}

In Remix projects, replace src with app.

husky

You can configure husky to only run if there have been changes to the code.

We also run our test harness on commit. Even though lint-staged and test can be configured to only run on changed files, tsc runs on the entire project. Only running when relevant files have changed saves time on commits which don’t change code (documentation changes, for example).

Here’s our pre-commit configuration for husky:

#!/usr/bin/env sh
 
declare HAS_SRC_CHANGED=$(git diff --cached --name-only --diff-filter=ACDM | grep 'src/')
declare HAS_TEST_CHANGED=$(git diff --cached --name-only --diff-filter=ACM | grep 'test/')
 
if [[ $HAS_SRC_CHANGED || $HAS_TEST_CHANGED ]]
then
	npm run typecheck
	npx lint-staged
	npm run test:lint-staged
else
    echo "\n✅  No changed files -- skipping lint-staged\n"
fi

In Remix projects, replace src/ with app/.

package.json

Here is an example of relevant package.json scripts:

{
  ...
  "scripts": {
    "lint": "eslint --fix --max-warnings=0 --ext js,jsx,ts,tsx src --cache --cache-location ./node_modules/.cache/eslint .",
    "lint-all": "npm run typecheck && npm run lint && npm run format && npm run stylelint",
    "format": "prettier --ignore-path .prettierignore --write \"**/*.{ts,tsx,html,md,mdx,css,json}\"",
    "stylelint": "stylelint --fix src/**/*.css",
    "typecheck": "tsc",
    "test": "vitest",
    "test:ci": "vitest --run --passWithNoTests --coverage --bail 1",
    "test:lint-staged": "vitest --run --changed --passWithNoTests --bail 1"
  },
  ...
}

In Remix projects, replace src with app.

Included are scripts that you can npm run manually if you need to.

lint runs ESLint on all files
lint-all runs everything (tsc, Prettier, ESLint, Stylelint) on all files
format runs Prettier on all files - customize the filetypes as needed
stylelint runs Stylelint on all files

Add to existing projects

Once you agree on your rule set and have everything configured, you’ll likely have a lot of code changes that are required.

The good news is that the majority of the changes will be automatic, and you’ll only have to fix a few things manually.

The bad news is that committing these types of changes will result in code conflicts with open branches, which can be a pain to resolve.

There are two ways to deal with this:

Schedule a time where all open branches will be merged, make the ESLint changes in a single PR, and (hopefully) get them all done in a day. Have your most senior developer take the lead on this, and the rest of the devs can watch and learn via screenshare, or be given non-coding tasks such as planning, a hackathon, or other team-building exercise.
Incrementally implement on a file-by-file basis as you go.

Incremental implementation

In order to do this:

Make sure you have eslint-plugin-eslint-comments installed and this rule enabled: 'eslint-comments/no-unused-disable': 'error'.
Do not enable ESLint fix-on-save or lint-staged (yet)
Inject /* eslint-disable */ at the top of every file in your project using a node script (see below) and commit that with --no-verify.
Run npm run lint to remove the eslint-disable on files which do not need it (because nothing in the file is in violation of any ESLint rules, as per the above no-unused-disable rule)
Merge the PR into your main branch
Have everyone merge main into their branches

Adding eslint-disable is unlikely to cause any merge conflicts and now every developer can choose when to remove that line, run fix on save, and refactor as needed.

For files which require more comprehensive refactoring, you can remove the eslint-disable line to partially fix a file and add the eslint-disable back again, and revisit the file on a dedicated refactor branch in the future.

Over time, your code base will get to a point where all of the eslint-disable lines are removed. Make this a top priority in order to reap the benefits as soon as possible.

node script to inject eslint-disable

In your project, add this file to the root of your project (you will delete it after you use it).

import fs from 'node:fs';
import path from 'node:path';
 
export const getAllFiles = (dirPath, arrayOfFiles = []) => {
  fs.readdirSync(dirPath).forEach(function (file) {
    if (fs.statSync(`${dirPath}/${file}`).isDirectory()) {
      arrayOfFiles = getAllFiles(`${dirPath}/${file}`, arrayOfFiles);
    } else {
      arrayOfFiles.push(path.join(dirPath, '/', file));
    }
  });
 
  return arrayOfFiles;
};
 
const INJECT = '/* eslint-disable */\n';
 
const injectEslintDisable = () => {
  const files = getAllFiles(path.resolve('src')).filter(
    (file) => ['.ts', '.js', '.tsx', '.jsx'].includes(path.extname(file))
  );
 
  const len = files.length;
 
  for (let i = 0; i < len; i++) {
    const readFile = fs.readFileSync(files[i]);
    if (readFile.includes(INJECT)) continue;
    const withInject = INJECT + readFile.toString();
    fs.writeFileSync(files[i], withInject);
  }
};
 
injectEslintDisable();

If you’re using Remix, change src to app.

In your package.json scripts, add this, and then call: npm run inject

"inject": "node inject-disable.mjs"

You can now delete the inject-disable.mjs file.

Recommended Rules

In the following articles, I go into detail about the ESLint rules I recommend.