ESLint Warnings Are an Anti-Pattern
Tyler Hawkins

Tyler Hawkins @thawkin3

About: Staff software engineer. Continuous learner. Educator. http://tylerhawkins.info

Location:
Lehi, UT
Joined:
Mar 2, 2020

ESLint Warnings Are an Anti-Pattern

Publish Date: Jul 13 '22
57 30

ESLint offers three settings for any given rule: error, warn, and off. The error setting will make ESLint fail if it encounters any violations of the rule, the warn setting will make ESLint report issues found but will not fail if it encounters any violations of the rule, and the off setting disables the rule.

I'd like to argue that using warn is an anti-pattern. Why? Because either you care about something, or you don't. The rule is either important to follow and violations should be fixed, or the rule is not important and developers shouldn't worry about it. Therefore, use error or off.


The Problem with Warnings

Now, there's no real issue with using the warn setting. The problem though is that when ESLint rule violations aren't enforced, developers tend to ignore them. This means that warnings will pile up in the ESLint output, creating a lot of noise and clutter.

So do you care about the rules that are being violated or not? If not, why do you have the rule enabled? If a rule serves no purpose and developers aren't addressing the warnings, get rid of the rule. If the rule is important, set it to error so that developers don't have the option of ignoring it.


One Caveat

There is one use case for the warn setting that I feel is valid, so let's address that. Remember, only a Sith deals in absolutes.

When introducing a new ESLint rule to your codebase, you may find that you have too many violations to fix all at once. In this situation, what are you to do? You want to keep your ESLint script passing, especially if you enforce it in your CI pipeline (which you should!). It may be counterproductive to add an eslint-disable-next-line comment along with a TODO comment for every violation of this rule as well.

So, if you're adding a new ESLint rule and find that for whatever reason you can't clean up the violations all at once, set the new rule to warn, at least for now. Understand though that this is a temporary setting and that it should be your goal to clean up the warnings as quickly as possible. Then, once the rule violations have been taken care of, you can change the rule setting to error.


Conclusion

ESLint warnings are an anti-pattern. Use only the error or off settings, and reserve using the warn setting only as a temporary stopgap.

