Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to react and react-dom 18.x #3372

Closed
5 tasks
davemcorwin opened this issue May 18, 2021 · 5 comments · Fixed by #4044
Closed
5 tasks

Upgrade to react and react-dom 18.x #3372

davemcorwin opened this issue May 18, 2021 · 5 comments · Fixed by #4044

Comments

@davemcorwin
Copy link
Contributor

davemcorwin commented May 18, 2021

  • update react dep to 18.x
  • update react-dom dep to 18.x
  • remove react from dependabot "ignore" config
  • remove react-dom from dependabot "ignore" config
  • code migrations
@davemcorwin davemcorwin self-assigned this May 20, 2021
@apburnes apburnes changed the title Upgrade to react and react-dom 17.x Upgrade to react and react-dom 18.x Oct 25, 2022
@drewbo drewbo assigned drewbo and unassigned davemcorwin Nov 28, 2022
@drewbo
Copy link
Contributor

drewbo commented Nov 28, 2022

@drewbo
Copy link
Contributor

drewbo commented Nov 29, 2022

It turns out the react upgrade itself is fairly easy but there are a few adjacent dependencies that we should also consider upgrading at the same time (my recommendations in emoji form):

  • 👍 Enzyme: As noted above, our testing framework enzyme doesn't provide adapters past React 16. There is an "unofficial" adapter for React 18 but it specifically says you probably shouldn't use it. Our tests could probably use a quick once over anyways so I'd like to upgrade this at the same time
  • 👍 @reach/router: Our router is merging with React-router v6; this seems like a fairly easy swap.
  • redux-form: The author of this library advises not starting new projects with the library and upgrading to their own replacement: https://final-form.org/react/. I'm okay leaving for now (with a peer dependency warning) and updating later
  • react-notification-system: This library and the associated redux wrapper haven't been updated in a few years but probably still work with react 18 (minus the peer dependency warning). There are a number of replacements (although I'm surprised there hasn't been consolidation) that we can update at another time.

@drewbo
Copy link
Contributor

drewbo commented Dec 2, 2022

So the further I move down this path, the more it becomes clear that this is a pretty tough refactor so I'm going to propose breaking it up even further and doing additional research. The main sticking point is that we're currently using Redux for state management and React-Router-v6 is extremely reliant on React Hooks (Redux and React Hooks being mostly mutually exclusive as patterns). It seemed like there was a way to maintain both except I'm not sure how to resolve Redux actions which are used for redirection, which now require a hook variable (and thus can't be made as "pure actions" or easily used outside React components). I think our path forward looks something like this

    1. Replacing Redux as the state management tool with React Hooks. Hooks are already available in React 16.8 so we don't need to simultaneously upgrade there. It might make sense to do this simultaneously with a router upgrade because of the additional loader property on the Routes which could help replace certain global state loading (like user/site/organization fetching).
    1. Separately upgrade the router if not done above
    1. Upgrade to React 18
    1. Replace the test suite with something like react-testing-library at this point (keeping Enzyme tests to track refactor stability)

@drewbo
Copy link
Contributor

drewbo commented Dec 14, 2022

More attempts at this and a new, more modular plan

Context: the original (above) plan is semi-plausible but requires a huge initial lift that would be quite hard to test and review. A partial attempt at this can be seen at chore-react-18. Also it is not necessary to remove Redux at first: we can instead convert all class components to functional components, add hooks as necessary, and still use useSelector to replicate the functionality of connect from react-redux. So in service of a more modular approach, I'd like to replace the first two bullets of the above plan with:

  • Create a new branch with the aim of removing class components, then I'll merge components in batches of ~1-3 to this branch for review. We can optionally merge this branch to staging as a "hooks-ready" option.
  • Now that everything is operating within functional components, we can update to hooks and react-router simultaneously with less change to their entire application at once.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants