Adding strict Eslint rules to the current code without problems

A strategy for applying new or strict linting rules to existing code without loss of development

Problem statement

In a previous story (Create your code style for the team from scratch. Eslint, Prettier, Commitlint and etc.), I broke down a way for a team to create their rules, organize them into packages in the recommended format, and apply them to a repository

Suppose you want to apply new Eslint rules or configs to an existing repository. You run a check on your repository and get a message like this:

1271 problems (1076 errors, 195 warnings)
345 errors and 37 warnings potentially fixable with the  --fix option.

What does this message tell us? — It says that Eslint rules found 1076 errors and 195 warnings. Of these, 345 errors and 37 warnings can be corrected automatically. We can fix only some parts of the code automatically, while the rest it requires manual correction. It is an unpleasant situation.

With lint-staged, any commit will “crash” on validation, because of errors in existing code. If you have a built-in check on the Lint job in CI/CD, it will also crash.

If the project is large and involves several participants, it is unlikely that you will be able to fix all the problems at once, even with auto-fixing. Even if you manage to do that, the current pool of Pull Requests will become conflicting, which will also affect the speed of development and the usability of the others.

We wanted the best, but we got a huge “blocker” of development

Strategies for solving the problem

Three main options can be identified:

• Don’t apply new rules

You realize that introducing new rules generates many problems and there is no time to adapt and correct the code. Your attempt to improve the code and standardize it has failed.

• Disable checks

Rules are needed, but they shouldn’t block work either, and perhaps you’ll look into fixing problems in the future. In this strategy, lint-staged checks do not include Eslint startup, and CI/CD jobs don’t block the pipeline process.

This strategy has a significant downside — your code review has become passive. Every developer can use his own style,
and your code review will be noticeably longer because you need to mark all the places to be fixed yourself when Eslint did it for you.

It is also not a fact that you will allocate time to fix problems in future and that the number of errors has not increased during this time.

• Adapt rules to the current repository

With this strategy, you disable or change the alert type to WARN of “blocking” rules that don’t support auto-remediation. This will take some time to implement, but it’s a straightforward golden mean to get out of the situation. You don’t unilaterally fix the code to fit the rules, which will help not to affect your other colleagues.

Is it all?
There is no more strategy to apply code inspection rules? — There are

Divide & Conquer

This strategy is basically based on the “Adapt rules to the current repository” strategy, with one difference — the old code is checked against the Eslint config you adapted, but the new code is checked against the original Eslint config you originally wanted to apply to the repository.

For this purpose 2 Eslint configs are created:

1. Source config. Config containing code main lint rules to be used for development.

2. Adapted config. It inherits from the source config, but it also inherits from the “temporary rules config”. “Temporary rules” are “blocking” rules of the source config in the ERROR state, but which have no auto-remediation capability. These rules must be obtained imperatively the first time you run the code checker and their status is set to WARN.

Main advantages:

• The approach ensures that new changes within the current repository will already be styled according to the new rules, but leaves the old code checked in. Over time, the new code will supersede the old code, and your repository will follow the current code style without exception.

• To speed up the process, you can build into the work cycle switch from WARN to ERROR 1 rule per sprint

• The approach can be applied to existing repositories with static analytic tools of code if you need to increase code rigour with a new plugin or rules

An abstract scheme for making changes and validating them would look like this:

Implementation

As an example, let’s see how to use the configuration strict from @my-team/eslint-plugin to the repository

Creating adapter eslint config

If we run the lint now, we will get the following log

There are 4 “blocking” rules that do not have autofix. Let’s put them in “temporary rules config” c WARN

• configs/temp-rules.eslint-config.js

module.exports = {
    overrides: [
        {
            files: ['*.ts'],
            rules: {
                '@angular-eslint/no-output-native': 'warn',
                '@typescript-eslint/explicit-member-accessibility': 'warn',
            },
        },
        {
            files: ['*.html'],
            rules: {
                '@angular-eslint/template/no-any': 'warn',
                '@angular-eslint/template/no-call-expression': 'warn',


            },
        },
    ],
};

Also, create an adapted config that inherits from the .eslintrc.js and our temp-rules.eslint-config.js

• .eslintrc.adapted.js

module.exports = {
    root: true,
    extends: ['./.eslintrc.js', './configs/temp-rules.eslint-config.js'],
};

If we run the check now, we should have no errors left behind

Files splitting for validation

Most implementations of lint-staged and husky are as follows starting call lint-staged on the pre-commit hook

#!/usr/bin/env sh
. "$(dirname -- "$0")/_/husky.sh"

npx lint-staged --quiet