Comments 30 total

  • Gulshan Aggarwal
    Gulshan AggarwalJul 13, 2022

    True, a rule should either pass or fail.

  • Drew Knab
    Drew KnabJul 13, 2022

    I don't agree.

    A warn is simply an acknowledgement of a code style that may be dubious but should be allowed if the circumstance merits it.

    There is a benefit to having a distinction between do not and ought not.

    imho, eslint-disable-next-line is the noisy anti-pattern here.

    • mikew
      mikewJul 16, 2023

      This is confusing, because eslint-disable-next-line + a comment explaining why is exactly what you do when something "should be allowed if the circumstance merits it"

      IMO, if you let warnings slide, you just end up with a lot of warnings. If no one pays attention to the warnings, they're just noise with no signal. So either fix them, or remove them.

      Like others mentioned, try using --max-warnings 0 in CI builds. You can still do whatever the heck you want locally, but no warnings / errors make it into your build. It's so refreshing working on projects where there's no warnings or errors.

  • Rense Bakker
    Rense BakkerJul 13, 2022

    Largely I agree, but on the other hand it can also be a little annoying if eslint makes your build fail even though it would "technically" compile without problems if there was no eslint rule... Dependency arrays in React come to mind for example... Although while not technically breaking, it does make your code perform unexpectedly, so in that sense I agree its good to have en eslint error there...

    • Zenobius Jiricek
      Zenobius JiricekJun 18, 2024

      I'd go further and make the build fail if you didn't comment on why you didn't include all dependencies.

      There's another eslint rule for react that prevents you from using fat arrow functions for useEffect. Instead you're expected to provide a named function whose name explains what's going on.

  • Jan Küster 🔥
    Jan Küster 🔥Jul 13, 2022

    Only thing where I could think of warn as usable is migrating a large codebase from one code style to another while having many contributions going on.

    • Fyodor
      FyodorJul 15, 2022

      And it is a very important case, as this or similar kind of things happens quite a lot in the enterprise world. Business priorities cannot be easily neglected.

  • JWP
    JWPJul 13, 2022

    To me, Eslint is an anti-pattern, as I prefer TypeScript.

    • Tyler Hawkins
      Tyler HawkinsJul 13, 2022

      Seems like an odd comment, no? TypeScript is great for type checking, and ESLint is great for enforcing best practices or agreed-upon coding styles/preferences. They're complementary tools, not competing tools that replace one another.

      Most TypeScript projects use ESLint. The TypeScript core team even abandoned TSLint in favor of supporting ESLint with TypeScript projects.

      • Jakob Attkinson
        Jakob AttkinsonJul 13, 2022

        They're complementary tools

        This! IMO ESlint is a must. I can't really work anymore in a large team (3+ devs) without eslint + prettier.
        Unless it's a 6 months project and I never have to touch the code again :)

    • eBitzu
      eBitzuJul 14, 2022

      Has nothing to do with eslint

    • eBitzu
      eBitzuJul 14, 2022

      Ts doesn't replace eslint

  • Jakob Attkinson
    Jakob AttkinsonJul 13, 2022

    That's actually quite an awesome idea.

  • Volodymyr Yepishev
    Volodymyr YepishevJul 14, 2022

    I would say developers ignoring linter warnings is an antipattern, not the linter proving the warning option :)

  • Joel Louzado
    Joel LouzadoJul 14, 2022

    You might want to check out betterer which allows you to set rules on things like warnings as well and then trigger CI to fail if the number of warnings crosses some threshold.

    The problem with having very strict linting is that it might slow down the team at a time where it might not be appropriate. In practice a policy of "no warnings" means that people see improvement work as being inherantly high-effort / low-reward and the refactoring piles up.

    • Tyler Hawkins
      Tyler HawkinsJul 14, 2022

      Thanks for sharing! I had never heard of Betterer before, but I spent some time this morning looking through their docs. It looks neat!

  • Tyler Hawkins
    Tyler HawkinsJul 14, 2022

    Love it! Someone else commented on this post suggesting to look into a tool called Betterer, and I think this is exactly what you're looking for. I had never heard of it before, but I poked around their docs this morning, and it looks like you can set rules that certain conditions should get bigger (more tests, higher coverage, etc.) or smaller (fewer ESLint rule violations, fewer TODO comments, etc.), and you can set dates for when goals must be met. Seems promising!

    phenomnomnominal.github.io/bettere...

  • Dariusz Cichorski
    Dariusz CichorskiJul 14, 2022

    Just like you said, I think that 'warn' setting is useful for introducing new rules in some old/big codebase. In most cases it's good to clean up the warnings and change the rule to 'error' ASAP. 🙂

  • eBitzu
    eBitzuJul 14, 2022

    I use "warn" for rules that have fixes that will be dealt with at commit using a hook, or at fix at save

  • Fyodor
    FyodorJul 15, 2022

    Remember, only a Sith deals in absolutes

    This phrases should be emphasized 😅

  • Sunny Golovine
    Sunny GolovineJul 15, 2022

    In all my projects I always configure ESLint to error only, no warnings. In my mind it's either a problem or it isn't. It's either an error that needs to be fixed or it's not an error.

  • Riki Fridrich
    Riki FridrichJul 18, 2022

    I like to use warnings in my projects for rules that are fine to break during development (e.g. no-console, no-unused-vars), but not in build. I use --max-warnings 0 option in build script. It looks something like this:

    {
      "scripts": {
        "lint-dev": "eslint .src/**/*.js",
        "lint-build": "npm run lint-dev -- --max-warnings 0"
      }
    }
    
    Enter fullscreen mode Exit fullscreen mode

    I find this really handy. The warnings do not block me during development and debugging. But they show me where the code is incomplete and where my debug logs are. It's easy to find these places and clean them up. Then, with zero warnings tolerance, the debug code will never get into production.

    Also, I use a pre-commit git hook to lint with zero warnings. This way these things never even leave my local computer.

    • Zenobius Jiricek
      Zenobius JiricekJun 18, 2024

      eslint shouldn't stop your dev hot module reload loop anyway?

  • folmert
    folmertJul 28, 2023

    ESLint errors are an Anti-Pattern. They should be enforced only on build or merge request, not on dev environment. No one cares about a missing space when you're fixing a bug. Preventing running the app due to eslint errors is ridiculous.

    • Tyler Hawkins
      Tyler HawkinsJul 28, 2023

      Except for your first sentence, I think we’re all agreed with you here. ESLint is typically run in a pre-commit or pre-push git hook or in the CI pipeline before allowing you to merge. It’s not something you would want blocking you from building or running locally.

      • Zenobius Jiricek
        Zenobius JiricekJun 18, 2024

        sorry no.

        using eslint as your auto formatter will save you all that time later on.

  • Zenobius Jiricek
    Zenobius JiricekJun 18, 2024

    When introducing a new ESLint rule to your codebase, you may find that you have too many violations to fix all at once. In this situation, what are you to do?

    no.

    in this case you create a second eslint.json and you only run this in git staged files.

    .eslintrc.json  
    .eslintrc.ci.json
    
    Enter fullscreen mode Exit fullscreen mode
    • .eslintrc.json is used by your editor and husky/precommit/lint-staged
      • it contains all your new rules plus it extends .eslintrc.ci.json
    • .eslintrc.ci.json is used by your editor
      • this runs on all code in the repo. everything

    this is how you ensure rules migrate from "too hard to cover everything" to "you touch it you fix it"

    no need for warnings anymore.

Add comment