My React 18 Migration Adventure

I recently completed the migration of a large mono-repo containing three applications and about 30 packages to React 18. This turned out to be a huge undertaking, not because of the React 18 migration itself, but because of the amount of work that was required to get the applications to a state where they could be migrated. I thought I would share some of the lessons I learned along the way.

The Problem

If you’re among the 1% that updates every package in your massive mono-repo regularly, then it won’t be very difficult to make the switch to the latest version of React. However, if you’re like most of us, you’re probably dealing with some libraries that are a bit dated, as was I. The good news is that not everything needs to be upgraded, but there are certain libraries that will send you through a world of pain if you don’t take care of them first. For me, the additional packages that needed to be upgraded were:

Doing the package upgrades was the easy part. It was the part where I had to figure out what changed in each of these libraries and what all needed to be refactored as a result that made things interesting. The main offenders here was react-query (now TanStack Query) and React Testing Library. Some honorable mentions go to react-dnd and react-aria (more on that later). The react-query upgrade was a bit of a pain, but the react testing library upgrade was a nightmare. I’ll go into more detail about that later as well.

The First Attempt

First I should take a step back and describe why my first attempt at this a few months ago failed. After upgrading the packages and modifying about 150 files to comply with the upgrades, I decided to rebase the branch from master to deal with any potential merge conflicts and makes sure I had the latest code. Prior to the rebase, I had fixed about 250 failing tests and had about another 500 or so to go. After the rebase, I had well over 1,000 failing tests all with the same error - “act implementation is not a function”. I had no idea what this meant, but I knew it was a serious problem because the failures prior to the rebase were all legit.

I spent a few days trying to figure out what was going on, but I couldn’t find anything that would help. I had a number of other developers look at it as well and they were all dumbfounded. One developer said they had seen it before when a library was using a different version of React, but that wasn’t the case here. Yes, the master branch was still on React 17 and the older versions of the other packages, but my upgrades were still intact after the rebase along with the pnpm.lock file. I knew it had something to do with the rebase but after trying absolutely everything, I finally ended up abandoning the branch and decided to put this migration on hold for a while. I was pretty frustrated at this point and I didn’t want to deal with it anymore, but my determination not to let it beat me took over and I decided to try again.

The Solution

The second time around, I took a much more methodical approach:

1. Upgrade all of the package.json files locally, but don’t commit them until the very last step. That way the branch can be rebased as needed until it is ready to be merged.
2. Since so many files had to be touched as part of the upgrades, make these changes iteratively, breaking them up into several PRs (which sounded great in theory but turned out to be unrealistic in practice)
3. Lock down the master branch between the time that those files are committed and the time the PR is merged to prevent any potential merge conflicts that would require a rebase.

The main reason there was such a high number of failing tests (somewhere in the vicinity of 800) was because as of the @testing-library/user-event v13 release, which requires all tests that involve the userEvent to by async and all userEvent’s to be awaited. Needless to say, we had a lot of tests that were not written that way, so a large portion were able to be fixed by simply adding the async keyword to the test and adding await to the userEvent calls. Additionally, the userEvent is now automatically wrapped with act(() => {}), so that had to be removed in about 200 places. My first PR took care of a lot of these as well as a few other fixes, all of which were backward-compatible with React 17, which the apps and packages were all still running. I couldn’t just upgrade everything yet because there was still a lot of things that needed to be fixed.

After that first PR got merged, I started to look at some of the more serious issues, the most prominent being the fact that the apps would not build because there was an issue with the QueryClientProvider from react-query that made it apparent that we would need to upgrade that library as well. I decided to do the react-query upgrade on a separate branch because it was clear after going through the migration docs that there would be lot of changes involved. Around this time I was able to get support from some of the other frontend engineers who freed up and saw what I was working and wanted to help.