Config .lintstagedrc.js from the root of the repository

module.exports = {
    '*.{js,ts,html}': [
        'eslint --quiet --fix',
    ],
    '*.{json,scss,md}': [
        'prettier --write',
    ],
};

So how can we achieve separate checking of our change with two Eslint configs?

To do this, we can refer to the lint-staged documentation and find the --diff-filter parameter. This is a Git parameter that allows you to separate files by how they are type of change (learn more here).

We should only be interested in two parameters — A (added) and M (modified). We will use them to call separate checks. So simple 🔥

Now our file for the pre-commit hook looks like this:

#!/bin/sh
. "$(dirname "$0")/_/husky.sh"

npx lint-staged --diff-filter M -c ./.lintstagedrc.adapted.js --quiet
npx lint-staged --diff-filter A -c ./.lintstagedrc.js --quiet

In addition to the main .lintstagedrc.js config, we also need to create .lintstaged.adapted.js, which will call our .eslintrc.adapted.js config

module.exports = {
    '*.{js,ts,html}': [
        'eslint --quiet --fix --config ./.eslintrc.adapted.js',
    ],
    '*.{json,scss,md}': [
        'prettier --write',
    ],
};

This completes the configuration

Testing

As a check, let’s perform some changes, namely:

1. create a component new-test-app-component identical to app.component

2. in the original app.component, add a few simple changes.

What for? The code of these files is almost identical, but different checks will be applied to them because of the type of change. In the first case, we’ve created conditionally new files that have problems and we don’t want to push them to the repository. In the second case, we changed the old files, but since we need non-blocking validation, we should just get warnings.

If we call git commit such changes, we should only get an error on the new code.

[STARTED] .lintstagedrc.adapted.js — 2 files
[STARTED] *.{js,ts,html} — 2 files
[STARTED] *.{json,scss,md} — 0 files
[SKIPPED] *.{json,scss,md} — no files
[STARTED] eslint --fix --config ./.eslintrc.adapted.js
[COMPLETED] eslint --fix --config ./.eslintrc.adapted.js
[COMPLETED] *.{js,ts,html} — 2 files
[COMPLETED] .lintstagedrc.adapted.js — 2 files

→ eslint --fix --config ./.eslintrc.adapted.js:
/Users/maksim/WebstormProjects/playground/example-styleguide/example-angular-app/src/app/app.component.html
   2:12  warning  Avoid calling expressions in templates  @angular-eslint/template/no-call-expression
  15:17  warning  Avoid using "$any" in templates         @angular-eslint/template/no-any
/Users/maksim/WebstormProjects/playground/example-styleguide/example-angular-app/src/app/app.component.ts
  12:13  warning  Output bindings, including aliases, should not be named as standard DOM events  @angular-eslint/no-output-native
  15:5   warning  Missing accessibility modifier on method definition test                        @typescript-eslint/explicit-member-accessibility
  15:13  warning  Unexpected any. Specify a different type                                        @typescript-eslint/no-explicit-any
✖ 5 problems (0 errors, 5 warnings) ✅

[STARTED] Preparing lint-staged...
[COMPLETED] Preparing lint-staged...
[STARTED] Running tasks for staged files...
[STARTED] .lintstagedrc.js — 3 files
[STARTED] *.{js,ts,html} — 3 files
[STARTED] *.{json,scss,md} — 0 files
[FAILED] eslint --fix --config ./.eslintrc.js [FAILED].

✖ eslint --fix --config ./.eslintrc.js:
/Users/maksim/WebstormProjects/playground/example-styleguide/example-angular-app/src/new-test-app-component/app.component.html
   2:12  error  Avoid calling expressions in templates  @angular-eslint/template/no-call-expression
  15:17  error  Avoid using "$any" in templates         @angular-eslint/template/no-any
/Users/maksim/WebstormProjects/playground/example-styleguide/example-angular-app/src/new-test-app-component/app.component.ts
  12:13  error    Output bindings, including aliases, should not be named as standard DOM events  @angular-eslint/no-output-native
  15:5   error    Missing accessibility modifier on method definition test                        @typescript-eslint/explicit-member-accessibility
  15:13  warning  Unexpected any. Specify a different type                                        @typescript-eslint/no-explicit-any
✖ 5 problems (4 errors, 1 warning) ❌

husky - pre-commit hook exited with code 1 (error)

As you can see, everything worked as intended, and now we can be confident in our style guide, and that new files will follow it without exception.

Resources

GitHub - Misterion96/frontend-style-guide-example: test repository to demo style-guide operation

I'm expanding my network of contacts on LinkedIn. I will be glad to meet new people