Adding strict Eslint rules to the current code without problems
Tags: tutorial, productivity, javascript, development
Published: 2024-02-29 17:28:33

#### 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.](https://medium.com/@maks-dolgikh/your-teams-style-guide-from-scratch-b9bb206845ac)), 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:

![scheme of committing changes](https://cdn-images-1.medium.com/max/5324/1*Cy7ywsSb5Nf7np8In1Mj6A.png)

## 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

![log](https://cdn-images-1.medium.com/max/2076/0*Ut54CgT2JNVNed9N)

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`

```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`

```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

![log without ERROR](https://cdn-images-1.medium.com/max/2072/0*DRNkQfQ0VcmjofJh)

### Files splitting for validation

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

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

npx lint-staged --quiet
```

Config `.lintstagedrc.js` from the root of the repository

```js
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](https://git-scm.com/docs/git-diff#Documentation/git-diff.txt---diff-filterACDMRTUXB82308203)).

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:

```shell
#!/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

```js
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.

```git
[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](https://github.com/Misterion96/frontend-style-guide-example)


---

I'm expanding my network of contacts on [LinkedIn](https://www.linkedin.com/in/max-dolgikh/). I will be glad to meet new people