Upgrade react-router from v3 to v5.1.2

alexandec commented 2 years ago

Upgrade documentation and tips

For help upgrading routes and links in an app, see this documentation further down in the ticket.

Background

The official React router documentation recommends upgrading to v5.1 first, then moving to v6 as an additional step. This is the approach we're planning to take. In this ticket we'll only cover the upgrade from v3 (our current version) to v5.1.2.

See Upgrade react in vets-website to v17 for additional information and context.

Upgrading from react-router v3 to v5.1.2

Determine the changes required to move from v3 (current) to v5.1.2 (latest 5.x release)
Document changes in this ticket
Create a PR containing the required changes and upgrading react-router to 5.1.2. Merge PR

Upgrading from react-router v5.1.2 to v6.2.1 (future steps)

Upgrade react itself
Determine the changes required to move from v5.1.2 to v6.2.1
Wait for backwards compatibility package to be released (optional)
Create a PR containing the required changes and upgrading react-router to 6.2.1. Merge PR

alexandec commented 2 years ago

Changes required:

Upgrade react-router to 5.1.2
Upgrade history to 5.2.0
Remove history-v4 and change imports to history
Change <IndexRoute component={Introduction} key="/intro" /> to <Route exact path="/" component={Introduction} key="/intro" />
Change import { Link } from 'react-router' to import { Link } from 'react-router-dom'
Change createHistory to createBrowserHistory
More changes to come

alexandec commented 2 years ago

Route definition changes

Previously routes were defined using objects like this:

[
  {
    path: '/first',
    component: FirstComponent
  },
  {
    path: '/second',
    component: SecondComponent
  },
]

WIth the router upgrade, routes must instead be defined using router components, like this:

<Switch>
  <Route path="/first">
    <FirstComponent />
  </Route>
  <Route path="/second">
    <SecondComponent />
  </Route>
</Switch>

alexandec commented 2 years ago

For an example of the correct route definition method in the upgraded react-router, see the vaos app, which already uses the new method. vaos/routes.jsx defines 4 top-level routes, each with an associated component. Here's a shortened version showing one route:

<VAOSApp>
  <Switch>
    <Route
      path="/new-unenrolled-covid-19-vaccine-booking"
      component={asyncLoader(() =>
        import(/* webpackChunkName: "unenrolled-vaccine" */ './unenrolled-vaccine')
          .then(({ UnenrolledVaccineSection, reducer }) => {
            store.injectReducer('unenrolledVaccine', reducer);
            return UnenrolledVaccineSection;
          })
          .catch(handleLoadError),
      )}
      ...
    />
  </Switch>
</VAOSApp>

Moving down into the UnenrolledVaccineSection component, we can see child routes defined there:

<FormLayout>
  <Switch>
    <Route
      path={`${match.url}/confirmation`}
      component={ConfirmationPage}
    />
    <Route path="/" component={PlanAheadPage} />
  </Switch>
</FormLayout>

We'll have to adopt this approach in order to upgrade react-router. On the positive side, we can refactor route definitions first, prior to upgrading react-router. We don't have to refactor everything at once.

alexandec commented 2 years ago

Redirects

Currently we perform redirects like this:

indexRoute: { onEnter: (nextState, replace) => replace('/introduction') },

This is a commonly-used redirect for form rendering. These redirects need to be modified to use Redirect instead, like:

<Route render={() => (<Redirect to="/introduction" />)} />

This change needs to be made in 29 locations.

alexandec commented 2 years ago

Here is a POC to transform the existing route objects returned by createRoutesWithSaveInProgress() into components suitable for use with the new react-router version. The file is src/applications/pre-need/routes.jsx:

import React from 'react';
import { Switch, Route } from 'react-router-dom';

import { createRoutesWithSaveInProgress } from 'platform/forms/save-in-progress/helpers';

import formConfig from './config/form';

