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:
Source config. Config containing code main lint rules to be used for development.
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 toWARN
.
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
toERROR
1 rule per sprintThe 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:
create a component new-test-app-component identical to
app.component
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
Top comments (0)