ESLint-ing a Legacy Codebase
I recently joined a new team in my organization, and upon arrival, realized how much I loved (and wanted to share) my previous developer experience: an application running on the latest version of React, a super fast Webpack build, and an ESLint configuration that kept our code clean and free of many accidental bugs that can be caused by good ol’ human error. This new team’s codebase is an older application, not just recently built from the ground up, so of course there are a few issues to tackle. When working with legacy code, one is untangling a history of code written by many people, with various levels of experience, in ways that used to be “popular” and syntactically correct, but have since fallen by the wayside in favor of other stylistic opinions.
As engineers, we tend to forget to look at our code as if we have a fresh set of eyes. I am in the unique position of seeing this code for the first time, and want to make sure that moving forward, we are going through it with a fine-toothed comb and fixing any errors we may see along the way. For example, the many people that have contributed to this codebase in the past may have introduced some improper usage of the React framework without knowing it. We want to make sure that we’re not directly calling functions within our React components’ props, not passing around entire objects with unexpected shapes, not using string refs, etc. These sorts of things could not have been so easily caught without a linter, so it is inevitable that some problems still linger. More importantly, these sorts of issues are not just syntax-related, but are ripe for causing bugs and unexpected user experiences down the line.
Enter ESLint. In my downtime, I was curious to see what kind of an endeavor linting this application would be like. So, I set us up with an ESLint config (one that our organization uses) and ran the necessary command…
When running the command line utility, a flag called “ — fix” can be appended to automatically fix smaller issues with whitespace, single vs. double quotes, adding trailing commas, etc. After running the command, I sighed a sigh of relief:
So where do I go from there? My first reaction was to just start going file by file and fixing every error I found manually. It was not going to get done quickly, and while I fixed things, more and more code was getting into master, with the possibility of adding to this giant stack of errors. I needed buy-in from the team. I couldn’t just take this all on by myself.
My first step was to introduce the config into our codebase, but make it optional for engineers to have ESLint running on every file change. (That would be overwhelming for anyone, not just little perfectionist me.) What this did (besides get my opinionated foot in the door) was made it possible for me to run the linter against any file I wanted. And so I began to create PRs that linted single files, in an attempt to fight the good fight.
That first step helps a lot, but still does not prevent bug-ridden code from entering the codebase. All I could think was, “How can I make this a piecemeal process? I don’t want to create a gigantic PR of every single file being fixed.”
I took a cue from the previous team’s dream setup and researched pre-commit hooks. Aha! I could just create a pre-commit hook that linted that branch’s changed files. That way, every team member would be contributing to this effort without us having to set aside a large chunk of time to get it all done at once.
The pre-commit hook was rather simple to create. I set us up with Husky and found a very useful shell script (see brunops’s comment from May 6, 2015) for running ESLint against staged commits.
Going forward, we will not be able to commit to branches unless the changed files pass our linting standards. Of course, in my free time, I want to continue to lint files that aren’t being touched, because I am super eager to get our overall linting errors to a glorious zero.
I plan to occasionally check up on our team’s progress in this effort and once we have gotten all files’ linting errors down to zero, we will make the change to have our Webpack build constantly running ESLint. Whenever we make a change to a file, the linter will warn us of any issues along the way. At that point, we will have a much cleaner codebase, a consistent style, more up-to-date usage of React, and most importantly: we are guaranteed to have less bugs. If you are ever entering a team with legacy code and the option of incorporating a linter seems overwhelming, do not fear: it really is possible to do this thing piecemeal. It may take some time, but the benefits greatly outweigh foregoing the effort.