const routeObjects = createRoutesWithSaveInProgress(formConfig);
const routes = routeObjects.map((routeObject, i) => (
  <Route
    path={`/${routeObject.path}`}
    component={props => (
      <routeObject.component {...props} route={routeObject} />
    )}
    key={i}
  />
));

const route = <Switch>{routes}</Switch>;

export default route;

With this approach I was able to work my way through each route in the burials-and-memorials/pre-need/form-10007-apply-for-eligibility form and reach the end (form submission). This shows that the approach works with our existing react-router version (3) so we can use it to prepare apps ahead of time, before upgrading.

However this solution isn't complete because we're putting all the child routes at the top level. The wrapper components are not used so the form layout isn't correct. I'll work on adding in those wrapper components next.

alexandec commented 2 years ago

Deprecated router usage

Route definitions

Apps marked as OLD-FORM use deprecated route definitions (JS objects) generated by the forms library. These apps cannot be updated to use the new react-router until the Design System team modifies the forms library route generation logic. This Design System support request covers the required updates.

Apps marked as OLD manually define their routes as JS objects. App owners can freely update these routes to use the new component approach.

Apps marked as NEW already define their routes using the new component approach. They do not require route definition changes.

Apps typically require further changes beyond route definitions, to address breaking changes in react-router. These are noted in the "Additional deprecations" column.

Inventory

App	Route definitions	Additional deprecations
appeals/10182	OLD-FORM	indexRoute, childRoutes, withRouter, old import
ask-a-question	OLD-FORM	indexRoute, childRoutes
auth	OLD	location.query
beta-enrollment	OLD	childRoutes
burials	OLD-FORM	indexRoute, childRoutes
caregivers	OLD-FORM	indexRoute, childRoutes
check-in/day-of	NEW	location.query, withRouter
check-in/pre-check-in	NEW	location.query
claims-status	NEW	IndexRedirect, IndexLink, nested child routes, withRouter, old import
coronavirus-research/sign-up	OLD-FORM	indexRoute (customized), childRoutes
coronavirus-research/update	OLD-FORM	indexRoute (customized), childRoutes
coronavirus-screener	NEW	Old import
coronavirus-vaccination-expansion	OLD-FORM	indexRoute, childRoutes, withRouter
coronavirus-vaccination	NEW	indexRoute, nested child routes, location.query, withRouter, old import
debt-letters	NEW	-
disability-benefits/2346	OLD-FORM	indexRoute, childRoutes
disability-benefits/686c-674	OLD-FORM	indexRoute, childRoutes
disability-benefits/996	OLD-FORM	indexRoute, childRoutes, old import
disability-benefits/all-claims	OLD-FORM	indexRoute, childRoutes, old import
disability-benefits/view-payments	NEW	Old import
discharge-wizard	OLD	indexRoute, childRoutes, old import
e-folders	OLD	-
edu-benefits/0993	OLD-FORM	indexRoute, childRoutes, withRouter
edu-benefits/0994	OLD-FORM	indexRoute, childRoutes
edu-benefits/10203	OLD-FORM	indexRoute, childRoutes
edu-benefits/1990	OLD-FORM	indexRoute, childRoutes
edu-benefits/1990e	OLD-FORM	indexRoute, childRoutes
edu-benefits/1990n	OLD-FORM	indexRoute, childRoutes
edu-benefits/1990s	OLD-FORM	indexRoute, childRoutes
edu-benefits/1995	OLD-FORM	indexRoute, childRoutes
edu-benefits/5490	OLD-FORM	indexRoute, childRoutes
edu-benefits/5495	OLD-FORM	indexRoute, childRoutes
edu-benefits/feedback-tool	OLD-FORM	indexRoute, childRoutes
facility-locator	OLD	indexRoute, childRoutes, browserHistory, location.query, old import
financial-status-report	OLD-FORM	indexRoute, childRoutes
gi-sandbox	NEW	-
gi	NEW	-
hca	OLD-FORM	indexRoute, childRoutes
health-care-questionnaire/list/questionnaire-list	NEW	-
health-care-questionnaire/list	NEW	-
health-care-questionnaire/questionnaire	OLD-FORM	indexRoute, childRoutes, withRouter
letters	NEW	IndexRedirect, nested child routes, old import
lgy/coe/form	OLD-FORM	indexRoute, childRoutes, old import
lgy/coe/status	NEW	Old import
login	OLD	indexRoute, childRoutes
medical-copays	NEW	nested redirect
messages	NEW	Old import
mock-sip-form	OLD-FORM	indexRoute, childRoutes
my-education-benefits	OLD-FORM	indexRoute, childRoutes
pensions	OLD-FORM	indexRoute, childRoutes
personalization/dashboard	-	Old import
personalization/profile	NEW	Old import
personalization/rated-disabilities	OLD	-
personalization/search-representative	OLD-FORM	indexRoute, childRoutes
personalization/view-dependents	OLD	-
personalization/view-representative	NEW	Old import
platform/user/authorization	-	location.query, browserHistory, withRouter
platform/startup	-	browserHistory, old import
post-911-gib-status	NEW	IndexRoute, nested child routes, old import
pre-need	OLD-FORM	indexRoute, childRoutes
sah/sahg	OLD-FORM	indexRoute, childRoutes
search	OLD	location.query, withRouter
terms-and-conditions	OLD	location.query
vaos	NEW	-
veteran-representative	OLD-FORM	indexRoute, childRoutes
virtual-agent	NEW	Old import
vre/28-1900	OLD-FORM	indexRoute, childRoutes, old import
vre/28-8832	OLD-FORM	indexRoute, childRoutes

