Introducing ESLint to your codebase smoothly
When adding a linter to an existing codbase, my methodology is as follows:
- Create lint config files and approve them with the team (people have strong opinions about tab widths), add them to the repo
- Run the linter a single time over the entire codebase, autofixing what it can, and adding comments to ignore what it can’t.
- Add some pre-commit / pre-merge rule to run the linter and preventing new violations being added
- Grepping the ignore comments we added in #2, fixing their violations carefully
While it might seem a bit complex, I like it because
- Each commit is easy to review - either small or non-risky (adding comments)
- There are no “no one commit, I’m doing a thing” moments that sometimes occur when refactoring
- We’re not blocked on “fixing all violations” before introducing linting
Ruff is great for this as it has --add-noqa
, which adds “ignore” comments over every violation it finds.
I recently had to do the same with ESLint, which doesn’t have that feature.
I had to improvise.
Solution
With some LLMing, I composed a script that iterates over ESLint’s errors, and adds an “ignore on next line” comment before it
// add_comments.js
const fs = require('fs');
const path = require('path');
const errorsFilePath = process.argv[2];
if (!errorsFilePath) {
console.error('Please provide the path to the eslint errors JSON file');
process.exit(1);
}
const errors = JSON.parse(fs.readFileSync(errorsFilePath, 'utf-8'));
errors.forEach(file => {
const filePath = path.resolve(file.filePath);
let content = fs.readFileSync(filePath, 'utf-8');
const lines = content.split('\n');
const rulesMap = new Map();
// Multiple rules per line
file.messages.forEach(message => {
const line = message.line;
const ruleId = message.ruleId;
if (!rulesMap.has(line)) {
rulesMap.set(line, []);
}
rulesMap.get(line).push(ruleId);
});
// Reverse order to avoid spoiling next lines
for (let line of Array.from(rulesMap.keys()).sort((a, b) => b - a)) {
const ruleIds = rulesMap.get(line).join(', '); // Join rule IDs for the comment
const comment = `// eslint-disable-next-line ${ruleIds}`;
lines.splice(line - 1, 0, comment);
}
content = lines.join('\n');
fs.writeFileSync(filePath, content, 'utf-8');
});
It’s then used like this:
git checkout :/ && npx eslint --format json > complaints.json ; node /tmp/add_comments.js complaints.json && npx eslint
Notable items about the script:
- Crude way to select an input file. I miss
argparse
- Grouping the errors by line, as one line might have multiple errors
- Applying the fixes in reverse, as adding new lines will skew the line numbers of the following lines
- Not caring about indentation - this can be handled by e.g.
prettier
Solution #2: jsx
Running the same procedure on files containing JSX didn’t work as well, as it produced JS comments in the middle of JSX templates. Consider this:
function best() {
return (
<div>
<ul>
<li key="item">Item 1</li>
<li key="item">Item 2</li>
</ul>
</div>
);
}
Naively, it would get processed into
function best() {
return (
<div>
The result of 1 + 1 is: {result}
<ul>
// eslint-disable-next-line react/jsx-key
<li key="item">Item 1</li>
<li key="item">Item 2</li>
</ul>
</div>
);
}
Where the new comment obviously breaks the JSX tempalte. It’d need to be converted into this:
function best() {
return (
<div>
<ul>
{/* eslint-disable-next-line react/jsx-key */}
<li key="item">Item 1</li>
<li key="item">Item 2</li>
</ul>
</div>
);
}
I considered improving the original script to detect whether we’re inside a JSX tag or not, but it was too complicated. Instead, I decided on running ESLint again, picking up its complaints on “you can’t have a comment here” and rewriting the lines referenced.
// jsx_comments.js
const fs = require('fs');
const path = require('path');
const assert = require('assert');
const errorsFilePath = process.argv[2];
if (!errorsFilePath) {
console.error('Please provide the path to the eslint errors JSON file as an argument.');
process.exit(1);
}
const errors = JSON.parse(fs.readFileSync(errorsFilePath, 'utf-8'));
errors.forEach(file => {
const filePath = path.resolve(file.filePath);
let content = fs.readFileSync(filePath, 'utf-8');
const lines = content.split('\n');
const messages = file.messages.filter(m => m.ruleId == "react/jsx-no-comment-textnodes");
messages.forEach(m => {
let lineIndex = m.line;
// Eat some newlines
while (/^\s*$/.test(lines[lineIndex])) {
lineIndex++;
}
const line = lines[lineIndex];
assert(/^\s*\/\/ eslint-disable-next-line/.test(line), `line needs to match: ${line}`);
const newLine = line.replace(/^\/\/ eslint-disable-next-line (.+)$/, '{/* eslint-disable-next-line $1 */}');
lines[lineIndex] = newLine;
});
content = lines.join('\n');
fs.writeFileSync(filePath, content, 'utf-8');
});
Then
git checkout :/ && npx eslint --format json > jsx_complaints.json ; node /tmp/jsx_comments.js jsx_complaints.json && npx eslint
Or all at once:
git checkout :/ \
&& npx eslint --format json > complaints.json ; node /tmp/add_comments.js complaints.json \
&& npx eslint --format json > jsx_complaints.json ; node /tmp/jsx_comments.js jsx_complaints.json \
&& npx eslint
Worked great.