How do we maintain the same code formatting across our team in the web department?
Adherence to uniform formatting and code quality rules should be a standard in every development team. See how we approach this problem in our web department.
![How do we maintain the same code formatting across our team in the web department?](https://cdn.prod.website-files.com/633a9e4ad2aac3c0b1ce95df/637ceb09dc119317e9b9fc84_eslint.webp)
This situation has probably happened to you… You pulled code from the repository, made a change, saved the file, and when you looked at the diff, you were horrified to see how the whole file lit up green, when you only changed one line. You can find yourself in the same situation when you did a code review for another developer, who ignored such a wild diff and pushed the change.
In my opinion, these missteps are often mistakenly attributed to junior developers who are not (in an exaggeration) enlightened enough to know how to set up their machine to avoid such situations. But this is the wrong approach. The primary cause of these problems lies somewhere else entirely, namely that the project is not set up robustly enough to cope with the different environments in which individual developers work, or rather to catch these errors if possible before they are pushed into the repository, but more on that later.
Where do problems most often arise?
1. CRLF vs LF
As they say “first things first”. Non-uniform line ends can be a frequent cause of unwanted diffs.
In short, Linux and MacOS use the control (invisible) LF character to mark a new line, while Windows uses the CRLF character. Although a developer cannot tell the difference in an IDE, between a file saved as LF and CRLF, Git can. So if two identical files are uploaded to Git, but each with different check characters for line breaks, it will evaluate them as diff.
There are a couple of ways to deal with this problem. I will not describe all of them, but I would like to describe the solution that we use on projects at Synetech.
The Solution
We don't rely on the developer setting up Git correctly. In the project we have a .gitattributes
and within it * text=auto eol=lf
. This solution reliably ensures that only LF type line endings are allowed into the repository.
2. ESLint, Prettier
Another frequent cause of unwanted diffs can be incorrectly configured work with ESLint and Prettier. Unfortunately, this is again another very broad topic, so I'll just try to briefly explain the basic problem.
Prettier
Prettier is used to enforce the same formatting rules for files across different developers, so at first glance it might seem that if Prettier is configured into a project, it is not possible, for example, for one developer to use semicolons at the end of a line and another not.
Unfortunately, this is actually a possibility. A situation can occur when one developer pushes a new file into the repository, using semicolons at the end of lines everywhere, and after X commits another developer comes with his modification and deletes all the semicolons. The resulting diff is useless and most importantly it does not carry any value and should not be part of the Git history at all.
This problem arises for only one reason. The developer did not run the prettier --write
command that "normalizes" (formats them into the same shape). Many developers run this command with an IDE that will automatically format the file after saving it. However, this functionality is usually provided in the IDE by a plugin. In the case of VSCode, it is the "ESLint" plugin (https://marketplace.visualstudio.com/items?itemName=dbaeumer.vscode-eslint), WebStorm has ESLint integrated without the need to install the plugin, but it must be configured correctly (https: //www.jetbrains.com/help/webstorm/eslint.html). If the developer does not have ESLint in the IDE or has it set up incorrectly (e.g. it is not set to automatically run the format after saving the file), the formatting will not occur and after the push, the unformatted code will be sent to the repository.
ESLint
Rather than unifying formatting (although it can also check such rules: https://prettier.io/docs/en/integrating-with-linters.html) ESLint serves to monitor the "code quality rules". E.g. it can check whether you have any dead code (unused variables), unnecessary code (console logs, alerts). You can find an overview of all the rules that ESLint can check here: https://eslint.org/docs/rules/#layout-formatting. You can probably imagine what happens when one developer lints the code and the other doesn't. It will have exactly the same problem as with Prettier, just on a different level. Reasons for pushing unlinted code to the repository can be the following:
- ESLint can be integrated with Prettier (https://prettier.io/docs/en/integrating-with-linters.html). However, some forms of ESLint integration force some developers to voluntarily disable the ESLint plugin in their IDE. For example, if you let Prettier report errors using ESLint. In this case, you will likely end up in a situation where you are already making some formatting mistakes while writing the code, and the IDE immediately underlines these mistakes for you, and you get rid of them only after saving the file. If this bothers you, you simply disable the plugin and perform autofix only in the console (
eslint --fix
), which you can easily forget about from time to time. - Performance. ESLint is slower than Prettier, therefore in some situations autofix can take a few seconds, especially if you work on a less powerful machine. In this case, again you are more likely to turn autofix off in the IDE and run the autofix command manually in the console, which again leaves room for you to forget to run this command.
- A developer can push bugs to the ESLint repository even if they have ESLint bug autofix set in their IDE or if they run a manual autofix of files before they run
eslint --fix
commit in the console. Not all ESLint errors are (unlike Prettier errors) autofixable. There are also bugs that the developer has to fix manually. If they don't do this and ignore the ESLint messages, they can still release unlinted code into the repository.
The solution
When we were looking for a robust solution at Synetech that would totally eliminate the above problems, we immediately thought that it would be good to run some pre-commit hook that could perform the necessary prettier/eslint error checks, so that the developers would not be allowed to create a commit until they had properly formatted/linted code.
The problem, however, is that pre-commit hooks are not part of the remote repository, but only exist at the level of the local repository, so it is not possible to save information about which pre-commit hooks should be executed in the repository, and thus it is not possible to distribute the pre-commit hook to other developers.
Husky
Fortunately, there is a Husky package (https://typicode.github.io/husky). According to the configuration file (which can be versioned), this package can create pre-commit hooks for each developer at the local level.
Using Husky, it is then possible to set the command eslint --fix
to be executed as a pre-commit hook. The problem is that if you run this command with a parameter for all JS files, you are checking files in the entire repository unnecessarily, which is not efficient. However, Husky has no information about which files you want to commit (which files are in the stage area). This is where another package comes to the rescue.
lint-staged
Lint-staged can run scripts only for staged files. In the pre-commit hook, there is no need to run eslint --fix
, but you can run a lint-staged
script that includes eslint --fix
(or any other check) as a parameter.
Note. For even better performance, it is possible to run the eslint --fix
additionally with the --cache parameter
, which only checks files that have changed since the last time this command was run.
Setup script
So we already know which tools will help us to lint/format the code using a pre-commit hook. Since this information was only piecemeal so far, I will try to explain a little more in depth how to actually configure these tools on a real project.
husky
and lint-staged
can be configured separately and manually, after all, there is already enough written for this in the documentation (https://www.npmjs.com/package/husky, https://github.com/okonet/lint-staged). However, it is interesting that there are also scripts that will do most of the configuration for you. At Synetech, we use npx mrm@2 lint-staged
, to speed up the setup of a project. It does the following:
- It installs
husky
andlint-staged
. - It adds to
package.json
prepare
script, which after the firstnpm
will also generate the folder.husky
in which you can define any hooks. For example, forpre-commit
it would be a pre-commit file containingnpx lint-staged
.
For clarification, This is what a lint-staged
script inside package.json
could look like:
"lint-staged": {
"{*.js,*.jsx,*.json}": "eslint --cache",
"{*.js,*.jsx,*.json}": "prettier --write", // pokud nemáte
Prettier integrovaný do ESLintu
}
99% solution
At the beginning I was selling you the idea that this solution would ensure that no unlinted/unformatted code gets into the code, I wasn't quite telling the truth.
As you might have guessed, pre-commit hooks only start working once they are installed, which happens only after launching npm i
. So if a developer clones a repository, and doesn't launch npm i
, even a commit with problematic code will pass. Fortunately, this is a very rare situation. Usually a developer makes some changes for which they need to install packages first.
The second way to bypass the pre-commit hook is to commit with the --no-verify
flag, which the pre-commit hook ignores and the commit normally goes through, which can actually be useful in exceptional situations.
100% solution
The solution above can be improved a bit further by setting up error checking on the pipeline as well. This does not prevent the developer from making a commit containing errors, but it can prevent this commit from at least merging into the main branch (typically dev/master). Of course, only under the condition that passing pipelines are set as a condition for completing the merge request.
Setting up such a job on the GitLab pipeline can look like this:
eslint:
stage: test
script:
- npx eslint . --ext .js,.jsx,.ts,.tsx
\
- package-lock.json
The last thing that often creates unnecessary diffs that I would like to cover in this article is the package-lock.json file
.
package-lock.json
from npm v7
exists in a newer version called lockfile v2
(https://docs.npmjs.com/cli/v8/configuring-npm/package-lock-json#lockfileversion). While this new version of lockfile
is backwards compatible with lockfile v1
, it doesn't mean that when you generate package-lock.json
with npm v6
, it will look the same as when you generate it with npm v8
, and that's the problem. Because if you have developers in your team with different versions of npm
, each of them creates package-lock.json
with different structures and this creates a completely unnecessary and long diff that usually only lets us know that the developer generated the lockfile according to a different specification, which again is something that shouldn't be in the Git history.
I would also like to mention some of the reasons why developers have a different version of npm
.
npm
is a package installed locally on each developer's computer, and its default installed version depends on the version of the OS the developer is using. For example, if a developer starts using a higher OS version, theirnpm
version can increase as well.- Some projects require using a certain version of
npm
, ornode
, which is closely connected withnpm
, mainly because of the dependencies used on the project, which need a certain version ofnode
. Developers who often change projects requiring a different version ofnode
are often using variousnode
version managers, the most famous of which is probablynvm
, which also exists as a version for Windows (https://github.com/coreybutler/nvm-windows). The problem is that they can easily lose track of what version they need on which project, or simply forget to change theirnpm
.
The solution
The solution is to agree with your team on which version you want to use to maintain package-lock.json
and to unify npm
versions between developers accordingly.
Once you're done, you can create a configuration file for nvm
.nvmrc
, which informs about the version of node
used on the project. The version of npm
depends on the version of node
, so if you want to use, for example, npm v8
, you would enter the value 16
into the folder, because node v16
uses npm v8
. Then the developer just needs to run the nvm
and nwm
will automatically switch to node in the version stored in the .nvmrc
.
The correct version of node
can even be directly enforced on the project. Just add it to .npmrc
:
engine-strict=true
and to package.json
:
"engines": {
"npm": ">=8.O.O" // nebo jakákoliv jiná verze
}
Whether to enforce npm
directly or just recommend, I'll leave up to you.
What do we use ESLint for at Synetech?
In addition to the fact that at Synetech we use linting on projects according to the basic ESLint rules (https://eslint.org/docs/rules/), we have also found use for ESLint in other cases:
Reduction of Git conflicts in imports (autosorting imports)
When a team is working in React, you sometimes run into one interesting problem. Sometimes it happens that two developers edit the same file at the same time, and sometimes both of them add a new import to the file. If a new import is added using the IDE's whisper menu, most IDEs will list the import alphabetically, reducing the risk of import conflicts. Because it is likely that each developer added a new import to a different place in the alphabet. But the problem arises in these cases:
- Not every developer needs to use the automatic addition of used imports in the IDE.
- Imports that have never been used in a project before are not whispered. Typically these are image imports.
In these cases, most developers write the new import manually and add the new imports to the end of the list of imports defined so far. Unfortunately, this workflow creates unnecessary situations where you have to resolve which of the added imports you want to keep, even though you always want to keep both.
However, these situations can be reduced. There is an ESLint plugin eslint-plugin-simple-import-sort
(https://www.npmjs.com/package/eslint-plugin-simple-import-sort) which solves import sort independently of the IDE, so even if the developer adds import to the end of the line, after saving the file (autofix), the import is automatically inserted into the correct place according to the alphabet.
This approach thus reduces conflicts in imports to a minimum. The only situation where a conflict arises anyway is when a new import from both developers happens to be on the same line (the imports have names that are close in the alphabet).
Note: Of course, this solution is most effective for larger files, or files that have more imports. For small files with few imports, there is still a pretty good chance that both of the newly added imports hit the same place in the alphabet.
Automatic deletion of unused imports
You've probably had ESLint reports that said something like "XXX is declared but its value is never read.". This message usually indicates that you have just deleted a component that is not currently in any other place in the file, but you forgot to delete it from the imports at the same time. However, this manual degreasing is a tedious and time-consuming activity. Fortunately, ESLint has the eslint-plugin-unused-imports plugin
(https://www.npmjs.com/package/eslint-plugin-unused-imports), which can automate the deletion of unused imports and automatically delete all unnecessary imports with autofix after saving the file.
Conclusion
I believe that sustainable and correct code formatting needs to be approached from many different angles. We at Synetech have been using this setup on the last few projects and it has worked well for us so far. If we change it, we usually just add new rules related to some technology used in the project, e.g. Cypress or MUI, but that would be a topic for another article…