alexandec commented 2 years ago

Upgrade tips

Update the Link component

You will need to change

import { Link } from 'react-router'

to

import { Link } from 'react-router-dom'

After doing so, you may notice unit test failures due to a missing context. The fix is to wrap the component under test in a router component, as described in the react-router testing guide.

Switch to the new startApp() function

The standard pattern for VSP apps is to call the startApp() function to bootstrap the app. However, when you switch to the new react-router v5 route definitions and components, the existing startApp() function is no longer compatible. Instead use the new startApp() function in src/platform/startup/router.js. This should be a drop-in replacement.

Replace withRouter with hooks

In react-router v3 it was common to use the withRouter HOC to access router information or perform navigation. In v5, withRouter is replaced by simple hooks like these:

import { useParams, useLocation, useHistory } from 'react-router-dom';

const params = useParams();
const location = useLocation();
const history = useHistory();

history.push(`${location.pathname}/more/${params.id}`);

The react-router documentation lists supported hooks with example usage.

Avoid using the render or component props

Instead of placing your component inside the <Route render=...> or <Route component=...> props, just make your component a child of the Route component, like:

<Route path="/mypath">
  <MyComponent />
</Route>

This method is cleaner because you will explicitly pass any props your component needs.

timwright12 commented 2 years ago

I don't remember by I closed this, but I reopened it based on today's Q3 planning conversation

taylorkaren commented 2 years ago

The Forms Library Team is no longer working on a React Router V3 --> V5 upgrade. Given that V3 and V5 apps currently co-exist on Vets Website, and that any form apps on Router V3 will need a rewrite to migrate to the new VA Forms System Core (VAFSC) anyway, providing a migration path from React Router V3 to V5 is a step that doesn't provide a good return on time invested for the Forms Library Team.

For details on this decision, please see these Confluence pages:

The Forms Library Team will instead be working on a React Router V5 --> V6 upgrade, see this ticket for details.

Other tickets that reference React Router V3 -V5 upgrades:

laineymajor commented 1 year ago

@k80bowman @caw310 --- can we please chat about this soon. It is a blocker for us right now.

caw310 commented 1 year ago

@laineymajor , let's chat on Monday. Katy is out sick this week.

laineymajor commented 1 year ago

@timwright12 can you specify what this is blocking within the ticket. Want to make sure we're tracking. Thanks!

department-of-veterans-affairs / va.gov-team