Upgrading React Query took a full week, although I was also juggling it with my regular work. Most of the changes were simple enough, like wrapping query key strings in arrays, modifying type declarations, and changing isLoading to isInitialLoading wherever a query was being disabled until it had some data or state set. I did get jammed up in a spot where useInifiniteQuery was being used directly from a component instead of the data-provider package that holds all of the data services, wihch required some refactoring. Luckily by the time I finished a lot of progress had been made on the test fixes needed on the React 18 upgrade branch.

Since a lot had been done in terms of fixing failing tests for React 18 while I was working on React Query, I wanted to submit another PR for it. This is when it became very apparent that my idea of doing the migration iteratively was about to go down the drain.

While all of their fixes worked fine locally in the React 18 environment, I saw some things failing in the CI environment that was still running the older versions. For example, there was a slew of tests that expected something to be called x number of times. To get those tests to pass on the React 18 upgrade branch, those numbers were all reduced by one. At first this seemed strange and it bothered me because I didn’t want to change those tests just to make them work for React 18. That’s when it occurred to me that the automatic state batching feature that ships with React 18 was the most likely culprit. While the flushSync function can be used to disable the automatic state batching where the state is being updated for those tests, the UI still worked as expected so I ended up deciding to leave those changes in place. The problem was that this meant the code would no longer make it through the CI pipeline in order to be merged. I actually spent almost a full day trying to make all of the changes that were made to the React 18 upgrade branch while I was working on the React Query upgrade to be backward compatible with the older versions, I settled on it being more trouble than it was worth and if I had to submit a PR with five hundred different files in it, so be it. The PR review would have to be a team effort and I would just communicate this to everyone well in advance of the PR being submitted.

Once the React Query upgrade was merged and I came back to the React 18 upgrade branch, there was still a bunch of abstract fixes that needed to take place, meaning things that did not necessarily have a straight-forward fix. Luckily with the support of my team we were able to work through them and finally arrive at a point where everything was working and the tests were passing. Then it was just a matter of regression testing, which I was able to elicit the support of the rest of the frontend developers to help with.

One of the more significant issues we ran into was the change to StrictMode in React 18. StrictMode is a tool for highlighting potential problems in an application. In React 18, StrictMode does a double invocation of components, where it mounts -> unmounts -> re-mounts. This is done to ensure that components are resilient to effects being mounted and destroyed multiple times in anticipation of a feature that will be added in the future that allows React to add and remove sections of the UI while preserving state. The problem was that it exposed some issues with our UI. For example, we found that when running the application locally, it would not redirect back to localhost anymore after login. To fix this, the /callback route had to be modified from this:

export function Callback({history, pendingElement}: TCallbackProps) {
  const {handleRedirectCallback, isAuthenticated} = useAuthServiceContext();

  useEffect(() => {
    if (!isAuthenticated()) {
      handleRedirectCallback({history});
    }
  }, [isAuthenticated()]); // eslint-disable-line react-hooks/exhaustive-deps

  return <>{pendingElement}</>;
}

To this:

export function Callback({history, pendingElement}: TCallbackProps) {
  const {handleRedirectCallback, isAuthenticated} = useAuthServiceContext();
  const authed = isAuthenticated();

  useEffect(() => {
    let ignore = false;

    if (!authed) {
      setTimeout(() => {
        if (!ignore) {
          handleRedirectCallback({history});
        }
      }, 0);
    }

    return () => {
      ignore = true;
    };
  }, [authed]); 

  return <>{pendingElement}</>;
}

The problem has to do with data fetching in a useEffect, which isn’t exactly what we are doing with the handleRedirectCallback, but the effect is essentially the same. This post on reddit is pretty informative on the subject and the technique to resolve it was found in the beta React docs.

All in all, the upgrade was a success this time around, and before I even had the React 18 upgrade branch merged, I had already branched off of it and started experimenting with server-side rendering to see if I could get the apps to start using SSR. So far this has proven a little more challenging than I’d hoped, but I expect to write about this next adventure in a future a post, so, to be continued…