How I dealt with over 30,000 ESLint errors

ESLint — Migrating to a Comprehensive Configuration

By Michael Chang

September 30, 2020

image

My Journey

When I first learned about ESLint, I was incredibly excited to enable it for my codebase. I was eager to get all the benefits a codebase with ESLint enabled offered. However, this involved a fairly intensive migration to upgrade my codebase up to my ESLint configuration standard. This was my experience.

My Configuration

I researched a lot about and ultimately decided with this configuration.

module.exports = {
  extends: [
    'airbnb',
    'airbnb/hooks',
    'plugin:jest/recommended',
    'plugin:jest/style',
    'plugin:prettier/recommended',
    'plugin:testing-library/react',
    'prettier/react',
  ],
};

Airbnb’s ESLint seemed to be the the de facto coding standard in the Javascript community, and I included Airbnb’s Hooks rules as well since I was beginning to use React Hooks. I used Jest and React Testing Library so it made sense to also include their recommended ESLint configurations. And finally, I included Prettier since I heard great things about how it essentially eliminates the need to think and worry about code formatting.

Migration

Armed with this configuration, I ran ESLint for the first time. Oh man, did I received a surprise punch to the gut. My codebase had over 30,000 errors! What had I done!?

After a few minutes of shock and disbelief and regretting my initiative, I ran ESLint’s auto fix. This time I only had about 5,000 errors. Well, that was a lot better. Auto fix fixed 25,000 errors but I still had a massive amount to manually resolve.

Turning off rules

At this point, I just had to sit down and resolve them one by one. I first started by cataloging every single rule that was violated. Then, I took all of those rules and turned them all off. And there we go — I passed lint!

Well, not exactly. While my code did seem good because ESLint is not throwing any errors, I simply masked the issue by disabling the rules. But this was intentional — I wanted to deal with the violations one rule at a time without being distracted and overloaded with all the errors at once. All told, I had disabled over 50 rules. Now the real work began — to enable them one by one to fix the errors.

The Ones that were Simple

Some rules were incredibly simple to fix. Rules, such as no-unused-vars, no-constant-condition or react/no-unused-state involved simply deleting the problematic code without having to understand the code. If only there were more of these simple fixes.

The Ones that were Complicated

Some rules required a lot more complicated. Rules such as import/cycle, which made me reorganize the exports and imports to avoid a circular dependency, required a lot of effort to resolve. This made the risk of unintended bugs and regression higher so extra effort was also needed to validate the change. While I didn’t like the amount of effort I had to put in for each rule violation, I had to put my head down and just do it. Fixing these rules were definitely the low point in this entire effort.

The Ones that I Learned From

Some rules I learned a lot from. In particular, I learned about accessibility from the jsx-a11y rules. Rules such as jsx-a11y/no-static-element-interactions, jsx-a11y/interactive-supports-focus, and jsx-a11y/click-events-have-key-events taught me that static elements such as divs that have some interaction (e.g. onClick) need to have a role, a tabIndex, and a keyboard event so assistive technologies can interact with the element. This was a nice side-benefit that I did not expect.

The Ones that Needed Tuning

Some rules were just unavoidable. But fortunately, they offered configuration options that allowed what I needed so I didn’t need to permanently turn the rule off.

module.exports = {
  rules: {
    'no-empty': ['error', { allowEmptyCatch: true }],
    'no-underscore-dangle': ['error', { allow: ['_id'] }],
    'react/jsx-filename-extension': [
      'error',
      { extensions: ['.js', '.jsx'] }
    ],
  },
}

The Ones that Needed Inline Disables

Some rules are good in principle but have cases where they need to be disabled. The react/jsx-props-no-spreading rule errors if you spread props into JSX but this is necessary for Higher Order Components and for hooks-based libraries such as React Dropzone or React Table. Since I only expect to have a few instances where prop spreading is needed, I opted to disable the rule inline instead of permanently.

const withAuth = (WrappedComponent, permissions) => {
  const PermissionCheck = props => {
    const hasPermission = ...;
    if (hasPermission) {
      /* eslint-disable-next-line react/jsx-props-no-spreading */
      return <WrappedComponent {...props} />;
    }
    return <Redirect to="/" />;
  };
  return PermissionCheck;
};

export default withAuth;

The Ones that were Temporarily Disabled

Some rules I would love to enable, but would take way too long to complete in one sitting since they accounted the bulk of the errors. As an interim fix, I would temporarily disable them and slowly work on fixing the errors folder by folder until one day, hopefully, these rules can permanently be enabled.

ESLint provides an overrides section that can target files and apply rules specifically to those files. Using this, I disabled entire folders which later on, I removed the disable one by one and resolved the errors.

module.exports = {
  overrides: [
    {
      // todo: slowly enable
      files: [
        'src/views/dashboard/**/*.js',
        'src/views/users/**/*.js',
        'src/views/settings/**/*.js',
      ],
      rules: {
        'react/destructuring-assignment': 'off',
        'react/prop-types': 'off',
      },
    },
  ],
}

The Ones that were Permanently Disabled

Fortunately, I didn’t think any rules warranted this. There certainly were rules that were a pain in the ass but I understood the reasoning behind them. I wanted to do my best to follow the Airbnb coding standard and framework recommendations and that meant keeping all rules enabled.

Overall Process

All told it took almost a month to fully integrate my ESLint configuration. But don’t let that scare you off! Resolving the simple issues and fine tuning the rules took just a day. The more complicated and effort-intensive ones took about a week. Between these, 80% of the ESLint errors were resolved.

All that was remaining were the ones I temporarily disabled for problematic folders. These, I took my time with. Since I only disabled them on a folder by folder basis, any new code not in those folders would have those rules applied. This, I felt, was a great compromise between completing the ESLint migration and the competing time pressures for product feature development.

Final Thoughts

While the entire process migrating ESLint configurations was lengthy, I was happy that I did it. Because of this initial effort — to setup a proper ESLint configuration and fix the codebase — we no longer need to think about code formatting or spend time worrying about code quality, and as a result, can spend all of our time being productive. For this peace of mind, the effort was well worth it.

Resources



Continue Learning