- Complex single-page application in production.
- Codebase created over 2 years ago (legacy code). People who architected and wrote a big part of the code don’t work here anymore.
- Still a lot of feature requests.
- More than 1300 files.
- Over 230K lines of code.
- Has Flow already but not everything was typed with Flow and Flow kind of sucks.
Motivation for Migrating to TypeScript
The major thrust to migrate the CAT Tool to TypeScript was Tech debt.
Tech debt has been slowing us down from implementing new features and caused.
In the words of Alex, one of the major contributors to the CAT Tool:
CAT Tool is monstrous ~Alex
It’s scary and frustrating to work with codebase with a lot of tech debt. There’s copy-pasted code everywhere. Some files has so many lines of code that lint stops working. Every little change has the potential to cause regression and new bugs.
We have many Jira tickets specifically targeted at addressing tech debt but those efforts require a lot of refactoring. Having everything properly typed in TypeScript makes refactoring and less scary.
How to Migrate?
Originally wanted to incrementally Migrate the CAT Tool to TypeScript.
It’s going to take years ~Alex
Another reason we chose to migrate all at once instead of gradually and incrementally was because this codebase is constantly being updated with new features.
Doing the migration in a Hackathon makes sense for us because everyone who participated in the Hackathon had previous experience migrating our other codebases to TypeScript and had experience working with the CAT Tool.
We created WIP PRs for things we are currently working on so they are saved remotely. Resolving merge conflict will not be trivial.
Configure the project for TypeScript. This included things like adding packages, adding tsconfig, updating lint rules, updating loaders, updating file names, deleting Flow stuff from the codebase.
A half day was spent preparing the codebase for migration on the day of the hackathon, we have all hands on deck resolving errors with build and type check.
The goal of the Hackathon was to migrate the codebase to TypeScript without making any changes to the source code (unless trivial or absolutely necessary).
We collaborated in the same workspace with VS Code Live Share.
The Hackathon was a great team building exercise. We really bonded over laughs about some weird code we discovered in the codebase when we were fixing the type errors.
There were some silly mistakes in the code that we fixed during the Hackathon (They would have been caught if the code were better typed). For example:
- In many places,
parseIntwas performed on a number that’s the index of an array. To fix this type error without making changes to the source code, we had to cast the number into a string so it can be converted back into a number with the
- Non-existed css classes were used in React component. Those css classes did not exist in any of the style sheets so the solution was to delete them from the code.
We started the migration with over 4000 errors. At the end of the hackathon, we had fewer than 700 errors.
This was the result of the PR when it was merged to master.
Challenges and Lessons Learned
There were some hurdles and issues during and after the migration.
Although the codebase was previously set up with Flow, not all files were properly typed. Some files were not typed at all. Flow is also less strict about typing than TypeScript. We ended up having to cast a lot of things to
Most of our test files were not typed at all and contained a lot of boilerplate. We decided to exclude the specs from ts and lint for now because it’d take too long to refactor/add types to the specs because they don’t use global mocks.
PR review and version control
We had to change many files from
.ts and we want to do it without losing commit history. Because we were committing a lot of files at once, git gets confused about the file renaming and thinks that the
js files were deleted and the
ts files are new.
The same problem happened with peer review on Github. This can make PR review very challenging. What we ended up doing to make PR review possible was to rename the ts files back to js files just for the PR, then revert them back to ts when we were ready to merge to master.
Celebrate With Whisky
… But not before fixing a regression introduced by the migration
A critical bug appeared after the migration but it was quickly fixed.
The cause was a typo. A one-line change was quickly merged to master.
There are a few reasons this happened:
- TypeScript did not catch the error because the typecasting to
- Did not have sufficient automated unit test coverage
- Did not perform a manual regression test thoroughly before merging to master
There are a lot of configurations and use cases for the CAT Tool that necessitates testing the application in multiple special setups
During our bi-weekly retro, we discussed the need to create a repository of test projects so we can effectively regression test. We also need better documentation from Product about how the product is supposed to work and all its features.
It’s not possible to perfectly migrate a codebase of this size in a few days or weeks. There were a lot of “hacks” (e.g., type-casting to
any and excluding files from linting and ts check) we used to complete the initial migration. We created follow-up tasks to address these.